Page MenuHomePhabricator

Elementary needs a complete focus overhaul
Closed, ResolvedPublic

Description

This was discussed on irc, and I think most all would agree, Elementary's handling of focus is poor. It needs an overhaul.

Related Objects

StatusAssignedTask
OpenNone
Opencedric
Resolvedbu5hm4n
Resolvedbu5hm4n
Resolvedbu5hm4n
Resolvedbu5hm4n
Resolvedbu5hm4n
Resolvedbu5hm4n
Resolvedsingh.amitesh
Resolvedbu5hm4n
Resolvedbu5hm4n
Wontfixbu5hm4n
Resolvedbu5hm4n
Resolvedbu5hm4n
Resolvedbu5hm4n
Resolvedherdsman
Resolvedbu5hm4n
ResolvedYOhoho
Resolvedbu5hm4n
Resolvedbu5hm4n
Resolvedbu5hm4n
Resolvedeagleeye
Resolvedbu5hm4n
Resolvedbu5hm4n
Resolvedbu5hm4n
Wontfixbu5hm4n
Resolvedbu5hm4n
There are a very large number of changes, so older changes are hidden. Show Older Changes
stephenmhouston added a project: Restricted Project.Jan 21 2016, 7:05 PM
jpeg added a subscriber: jpeg.Jan 21 2016, 8:12 PM

I added an irc conversation from earlier today here that better explains what should be done.

<cedric> bu5hm4n, what are you talking about focus ??
<bu5hm4n> https://phab.enlightenment.org/T3088
<bu5hm4n> cedric: okra started it :P
<cedric> ah
<cedric> i can explain how to do it properly then
<cedric> the main issue with focus is that it is actually a graph algorithm
<cedric> you are going from one place to another
<cedric> if you try to implement it any other way, you will end up with dead end
<cedric> or weird behavior
<bu5hm4n> basically discover with depth search first
<cedric> stop their, won't work :-)
<cedric> you need to think that each object is a rectangle on a map
<cedric> and the goal is to make sure that from anywhere on that map you can reach every rectangle
<cedric> you have 2 set of different move
<cedric> tab/shift-tab and arrows
<cedric> they need to be handle differently
<cedric> so you will end up with 2 graphs
<bu5hm4n> eeew
<cedric> one for tab and another for arrow
<cedric> you start by building an graph with all easy link
<cedric> basically you get every rectangle neighbour and you link them with their neighbour
<cedric> a graph is better emulated by a matrix
<cedric> so you have the information to go from one rectangle to the other
<cedric> but splitted
<cedric> now the first step is to walk that graph

  • arrowd has quit (Read error: Connection reset by peer)

<cedric> and make sure that you can back and forth from every object
<cedric> it is very common that your focus chain won't be the same when you go back if you do not adjust it
<cedric> of course the issue with adjustement is that you are removing link
<cedric> leading to the potential that some object will have no way to be reached
<cedric> also you need to make sure that you do not leave any group of object disconnected from the rest
<cedric> you need to have forcefully all object connected
<cedric> once that graph is done, life is easy
<bu5hm4n> all objects can be done with the widget graph (for the tab case)
<cedric> you just need to look at the right row/column to know where to give focus next depending on your key
<bu5hm4n> thats very ugly
<cedric> why ?
<cedric> it is how every graph algorithm are implemented
<cedric> and that's exactly the problem you have
<cedric> trying to solve a graph problem, without using graph algorithm
<bu5hm4n> cedric: but checking which widget is right or left is nog that easy
<cedric> (there is a reason why nobody as tackled this problem so far, it is not trivial)
<cedric> it is
<cedric> you have to graphs
<cedric> one for tab and one for arrow
<bu5hm4n> yeah sure, if you have build this graph
<cedric> for arrow, you build your graph for all 4 arrows direction
<cedric> as I said, there is no way around having a proper result without resorting to graph
<bu5hm4n> but this would mean that every scroll on a scroller would mean we have to rebuild the graph
<cedric> at the beginning yes
<cedric> an easy optimisation is to jump mark it dirty and wait for someone to ask for the next focus
<cedric> also you are treating a small subset of the object on screen
<cedric> so it should not be very costly to do so
<bu5hm4n> cedric: is there a api to get all visible widgets which are displayed in a elm_win ?
<cedric> visible + focusable
<bu5hm4n> true
<cedric> i don't think so, but i think the proper way would be for each widget to register itself into the focus chain
<bu5hm4n> true
<cedric> and unregister when not needed anymore
<cedric> that should be part of the focus infra overall
<zmike> oh cool you guys are going to reimplement e_client's focus stack
<cedric> short story long, focus is way more complex than first view from air plane :)
<cedric> but thanks, graph theory is full of beautiful and fun algorithm ready to be implemented

  • zehortigoza has quit (Remote host closed the connection)
  • zehortigoza (~zehortigo@134.134.139.72) has joined

<bu5hm4n> cedric: do it :P
<cedric> :-D

zmike raised the priority of this task from Wishlist to High.Feb 17 2016, 12:54 PM
zmike added a subscriber: zmike.

Given the sheer number of focus-related issues and the impact that those issues have on users attempting to use elementary-based applications, this is by no means a low priority ticket.

I think there are significantly more severe issues than just the focus cycling. For example, there are a number of focus-related config options: all of them are broken by design in some way.

  • focus highlight+relateds - T2193, T3067 and others
  • item_select_on_focus_disable - unbelievably broken. I genuinely cannot understand how this exists in the tree. It effectively binds focus to select on all list-type widgets, enforcing selection (and selection callbacks!) in all the cases where focus is already wrong
    • also cannot be toggled without totally breaking keyboard navigation of lists: try "list" test and press down a few times after enabling this option
  • first_item_focus_on_first_focus_in - a global change for a single-scenario issue which ends up breaking a practically unlimited number of cases (eg. rELM974c8b05a4c97367b636593f5420ff01e8364aac, which is STILL present in the codebase)

Something needs to be done.

hi,

a short summary what i have done for now:

  • I wrote a little graph which tracked the creation of new focusable widgets and calculates the right left top down widgets. This was not as havy as expected, since there are just a bunch of focusable widgets at a time, (maybe this changes when you are having a 4000x4000 screen full of buttons.
  • The _real_ implementation idea was that there will be a focus interface like:
interface Elm.Focus {
    methods {
        focusable {
            [[return if this object is focusable or not]]
            return : bool;
        }
        focus_calculate {
            [[calculates the focusobjects regarding its direction]]
            params {
                left : Elm.Focus**;
                right : Elm.Focus**;
                top : Elm.Focus**;
                bottom : Elm.Focus**;
                back : Elm.Focus**;
                forth : Elm.Focus**;
                set : list<Elm.Focus**>; [[Set of all focusable objects in the tree of this widget]]
            }
        }
    }
}

Elm.Widget should implement a standard solution where the directions are calcuated based on the dimensions of the focus objects in the set.
So for example list could set there own left right items and use the standard implementation of top/bottom (depending on the horizontal mode).
This means there is a single implementation of standard stuff, so if there is a focus issue its not a problem accress the swalloing of 3 different widgets which are handling focus differently.

Elm.Widget.Item would not be focusable or have any logic for focus, since only realy widgets will need keyboard input and so focus. So there will be a bunch of api useless.

  • options like first_item_focus_on_first_focus_in would be gone for the case of such an implementation, since the container would not handle focus for child-widgets anymore, gengrid/genlist would not even need to be focusable.
  • item_select_on_focus_disable is indeed very borken, this should more be a option on the container, since for something like a fm the selection is not very interesting, the focus could select this without a problem, so you can easily move the selection with the mouse keys. But for your shutdown/suspend usecase from the ml its very very dangerous.
  • Also: focus highlight, I still dont understand when the rect appears, and when it doesnt, but the bigger problem here is that the rect does not stay on the widget and for example "looks out of the scroller" (just open a randon gengrid genlist scroller test and play with it, you will see a bunch of not fitting blue rects).

Okay new sample interfaces:

interface Elm.Focus {
    [[Implemented by objects which may be focusable]]
    methods {
        focusable {
            [[returns true if this object is focusable]]
            return : bool;
        }
    }
}

interface Elm.Focus_Manager {
    [[A focusable Object should register itself to the next available Focus_Manager.

     The next Focus_Manager which is found up in the widget tree is used.

     Container widgets can also implement this interface to set there own right left top bottom objects, they just have to redirect the call up the widget tree so the next focus_manager registers the object]]
    methods {
        register {
            [[register a focusable object to a focusmanager.

              If the given direction objects is $null the manager should try on its own to evaluate this direction.
            ]]
            params {
                object : Elm.Focus*; [[object to add to the graph]]
                right : Elm.Focus*; [[Next focusable object for direction right]]
                left : Elm.Focus*; [[Next focusable object for direction left]]
                top : Elm.Focus*;  [[Next focusable object for direction top]]
                bottom : Elm.Focus*;  [[Next focusable object for direction bottom]]
                back : Elm.Focus*;  [[Next focusable object for tab presses]]
                forth : Elm.Focus*;  [[Next focusable object for forth presses]]
            }
        }
        register_simple {
            [[registers a focusable object to this Focus_Manager without any presetted direction objects.]]
            params {
                object : Elm.Focus*;
            }
        }
    }
}

after a talk with @ami on irc there was a problem with container widgets. fixed now.

conr2d added a subscriber: conr2d.Feb 18 2016, 7:45 AM

Can we use "next" instead of "forth"? I think it's a little more common in that scenario...

abyomi0 added a subscriber: abyomi0.Mar 1 2016, 8:12 AM
stefan_schmidt edited projects, added efl; removed Restricted Project.Jul 20 2016, 7:27 AM
jpeg added a comment.Feb 16 2017, 4:20 AM

@bu5hm4n what's the progress on this?

Basically I failed to review it properly in time, but it is ready to be included in next release in my opinion (it is not perfect, but way better than what we have).

jpeg added a comment.EditedJul 11 2017, 10:27 PM

@bu5hm4n I guess we can close this or at least lower the priority? I know focus is not 100% done yet but a lot has been done already?

@bu5hm4n bush woman! Answer us!

I would like to keep that opened ... Its still not done until the old api is dead ... :).

And i am currently working on killing it ...

YOhoho added a subscriber: YOhoho.Dec 20 2017, 3:03 PM
bu5hm4n lowered the priority of this task from High to TODO.Apr 11 2018, 8:02 AM
bu5hm4n raised the priority of this task from TODO to High.
bu5hm4n added a project: Efl.Ui.Focus.
zmike edited projects, added Restricted Project; removed efl.Jun 11 2018, 6:54 AM
bu5hm4n edited projects, added efl: widgets; removed Restricted Project.Jun 11 2018, 7:07 AM
bu5hm4n edited projects, added Restricted Project; removed efl: widgets, Efl.Ui.Focus.Jun 12 2018, 12:05 AM
bu5hm4n closed this task as Resolved.Dec 17 2018, 12:48 AM

This is done.