Skip to content

QuickMenu: anchor to gesture position 2#10641

Closed
hius07 wants to merge 4 commits into
koreader:masterfrom
hius07:anchor-quickmenu
Closed

QuickMenu: anchor to gesture position 2#10641
hius07 wants to merge 4 commits into
koreader:masterfrom
hius07:anchor-quickmenu

Conversation

@hius07

@hius07 hius07 commented Jul 4, 2023

Copy link
Copy Markdown
Member

Follow up to #10636.

(1) Enable anchoring for profiles
(2) Anchoring is optional
(3) QuickMenu width is adjustable, not less than 0.5 of the screen width.

In Gestures manager:

1

In Profiles:

2


This change is Reviewable

@poire-z

poire-z commented Jul 4, 2023

Copy link
Copy Markdown
Contributor

and maybe will make it optional for profiles, I used to the centered QMs

Feels a bit quick to go with yet another setting. I would have liked some discussion first :) and/or you try to live with it for a while before going the hard way of adding a superfluous-feeling setting.

  • May be there are some gestures that you use where anchoring is not welcome and best not enforced for all users ? Or may be we could find out some rules/logic (gesture type, position) where anchoring should be done or not.
  • Also, it looks like it's per-profile ? Seems fastidious. It feels it would make more sense per gesture (but again, not familiar with their interactions, and I see it on both your screenshots), or a global setting.
  • You enabled it for profile because it was not, dunno for what it was enabled by the previous PR - are there stuff where it is not enabled ?

Anyway, no hard objection ;)

@hius07

hius07 commented Jul 4, 2023

Copy link
Copy Markdown
Member Author

(1) Actually #10636 was quick, got jumping QMs without discussion.
(2) The setting is per gesture and per profile.
If we assign a gesture to call a profile which is a QM, and we want this profile's QM to be anchored, the gesture should be set not anchored, and the profile should be set anchored.
(3) The above case wasn't enabled.

@poire-z

poire-z commented Jul 4, 2023

Copy link
Copy Markdown
Contributor

The setting is per gesture and per profile.
If we assign a gesture to call a profile which is a QM, and we want this profile's QM to be anchored, the gesture should be set not (really?) anchored, and the profile should be set anchored.

Again, not familiar with all this, but the above seems complicated (and odd). Possibly implementation requiring all this, but may be this should not face the user.

Actually #10636 was quick, got jumping QMs without discussion.

I know - so let's discuss it here :) Pinging @jonnyl2 @yparitcher .
What gestures are you assigning a QM with?
(I guess I would not welcome anchoring with any multiswipe - spanning some distance, going back, what to use, the start or end, or some middle point...)

@hius07

hius07 commented Jul 4, 2023

Copy link
Copy Markdown
Member Author

but the above seems complicated (and odd)

I do not think so, and our User Guide describes both ways of getting QuickMenus: (1) as a gesture with multiple actions and (2) as a profile. I prefer (2).

In that case the gesture has one action exec_profile, it is not QuickMenu and is not anchored.

@hius07

hius07 commented Jul 4, 2023

Copy link
Copy Markdown
Member Author

Also asking QM heavy users @mergen3107 and @offset-torque.

@Frenzie

Frenzie commented Jul 4, 2023

Copy link
Copy Markdown
Member

I do not think so, and our User Guide describes both ways of getting QuickMenus: (1) as a gesture with multiple actions and (2) as a profile. I prefer (2).

I think the complexity is referring to having a special setting just about its location?

I'd think all you need is:

  • Gesture-based → open at gesture location
  • Profile → open at center of screen

@jonnyl2

jonnyl2 commented Jul 4, 2023

Copy link
Copy Markdown
Contributor

What gestures are you assigning a QM with?
(I guess I would not welcome anchoring with any multiswipe - spanning some distance, going back, what to use, the start or end, or some middle point...)

All corner long-presses and the following multiswipes: N-E-W and SE-NE. For the corners, anchoring in the respective corners is welcomed. For the multi-swipes, the most natural and convenient anchor would be a little above the end point of the last leg of the gesture (where I lift the finger) such that the finger is positioned near the left-hand side of the first option of the QM.

(For this reason I also suggest to make the QM as narrow as possible, because if the gesture is effected near the edge of the screen, a wider width will cause the QM to open further away from the gesture. I think of it kind of like a right-click context menu that opens right by the mouse cursor.)

@offset-torque

Copy link
Copy Markdown

For my use case current implementation is perfect.
I made this diagram to explain my perspective better:
quickmenu

  • I open QM with "short swipe" of my thumb (red line).
  • Because QM is very wide I can press all the items with my thumb (green region) without moving my hand.
  • I am using Libra (asymmetrical edge) and I can still reach these 10 items over the wide edge.

Now I have a question. According to the new system what will be the position of my QM ?
I guess it will shift down considering the starting point of my gesture. Is this correct ?

@hius07

hius07 commented Jul 4, 2023

Copy link
Copy Markdown
Member Author

I guess it will shift down considering the starting point of my gesture. Is this correct ?

No, the @poire-z 's logic is trickier

-- We prefer displaying above the anchor if there is room (so it looks like popping up)
-- except if anchor() returned prefers_pop_down

Depends on the size of the QM.
We may think of setting prefers_pop_down to make it more predictable.

@hius07

hius07 commented Jul 4, 2023

Copy link
Copy Markdown
Member Author

Anyway, with the current PR you will be able to switch the anchoring off.

@mergen3107

Copy link
Copy Markdown
Contributor
  • Gesture-based → open at gesture location
  • Profile → open at center of screen

I agree with @Frenzie , however I only use gesture-based QM, not profile-based.

I also don’t know the intentions of original FR as to why anchoring is useful.

@jonnyl2

jonnyl2 commented Jul 4, 2023

Copy link
Copy Markdown
Contributor

I tested the new files and for me it works very well. (I didn't test QM as Profile.)

What does Always active do?

@offset-torque I tested your set-up and I think it would work very conveniently for you with the anchoring active; however, you would have to use a quick diagonal swipe in North-East direction. If you do South-West (as in your diagram), the QuickMenu will open above the swipe line and the options will be harder to reach.

@hius07

hius07 commented Jul 4, 2023

Copy link
Copy Markdown
Member Author

What does Always active do?

The gesture is available when the touch input is off.

The position of a QM doesn't depend on the direction of a gesture.

@jonnyl2

jonnyl2 commented Jul 4, 2023

Copy link
Copy Markdown
Contributor

The position of a QM doesn't depend on the direction of a gesture.

It depends on where one initiates the gesture. This is what I meant:

Edited_20230704_173340.mp4

@offset-torque

Copy link
Copy Markdown

Here is an idea to prevent that menu popping up randomly:
snap positions
Dividing screen into 9 equal (or non-equal) zones like this and snapping menu to the closest region according to gesture starting point.

Because stable position is a benefit for a menu. For example I can now reflexively press the buttons on my QM without searching the relevant button because QM opens at the same position every time. So at least maybe we can anchor it to predictable positions like in the illustration (4 corner+4 edge+center). I think this will cover every possible holding position.

@NiLuJe

NiLuJe commented Jul 4, 2023

Copy link
Copy Markdown
Member

That feels weirder and more complex for no good reason; either anchor it or don't anything in-between would feel super weird.

@offset-torque

Copy link
Copy Markdown

That feels weirder and more complex for no good reason; either anchor it or don't anything in-between would feel super weird.

If you want to see something super weird have a look at 5 second mark of @jonnyl2's last video. He swiped lower half of the screen -> Menu opened at the top half of the screen. Zero overlap between finger position and menu.

Overall, I am ok as long as I can have a fixed position menu. I don't know how many QM users are there but most of them got used to the centered menu and probably won't want to change too, especially if they have a small screen. After all my Libra screen is the size of my hand (even smaller) so moving this menu around won't benefit me much. This might be more useful if I upgrade to a 10" reader.

But the proposal looks nicer and feels more like a right-click menu so it is aesthetically more pleasing that is for sure.

@yparitcher yparitcher left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hius07
Sorry for the rush, i thought everyone had chimed in.

Passing the gesture to the profile is a good idea. However as others have said if we add a setting, only the gesture itself should need the setting, not the profiles afterwards.

As discussed in the other pr, if we want to use the end pos, gesturedetector sould be extended to pass current_tev as endpos

Comment thread frontend/dispatcher.lua
if settingsList[k].category == "none" then
if settingsList[k].arg ~= nil then
UIManager:sendEvent(Event:new(settingsList[k].event, settingsList[k].arg))
UIManager:sendEvent(Event:new(settingsList[k].event, settingsList[k].arg, gesture))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check that this doesnt break any of the other handlers

Comment thread frontend/dispatcher.lua
function Dispatcher:execute(settings, gesture)
if settings.settings ~= nil and settings.settings.show_as_quickmenu == true then
return Dispatcher:_showAsMenu(settings, gesture)
return Dispatcher:_showAsMenu(settings, settings.settings.anchor_quickmenu and gesture)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe do the check in _showAsMenu to keep the details out of execute?

@jonnyl2

jonnyl2 commented Jul 4, 2023

Copy link
Copy Markdown
Contributor

Overall, I am ok as long as I can have a fixed position menu. I don't know how many QM users are there but most of them got used to the centered menu and probably won't want to change too, especially if they have a small screen.

Perhaps it would be best to use 2 different QM width settings:

  1. Wide (as in the current release) when centered
  2. Narrow and adaptive when anchored

Then nothing will change for users who will opt not to use the anchor feature.

@NiLuJe

NiLuJe commented Jul 4, 2023

Copy link
Copy Markdown
Member

gesturedetector sould be extended to pass current_tev as endpos

FWIW, I was bored last night, so I have something in the fire, I just got sidetracked because I unearthed a bit of a behavior difference between single and two-finger pans/swipes in the process, and I'm pondering on how best to deal with that ;).

The upside is that basically everything already uses the release position except for swipes, so the actually relevant and non-OCD-related stuff is fairly trivial ;).

@hius07

hius07 commented Jul 4, 2023

Copy link
Copy Markdown
Member Author

Passing the gesture to the profile is a good idea. However as others have said if we add a setting, only the gesture itself should need the setting, not the profiles afterwards.

Why so?
I have a profile with certain actions. I want this profile to be shown as QuickMenu on tap on the bottom right corner. And I want this QM to be anchored. How should I proceed?

@offset-torque

Copy link
Copy Markdown

jonnyl2 Perhaps it would be best to use 2 different QM width settings

Sounds good to me. Current wide menu is ugly and covers the book, but as a side effect it is accessible from both sides.

Maybe we can have:

  • A static QM, which is the current logic, fat and centered. Makes sense on small/low-res screens and especially side holdable models like Libra and Oasis.
  • And a dynamic QM which is slender and opens at your finger. Better on bigger screens and bottom holdable models.

@yparitcher

Copy link
Copy Markdown
Member

Passing the gesture to the profile is a good idea. However as others have said if we add a setting, only the gesture itself should need the setting, not the profiles afterwards.

Why so?
I have a profile with certain actions. I want this profile to be shown as QuickMenu on tap on the bottom right corner. And I want this QM to be anchored. How should I proceed?

The tap bottom corner should have anchored set, and it should pass through automatically to the profile, the profile should be ignorant of how it was called, the user should only need to set the setting on the gesture and it should pass through to nested profiles, and only checked by the qm popup.

@hius07

hius07 commented Jul 5, 2023

Copy link
Copy Markdown
Member Author

So strange, the second today commit (to Dispatcher) is in my branch but doesn't appear here.

@Frenzie Frenzie added this to the 2023.06.1 milestone Jul 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants