Skip to content
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

Keyboard popup propagates #5555

Closed
Frenzie opened this issue Oct 31, 2019 · 7 comments · Fixed by #5573
Closed

Keyboard popup propagates #5555

Frenzie opened this issue Oct 31, 2019 · 7 comments · Fixed by #5573
Labels

Comments

@Frenzie
Copy link
Member

Frenzie commented Oct 31, 2019

  • KOReader version: 602a821
  • Device: all

Issue

The keyboard popup propagates the event. I'm not sure which change is responsible atm but #5380 is probably the most suspicious at a glance.

https://github.com/koreader/koreader/commits/master/frontend/ui/widget/virtualkeyboard.lua

Steps to reproduce

Hold on a key and let go.

@Frenzie Frenzie added the bug label Oct 31, 2019
@poire-z
Copy link
Contributor

poire-z commented Nov 4, 2019

Possibly related:

  • Edit some text file in text editor, go at end of some line:
  • Long press on Backspace, to delete the line
  • When releasing that long press: the last char of the previous line is also deleted

@Frenzie
Copy link
Member Author

Frenzie commented Nov 4, 2019

Indeed, that sounds like the same issue.

#5380 isn't to blame, but the issue didn't exist in 2019.09 while it does in 2019.10. I've traced the undesired behavior to #5452.

@Frenzie
Copy link
Member Author

Frenzie commented Nov 4, 2019

It seems to me that #5452 breaks the way modal widgets are supposed to work. The top modal widget is supposed to be the top modal widget.

@Frenzie
Copy link
Member Author

Frenzie commented Nov 4, 2019

PS I forgot to add this above, but this would be one way to get rid of the biggest problem. The thing is, if you hover over some random key outside of the popup it will still activate. Complex workarounds would be possible, but shouldn't be necessary because we've got the top modal dialog in play here.

diff --git a/frontend/ui/widget/virtualkeyboard.lua b/frontend/ui/widget/virtualkeyboard.lua
index 3a7d0992..2e48321c 100644
--- a/frontend/ui/widget/virtualkeyboard.lua
+++ b/frontend/ui/widget/virtualkeyboard.lua
@@ -377,6 +377,7 @@ function VirtualKeyPopup:init()
                 virtual_key.onHoldReleaseKey = function()
                     virtual_key:onTapSelect(true)
                     UIManager:close(self)
+                    return true
                 end
                 virtual_key.onPanReleaseKey = virtual_key.onHoldReleaseKey

@poire-z
Copy link
Contributor

poire-z commented Nov 6, 2019

Actually, with the current code and my above issue:

When releasing that long press: the last char of the previous line is also deleted

There were 3 stuff done: delete to start of line when long press. And when releasing: one delete that removed the CR, and another delete that removed the last char of previous line.

Looks like we may have forgotten some return true in some evenr handlers:

function VirtualKey:onHoldReleaseKey()
Device:performHapticFeedback("LONG_PRESS")
if self.keyboard.ignore_first_hold_release then
self.keyboard.ignore_first_hold_release = false
return true
end
self:onTapSelect()
end
function VirtualKey:onPanReleaseKey()
Device:performHapticFeedback("LONG_PRESS")
if self.keyboard.ignore_first_hold_release then
self.keyboard.ignore_first_hold_release = false
return true
end
self:onTapSelect()
end

If I add that return true, one of the 2 delChar is avoided:

11/06/19-18:33:29 DEBUG hold gesture detected in slot 0
11/06/19-18:33:29 DEBUG in hold state...
11/06/19-18:33:29 DEBUG adjusted ges: hold
11/06/19-18:33:29 WARN  backspace hold callback called
11/06/19-18:33:29 DEBUG delete to start of line
[...]
11/06/19-18:33:32 DEBUG in hold state...
11/06/19-18:33:32 DEBUG hold_release detected in slot 0
11/06/19-18:33:32 DEBUG adjusted ges: hold_release
11/06/19-18:33:32 WARN  backspace callback called
11/06/19-18:33:32 DEBUG delete char

If I remove the self:onTapSelect() at the end of VirtualKey:onHoldReleaseKey(), the other delChar is avoided.
I guess these self:onTapSelect() are here to only cause the tap on the key (and the output of the corresponding char) only when we release the key.
Might be we should just avoid calling it when a real hold action has been done before, with an added flag like self.no_tap_on_hold_release (we already have that kind of trick around, might be not nice, but still sound logical).

It seems to me that #5452 breaks the way modal widgets are supposed to work.

Yes, everything works as before when we revert #5452. But it doesn't feel like it needs reverting (and it's quite obscure understanding why it break/solves the issue :). The above findings like some more logical fixes to that issue.

@Frenzie
Copy link
Member Author

Frenzie commented Nov 6, 2019

#5452 breaks modal dialogs and should be reverted. The top-most modal dialog should always take precedence.

The problem is that for whatever reason the Evernote plugin apparently doesn't work correctly. It should quite simply behave like the Update check.

@Frenzie
Copy link
Member Author

Frenzie commented Nov 6, 2019

Fixed by #5574.

@Frenzie Frenzie closed this as completed Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants