Page MenuHomePhabricator

focus(scroller): backward compatibility for scroller focus move
Closed, ResolvedPublic

Description

reproduction case:
see test code.

current scroller focus movement is different to previous movement without focus manager.

legacy focus behavior:

state in viewport\behaviorpress tab keypress arrow keyon focus
no focusable contentfocus next object outside of scrollermove scroll barfocus scroller itself.
focusable content with next focus objectif the next object is not last leaf of focus tree, focus next object of focusable content. otherwise, focus next object outside of scroller.if the next object is at arrow direction, focus next object of focusable content. otherwise, move scroll bar.focus the top content of focus tree.

currnet focus behavior:

state in viewport\behaviorpress tab keypress arrow keyon focus
no focusable contentfocus next child object of scrollermove object at arrow direction. sometimes scroll bar is moved (i don't know that condition)focus child object of scroller and autoscrolled.
focusable content with next focus objectfocus next child object of scrollermove object at arrow directionfocus the top content of focus tree.
YOhoho created this task.Mar 27 2018, 12:09 AM
YOhoho triaged this task as High priority.

What is meant by on_focus ?

And where are things different to what we have right now ?

YOhoho updated the task description. (Show Details)Apr 1 2018, 7:16 PM
YOhoho added a comment.Apr 1 2018, 7:20 PM

What is meant by on_focus ?

on_focus means 'when scroller is focused'

And where are things different to what we have right now ?

Sorry, I udpated currnet focus behavior.

zmike added a subscriber: zmike.Apr 30 2018, 10:30 AM

It seems that with the new focus system, focus is never applied directly to a scroller and so it is never possible to scroll using the arrows by default. This is probably fine for most cases? I think the only change needed for current behavior is if the scroller has no focusable content then the scroller itself should receive focus as long as it is still focusable. So it would be something like:

  1. focusable scroller receives focus -[has focusable content]> focus goes to content
  2. focusable scroller receives focus -[has no focusable content]> focus goes to scroller
  3. unfocusable scroller cannot receive focus

Currently 2. does not exist I think? There is no test for it, so it's hard to say.

A related thing would be 'focus 4' in elm_test.

With previous focus system, focus would begin...nowhere. Clicking on the scroller background would focus the scroller and then the arrow keys would scroll the scroller. It was impossible to 'escape' the scroller using any combination of keys. Clicking to focus the button would focus the button. It would then be impossible to unfocus the button. Focusing 'the scroller' in this test actually was focusing a layout object in the scroller which had a swallow for the button.

Currently the layout never gets focus, or perhaps just does not trigger scrolling when it is focused an the arrow keys are pressed? Either way there is no way to scroll the scroller, which for this case feels wrong.

zmike edited projects, added Restricted Project; removed efl.Jun 11 2018, 6:50 AM
bu5hm4n edited projects, added efl: widgets; removed Restricted Project.Jun 11 2018, 7:04 AM
bu5hm4n edited projects, added Restricted Project; removed efl: widgets, Efl.Ui.Focus.Jun 12 2018, 12:05 AM
bu5hm4n moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Jun 12 2018, 2:49 AM

After this patch you can (after resizing) move focus outside the application.

I tested with the following application:

https://git.enlightenment.org/devs/bu5hm4n/evas_in_elm.git/

The reason for the resize is that elm_scroller.c makes some weird things that result in "return EINA_TRUE;" which is obviously a bug, since the keystroke is definitly NOT taken.

Can someone else also check this and tell if this is resolved or not ?

YOhoho added a comment.EditedJul 12 2018, 10:56 AM

Hello. :)

Thank you to make nice patch.
D6578 seems to fix 2. focusable scroller receives focus -[has no focusable content]> focus goes to scroller

Unfortunately, I bring annoying tests for 1. focusable scroller receives focus -[has focusable content]> focus goes to content

If there is a focusable content which is focused in the scroller, I can't reach to whole content of the scroller
https://github.com/dasg34/focus_test/blob/master/test_scroller.c

If there are focusable contents in the scroller and a content is focused, I can't get information of no focusable content between the focusable contents.
https://github.com/dasg34/focus_test/blob/master/test_scroller2.c

Okay, but this does not sound to me like a focus issue in first place:

  1. The btn in the scroller is focused,
  2. You press down

What happens now is that the Scroller does not take this keystroke as moving down, but rather doing nothing (i dont know why tbh). As a result of that, the keystroke is propagated to the window, which emits the focus movement.

What should happen however, is that the scroller should start to scroll down with a keystroke, until the mex / min position in this given direction is reached...

Thats why i would say that this is rather something elm_scroller should handle in the keyhandling code, and is not related directly to focus handling of the scroller.

YOhoho added a comment.EditedJul 12 2018, 11:32 AM

Thats why i would say that this is rather something elm_scroller should handle in the keyhandling code, and is not related directly to focus handling of the scroller.

Yes, That is what i want to say.but you removed the keyhandling code in elm_scroller. (073c72acce552245eb1131331985484ee2bfd489) after this patch, scroller movement backward compatibility is broken.

As far as i can see that code is just transformed, not removed, the code that got removed there is just handling moving the actaul focus not moving the scrollable content. However, there can be a additional return that should not be there, I will check. But thx for sharing the revision!

bu5hm4n removed bu5hm4n as the assignee of this task.Aug 16 2018, 12:27 AM
bu5hm4n added a subscriber: bu5hm4n.
YOhoho assigned this task to eagleeye.Aug 16 2018, 1:36 AM
bu5hm4n moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Mon, Nov 26, 11:44 AM

With this added there is only one questionable case, where you have focusable content, but that is outside of the viewport, then the scroller is not directly accessable. I want to do other things before fixing this particular case, however, i think this case is not happening that often in practice ?

YOhoho closed this task as Resolved.Sun, Dec 16, 4:10 PM