Repository navigation
[SortWidget] bug fix: Fix keyboard hold events targeting wrong item - #14252
Conversation
Commodore64user
changed the title
|
It breaks long-press on a touch device. |
|
I didn't think of that. Two methods with early returns based on touch/non-touch? |
|
Same here @Frenzie, this does leave some devices falling through cracks though, i.e touch devices with keyboards can only use the touch based approach... so not optimal |
| } | ||
| } | ||
| self.ges_events.Hold = { | ||
| self.ges_events.HoldTouch = { |
There was a problem hiding this comment.
Actually, this creates an awkward situation where sometimes this is triggered and the same broken behaviour happens, so yeah, name change is needed.
There was a problem hiding this comment.
I don't think so.
Moreover, onHoldNonTouch should not duplicate the code but call item:onHold.
There was a problem hiding this comment.
well I am telling you, naming it onHold does in fact interfere and break the functionality sometimes.
There was a problem hiding this comment.
break the functionality sometimes
Could you give more details?
There was a problem hiding this comment.
Focus Manager triggers onHold with keys
Could you show the code where it's happening?
There was a problem hiding this comment.
koreader/frontend/ui/widget/focusmanager.lua
Lines 61 to 62 in 00db7ca
There was a problem hiding this comment.
Thanks.
Since in this FocusManager instance you introduce new events bound to certain keys, the correct way would be to disable the default ones provided by FocusManager base class.
To avoid sending 2 different events by one key or key combination.
If I remember correctly, you do this in some other places.
self.key_events[smth] = nil
There was a problem hiding this comment.
But it looks like that we don't do this on regular basis, so I agree with your proposition to rename.
There was a problem hiding this comment.
In that case, like and subscribe ;)
| end | ||
|
|
||
| local item_idx = idx_offset + focused_row | ||
| local item = self.item_table[item_idx] |
There was a problem hiding this comment.
Can this be used instead?
koreader/frontend/ui/widget/focusmanager.lua
Line 431 in 672be80
|
Now it also works for touch+keyboard, correct? |
|
Yes, works everywhere. Touch, non touch, touch with keyboards, all covered. |
…#14314) ### what's new * Added logic in `init` to override FocusManager's `onHold` key event. * Implemented the `CalendarDayView:onHoldNonTouch()` method to handle keyboard-triggered hold events by invoking the `onHold()` method on the currently focused calendar item. ### related issues * fix #14238 * same as #14252
|
7th day... the second coming of koreader is nigh (yeah, I am sprinkling some religious satire as well) |
…koreader#14314) ### what's new * Added logic in `init` to override FocusManager's `onHold` key event. * Implemented the `CalendarDayView:onHoldNonTouch()` method to handle keyboard-triggered hold events by invoking the `onHold()` method on the currently focused calendar item. ### related issues * fix koreader#14238 * same as koreader#14252
3 participants
bug fix
When using shift/screenkb + press on non-touch devices, hold events were always affecting the first item instead of the focused one. Added onHold() handler to SortWidget that directly targets the focused item, bypassing the coordinate-based gesture system that was causing the misdirection.
onHoldmethod toonHoldTouchinSortItemWidget, ensuring the method does not clash with FocusManager's own Hold events.registerKeyEvents()method inSortWidgetto add a newHoldNonTouchevent for keyboard-based hold actions, and unified modifier handling for screen keyboard and physical keyboard inputs.onHoldNonTouchmethod inSortWidgetto handle keyboard-triggered hold events by invoking theonHoldTouchmethod on the currently focused item, enabling consistent hold behaviour for non-touch devices.related issues
This change is