[pieh]

Hi guys,
currently we can't focus empty container and this is causing some headaches with focus sometimes (see http://forum.xbmc.org/showthread.php?tid...pid1303866 ).

Most desired fix is to make empty container focusable but there are some issues that need to be thought about:

• Making empty container focusable could mess directional navigation in some situations - now if container is empty, we just pass navigation through that container and move along (I mean <onleft> etc navigation). With proposed fix we would stop navigation on empty container which might be unwanted in some cases (f.e. in confluence home window we could focus recently added/addon shortcuts container even if they would be empty). To avoid that we would need to add <enable> condition to containers where we want old behaviour or the other way around - add some flag that will allow to focus empty containers (as most of the times focus headaches are with containters with media listings and not static content).
  
• We have no builtin way to indicate that container is focused. So here goes question - do we need such builtin method? We already need to make use of "Control.HasFocus(container_id)" in itemlayouts to differentiate unfocused container from focused one. To show/hide indicator we could use visible condition "Control.HasFocus(container_id) + StringCompare(Container(container_id).NumItems,0)" (or something like that, this was not tested).

So what are your thoughts on this?

EDIT: forgot to add - this for sure won't be added before Frodo, so don't worry about breaking your skins now

[`Black]

We have no builtin way to indicate that container is focused.

Doesn't Control.HasFocus(id) do that?

Anyway, if a container is empty focusing recently added or something else shouldn't be a problem. Let's say you have recently added with onup and a visible condition for recently added if Container(id).HasFocus(some_item_id). If a container is empty, Container(id).HasFocus(item_id) should always return false.

[MassIV]

Isn't that what default control is for? Shouldn't IT just keep trying and trying?
Or does the container in that case keep indicating that it is empty (when it was just slow)?

[pieh]

We have no builtin way to indicate that container is focused.

Doesn't Control.HasFocus(id) do that?

Yes, but as "builtin" I mean some direct xml tag in container control. To use that infolabel You'd need seperate control. That was point of second question - if we need new feature in containers to cover empty items scenarios or it's enough if this can be currently handled by additional control with visible condition containing "Control.HasFocus(id)"

If a container is empty, Container(id).HasFocus(item_id) should always return false.

Sure, but you would have to check all item_id's.

From brief view, in confluence there is no <onup> conditions, we don't try to focus recently added container because this is how xbmc behaves currently - we check <onup> navigiation, if next control can't be focused (control is disabled / container has no items) we will check that control's <onup> and repeat that until we can focus something or when we finished loop through <onup> navigiation and ended up again in one of the controls we already checked. So at least confluence's home window navigation relies on fact that we can't focus empty container and I bet there is much more skins using this behaviour.

There is way to add condition <enable> condition to keep current behaviur but it still would require to update skin .xmls.

Isn't that what default control is for? Shouldn't IT just keep trying and trying?
Or does the container in that case keep indicating that it is empty (when it was just slow)?

We test if currently focused item can still have focus. If defaultcontrol can't have focus we end up with no focus at all (if we don't have focus we don't fallback to defaultcontrol).

[MassIV]

if we don't have focus we don't fallback to defaultcontrol.

Well that would be my suggestion i guess (keep trying default control if you have no focus). Or as alternative something in the direction of making skins handle the first step after default control. As in, don't make the container your default control but use a 'dummy'.
The downside is that you need an action to go from the dummy to the container.

What confuses me though is that you say:

Making empty container focusable could mess directional navigation in some situations

But don't we use visible conditions? Or skin strings that we set ourselves? Instead of checking if something is actually empty or not?
Does empty automatically mean not visible?

[pieh]

Well that would be my suggestion i guess (keep trying default control if you have no focus).

This won't work for mouse/touch input - as most of the time then we don't have anything focused and if xbmc would try to focus default control constantly this could be messy.

Or as alternative something in the direction of making skins handle the first step after default control. As in, don't make the container your default control but use a 'dummy'.
The downside is that you need an action to go from the dummy to the container.

This is not just about defaultcontrol. There was report that when we open selectdialog to pick music scraper and there are no enabled scrapers (default scraper is marked broken) we couldn't navigate to "get more" button. Sure, this could be dealt in skin by adding some conditional navigation but just tell me, how often do you consider situation where container is not focusable and you prepare alternative navigation to cover that scenario?

What confuses me though is that you say:

Making empty container focusable could mess directional navigation in some situations

But don't we use visible conditions? Or skin strings that we set ourselves? Instead of checking if something is actually empty or not?
Does empty automatically mean not visible?

Empty doesn't mean not visible, it means not focusable (at least now) - there is difference. If visible condition was used then there would be no change. There would be changes in navigation that could cause problems if visible/enable conditions weren't used. I'll use confluence example again: home window, sub menu item is focused. Going down should focus addon shortcuts if there are any or power button if there are no addon shortcuts. Now there is no visible conditions used to check if there are any shortcuts, so empty container is technicly visible. But we can't focus empty container so that's why we pass navigation through and focus power button. So, proposed change, making empty container focusable, could mess navigation in some situation.

[`Black]

Is "no focus" the only problem? Or are there any other problems because an empty container is not focusable? Couldn't you add some kind of callback so that as soon as the container has content, it gets focus? Only if it's supposed to have focus of course.

[jmarshall]

In some cases an empty container should be navigated to (i.e. CanFocus() should return true). In other cases it shouldn't. I suspect the latter occurs in the exact same cases where the control should be hidden (it's empty after all, so you can't see anything anyway).

Thus, one solution may be for the skinner to rely on whether the container is empty as it's visible/enabled condition. i.e. <visible>!Container(id).IsEmpty</visible>. This relies on the skinner knowing those containers that might be empty but aren't relied on (such as the RA items on confluence home).

One thing we might need is a way to show the user that they're focused on that empty container. Currently we have no way to do this, so to the user, it might seem as if they're stuck. In fact, this alone might be enough to solve the issue (along with turning off the no-items-no-focus thing).

Side-note: In current master (and Frodo) there should be no empty lists in media windows as the ".." item pops up if the list is empty. This doesn't apply to other containers that are filled by XBMC such as the select dialog or PVR windows though.

Cheers,
Jonathan

[pecinko]

Hmm..

I personally would vote for empty=hidden and thus not focusable. Dialog progress should keep focus until there's something to pass focus to (it has cancel after all). In case container stays empty after given time (happens with some addons), Dialog Ok could pop-up with a message, whilst Ok button in it could pass focus to previous window/control.

[jmarshall]

That's not the case we're concerned with. The case that is problematic is where the list is supposed to be empty, yet navigation relies on it being focused. The select dialog is a good example - who would have thought it could ever be empty? It can. In that case XBMC could detect it and set focus to the "get more" button as we know which control should be focused, but in other cases we may not know that (custom skin windows for example). Being able to keep focus on the list means that navigation that depends on it being there still works correctly.

As pieh suggests, I'd envision this would be optional - if you want it hidden when empty, then specify <visible>!Container(id).IsEmpty()</visible> (we may be able to optimise this vis condition in future).

@pieh: does allowhiddenfocus have any effect here?

[pieh]

allowhiddenfocus will work as with any other control (https://github.com/xbmc/xbmc/blob/master...l.cpp#L370). It currently won't change anything because we check if container is empty before checking if container is visible or allow hidden focus. Not sure if that's what You were asking.

[`Black]

That's not the case we're concerned with. The case that is problematic is where the list is supposed to be empty, yet navigation relies on it being focused. The select dialog is a good example - who would have thought it could ever be empty? It can. In that case XBMC could detect it and set focus to the "get more" button as we know which control should be focused, but in other cases we may not know that (custom skin windows for example). Being able to keep focus on the list means that navigation that depends on it being there still works correctly.

Couldn't you search for active controls in the current window/dialog? If list is empty in a custom dialog, search for next focusable control and set the focus to it.

[jmarshall]

What is "next focusable control" is the problem. Do we just pick a random one, or pick the first one, or? What if the focus on that one does something destructive (skin can have a focus button do something like close the window)? We don't do this in any other cases.

@pieh: what about adding the allowhiddenfocus check to CGUIBaseContainer::CanFocus() so it overrides the no-items case. That way we can specifically state which ones are OK to allow hidden focus on.

IMO though, if we're allowing focus on a hidden list, then there needs to be a way to show the user where the focus is when there isn't any items.

[pieh]

@jmarshall:
I don't like idea of multipurposing allowhiddenfocus here to be honest. Sometimes skinner might want container to be hidden, allow hidden focus but not allow to focus empty container - I remember that allowhiddenfocus on container is used in alaska skin, multipurposing allowhiddenfocus could mess this up.

The more I think about it just removing item count check from CGUIBaseContainer::CanFocus (so making it behave like any other container) and adding boolean condition to check if container is empty ("Container(id).IsEmpty" ?) would be enough. Skinners job then would be to add "<enable>Container(id).IsEmpty</enable>" to containers they want old/current behaviour and add separate control with <visible>Control.HasFocus(container_id)</visible> to indicate that container has focus (skinners are already forced to use Control.HasFocus condition to "focused container / focused item" look different than "not focused container / focused item").

[jmarshall]

Sounds fine to me.