Distraction free mode: Fix keyboard shortcut not working #47900
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
t-hamano
added
[Package] Keyboard Shortcuts
|
Size Change: +23 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
t-hamano
force-pushed
the
fix/distraction-free-keyboard-shortcut
branch
from
fd78509 to
2ba70ae
Compare
t-hamano
changed the title
t-hamano
commented
Feb 10, 2023
| const replacementWithShiftKeyMap = { | ||
| Comma: ',', | ||
| Backslash: '\\', | ||
| // Windows returns `\` for both IntlRo and IntlYen. |
There was a problem hiding this comment.
This comment was taken from the source code of chromium
|
Thank you for the review, @alexstine! I will wait for someone to test it on Mac (because I don't have Mac...). |
|
@t-hamano I have Mac as well, what is the keyboard shortcut? Spell it out if you do not mind, I cannot figure out what the icons mean in the PR description. |
|
Thank you for the test, @alexstine. The shortcut on the Mac would be ‘shift‘ + |
|
@t-hamano Yes, this issue does occur on Mac for trunk. It also is not impacted by your PR. Pressing cmd+shift+\ results in opening the new tabs in Safari. Seems like event is not prevented or the shortcut is not getting interpreted correctly. |
|
Thanks for the test, @alexstine! I have removed Shortcut key is |
ntsekouras
approved these changes
Feb 13, 2023
There was a problem hiding this comment.
Shortcut key is shift + command + , (Comma). It would be great if you could also test if this shortcut works as well on Macs as it has in the past 🙏
Tested and works before. Thanks @t-hamano !
ntsekouras
pushed a commit
that referenced
this pull request
Feb 13, 2023
* Distraction free mode: Fix keyboard shortcut not working on Windows * Include mac
|
Cherry-picked this PR to the wp/6.2 branch. |
ntsekouras
removed
the
Backport to WP 6.7 Beta/RC
DaisyOlsen
added
the
[Type] Bug
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: #47873
Related to: #43428
What?
This PR fixes a problem where the shortcut for switching distraction flow mode did not work (probably Windows only).
Why?
The shortcut for switching the distraction free mode is as follows:
Ctrl+Shift+\⇧+⌘+\The
event.keyfor determining shortcuts has a value that takes into account modifier keys such as theshiftkey. Therefore, as I have investigated in this comment, I believe the cause is that when theshiftkey is combined with other keys,event.keyhas a value that is not originally expected. Furthermore, the character changed by theshiftkey depends on the keyboard layout, soevent.keycannot be used as the source of the decision.How?
Refers to the value of
event.codethat represents a physical key on the keyboard. Based on that value, it is converted to the originally expectedevent.key.This PR fixes the bug in Windows. However, as mentioned in the two comments from here, the test results seem to be different for Mac. I would appreciate it if you could test to see if this shortcut issue is currently occurring on Mac.
Testing Instructions for Keyboard
Screenshots or screencast
English Keyboard Layout
event.codeisBackslash.en_keyboard.mp4
Japanese Keyboard Layout
event.codeisIntlRoorIntlYen.jp_keyboard.mp4