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

Make the toolbar buttons usable with keyboard when Page Fit is used #17074

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

calixteman
Copy link
Contributor

No description provided.

Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

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

Sorry, but I think that there's a number of "issues" with this:

  • Does the problem not apply in the same way to spacebar as well?
  • This ignores the shift + Enter case, which I don't think is intended? Please note this part of the code.
  • Last, but definitely not least, this is adding even more special-cases to an already somewhat complex event handler. Can you please see if it's possible to integrate this a little bit better into the existing checks, e.g. by re-factoring the code, to simplify things?

@Snuffleupagus
Copy link
Collaborator

While the updated patch is indeed much simpler, I do wonder if it's not catching way too many cases now?

With this patch if you click on one of the toolbar-buttons you now need to manually click on the viewer (or tab to it) in order for it to receive e.g. any keyboard-scrolling events. This is changing behaviour that's existed for years and it's likely to catch users off guard (and probably lead to bug reports).

web/app.js Outdated Show resolved Hide resolved
@calixteman calixteman merged commit 2453b79 into mozilla:master Oct 5, 2023
3 checks passed
@calixteman calixteman deleted the issue17071 branch October 5, 2023 18:29
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.

Cannot use the keyboard to "click" on the toolbar buttons when Page Fit is used
2 participants