New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FocusManager] More intuitive key naviguation #3774

Merged
merged 6 commits into from Mar 21, 2018

Conversation

Projects
None yet
3 participants
@onde2rock
Contributor

onde2rock commented Mar 18, 2018

The focusmanager now find the closest widget on the right or left on
inner horizontal border.
See : #3765 (comment)

It even make the touchmenu better by allowing to go down from every item of the top menu.

onde2rock added some commits Mar 18, 2018

[FocusManager] More intuitive key naviguation
The focusmanager now find the closest widget on the right or left on
inner horizontal border.
@Frenzie

This comment has been minimized.

Member

Frenzie commented Mar 18, 2018

It would be neat if you could write a unit test for that to prevent accidental breaking. This kind of thing is generally unlikely to come up in regular use. :-)

@onde2rock

This comment has been minimized.

Contributor

onde2rock commented Mar 18, 2018

Ok, I will look into it.

@poire-z

This comment has been minimized.

Contributor

poire-z commented Mar 18, 2018

Working really fine.
Dunno if you missed that or if it's a limitation:
3cols > 2cols > 1 cols work fine when you go down.
Less fine if you go up: you stay on buttons that belong to lines with 3 columns.
With hold on file:
down: Purge sdr > Rename > Book information > View book description
up: Book information > Rename > Purge sdr > Book information

@onde2rock

This comment has been minimized.

Contributor

onde2rock commented Mar 20, 2018

Yes, the navigation is not "symmetrical" now. In fact the fix I implemented is not very complicated, it just drops you at the end of the next line if the item below doesn't exist.

To do what I understand you suggest, Focusmanager need to somehow "remember" the previous position in order to make a decision on where to go back. And I have no idea if it can be done without overcomplicating everything.
I figure that just for this menu, it's enough and still useable.

@poire-z

This comment has been minimized.

Contributor

poire-z commented Mar 20, 2018

No, I wasn't talking about this needs to remember thing (I see what you mean, no problem for me with that).

the fix I implemented is not very complicated, it just drops you at the end of the next line if the item below doesn't exist.

But does it drops you at the end of previous line if the item above doesn't exist ? :) It doesn't for me, you just get stuck on current line (you need to move left to be able to go above). Again, not much and not a blocker (but may be the fix is as easy as the one you did for going down :)

@onde2rock

This comment has been minimized.

Contributor

onde2rock commented Mar 20, 2018

But does it drops you at the end of previous line if the item above doesn't exist ?

Hum I think it do. On the hold-menu, if I go down then up :
2018-03-20-212418_540x720_scrot

In fact, in the code, the same logic apply if you try to go up or down

@poire-z

This comment has been minimized.

Contributor

poire-z commented Mar 20, 2018

Yes, a picture is better :)

zzz

up: Book information > Rename > Purge sdr > Book information
I would expect it, when moving up from Purge sdr, I would be on Refresh cached book info, the reverse of your blue movement.

edit: I wrote before you just get stuck on current line , which was wrong: you're not stuck, but you're just cycling on the lines that have that same number of columns - which does not happen when you move down

@Frenzie

This comment has been minimized.

Member

Frenzie commented Mar 20, 2018

Btw, I don't know what we're using to draw things but in Gimp and Photoshop you can draw straight lines by holding shift. :-P (Probably also in Paint.NET.)

bla

@onde2rock

This comment has been minimized.

Contributor

onde2rock commented Mar 20, 2018

Oh, you mean the vertical wrapAround is weird, yes totally. Before it didn't wrap in "the middle" of the menu, and that was a bug, and since this commit fix this bug, it's weird.

I will try to change so it wrap, not to the last line containing enought colum, but just to the last line.

@Frenzie Thanks for the laugh :-)
37681770-e4c9b6b6-2c87-11e8-8ea0-c4e9fb8c5ffe

onde2rock added some commits Mar 20, 2018

@poire-z

This comment has been minimized.

Contributor

poire-z commented Mar 20, 2018

Wrapping fine in both directions now :) thanks!

@onde2rock

This comment has been minimized.

Contributor

onde2rock commented Mar 21, 2018

@Frenzie Ok, I tried to make some test, can you tell me if it's enough ?

focusmanager.layout = layout
focusmanager.selected = {y = 1,x = 1}
Right(focusmanager)
assert.are.same(focusmanager.selected, {y = 1,x = 2})

This comment has been minimized.

@Frenzie

Frenzie Mar 21, 2018

Member

It's expected, object so you'll have to switch them all around. It doesn't really affect test failure but it'll give confusing error messages otherwise.

@Frenzie

This comment has been minimized.

Member

Frenzie commented Mar 21, 2018

@onde2rock Nice test! We might also want to test it on some real widgets (such as ButtonDialog and TouchMenu) but for this PR it's excellent besides the one comment. :-)

@Frenzie Frenzie merged commit 1b91470 into koreader:master Mar 21, 2018

1 check passed

ci/circleci Your tests passed on CircleCI!
Details
@Frenzie

This comment has been minimized.

Member

Frenzie commented Mar 21, 2018

Thanks!

@onde2rock onde2rock deleted the onde2rock:fix-focusmanager-naviguation branch Mar 21, 2018

Frenzie added a commit to Frenzie/koreader-base that referenced this pull request Mar 30, 2018

[feat] SDL2: preliminary gamepad support
This is a "dumb" implementation that spits out fake keyboard events.

* Left trigger & d-pad: arrow keys
* Bumpers and right trigger: page up/down
* Menu button: menu
* A: enter
* B: back

This is sufficient to use most of the program.

Made possible by @onde2rock's recent efforts in koreader/koreader#3796
koreader/koreader#3785 koreader/koreader#3774
koreader/koreader#3765 and koreader/koreader#3745

Frenzie added a commit to koreader/koreader-base that referenced this pull request Mar 31, 2018

[feat] SDL2: preliminary gamepad support (#628)
This is a "dumb" implementation that spits out fake keyboard events.

* Left stick & d-pad: arrow keys
* Bumpers and right stick: page up/down
* Menu button: menu
* A: enter
* B: back

This is sufficient to use most of the program.

Made possible by @onde2rock's recent efforts in koreader/koreader#3796
koreader/koreader#3785 koreader/koreader#3774
koreader/koreader#3765 and koreader/koreader#3745

Frenzie added a commit to Frenzie/koreader that referenced this pull request Mar 31, 2018

[feat, UX] bump base for SDL2: preliminary gamepad support
* [feat] SDL2: preliminary gamepad support koreader/koreader-base#628

This is a "dumb" implementation that spits out fake keyboard events.

* Left stick & d-pad: arrow keys
* Bumpers and right stick: page up/down
* Menu button: menu
* A: enter
* B: back

This is sufficient to use most of the program.

Made possible by @onde2rock's recent efforts in koreader#3796
koreader#3785 koreader#3774
koreader#3765 and koreader#3745

Frenzie added a commit that referenced this pull request Mar 31, 2018

[feat, UX] bump base for SDL2: preliminary gamepad support (#3819)
* [feat] SDL2: preliminary gamepad support koreader/koreader-base#628

This is a "dumb" implementation that spits out fake keyboard events.

* Left stick & d-pad: arrow keys
* Bumpers and right stick: page up/down
* Menu button: menu
* A: enter
* B: back

This is sufficient to use most of the program.

Made possible by @onde2rock's recent efforts in #3796
#3785 #3774
#3765 and #3745
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment