RichText: add non breaking space shortcut on Windows #43150
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
requested review from
ellatrix,
fluiddot,
noisysocks,
draganescu,
talldan,
tellthemachines,
adamziel and
kevin940726
as code owners
t-hamano
changed the title
|
Size Change: +173 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
t-hamano
added
[Package] Rich text
t-hamano
changed the title
ellatrix
reviewed
Aug 11, 2022
ellatrix
reviewed
Aug 11, 2022
ellatrix
reviewed
Aug 11, 2022
ellatrix
reviewed
Aug 11, 2022
| } | ||
|
|
||
| // Shift + Ctrl + Space shortcut is built in on the Mac. | ||
| if ( isAppleOS() ) { |
There was a problem hiding this comment.
Does it hurt to override the default?
There was a problem hiding this comment.
I deleted it, but I don't have a Mac, could you test it?
ellatrix
reviewed
Aug 11, 2022
|
|
||
| return ( | ||
| <RichTextShortcut | ||
| type="primaryShift" |
There was a problem hiding this comment.
Btw, the original issue had a typo. It should be Shift+Alt+Space. But apparently Alt+Space also works so we could simplify it to just Alt+Space?
There was a problem hiding this comment.
As shown in this source, Alt+Space is often used to access the menu of the program itself.
For example, in Chrome, it would appear as follows (this was also the case in FireFox and Edge):
Shift+Alt+Space also showed the same behavior.
If we override this shortcut, we will not be able to access the browser menu unless we remove focus from RichText.
I recommend Ctrl+Shift+Space, which is a continuation of the Microsoft Word shortcut.
There was a problem hiding this comment.
Shift+Alt+Space has the same behaviour? I think it's fine to override the Shift combination then.
I think we should stick to an Alt combination because Alt indicates an alternative for the current key/character. I think this is the same for Windows? Alt combinations input different characters, whereas Ctrl is more for commands/controls.
There was a problem hiding this comment.
I think we should stick to an Alt combination because Alt indicates an alternative for the current key/character. I think this is the same for Windows? Alt combinations input different characters, whereas Ctrl is more for commands/controls.
Yes, that is correct.
But unfortunately, Alt+Space (Alt+Shift+Space) didn't respond to triggering RichtextShortcut, and the browser menu opens.
From my research here, it appears that Alt + Space is not a browser shortcut, but is implemented at the OS level.
Alt + Spacebar | Open the shortcut menu for the active window.
Then, possible patterns and possibilities are as follows.
- ✅
Ctrl + Shift + Space: Confirmed to work correctly - ✅
Ctrl + Space: Confirmed to work correctly - ❗
Shift + Space: Switch character width between full-width and half-width. (OS shortcut) - ❗
Ctrl + Space: Turn the Chinese input method editor (IME) on or off. (OS shortcut)
ellatrix
reviewed
Aug 12, 2022
package-lock.json
Outdated
| @@ -17435,6 +17435,7 @@ | |||
| "@wordpress/html-entities": "file:packages/html-entities", | |||
| "@wordpress/i18n": "file:packages/i18n", | |||
| "@wordpress/icons": "file:packages/icons", | |||
| "@wordpress/keycodes": "file:packages/keycodes", | |||
|
Ignore this message. Testing if this subscribes me in the mentioned notifications. @ellatrix |
ellatrix
reviewed
Mar 1, 2024
| @@ -46,4 +46,8 @@ export const textFormattingShortcuts = [ | |||
| 'Convert the current paragraph or heading to a heading of level 1 to 6.' | |||
| ), | |||
| }, | |||
| { | |||
| keyCombination: { modifier: 'primaryShift', character: 'SPACE' }, | |||
There was a problem hiding this comment.
Can we use a literal space here as you did below for the rich text shortcut?
There was a problem hiding this comment.
Do you mean to use \u00a0 instead of SPACE? In that case, the shortcut modal will display as below.
There was a problem hiding this comment.
Ah, this is for displaying the shortcut? Sorry it wasn't clear from the code.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @rezeau, @pierrefrance, @burnuser, @zackkatz. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
ellatrix
reviewed
Mar 1, 2024
| name, | ||
| title, | ||
| tagName: 'span', | ||
| className: 'nbsp', |
There was a problem hiding this comment.
This could cause existing span tags with this class to be wrongly processed as this format. I wonder if we could set the tagName to nbsp to work around the rules. 🤔
There was a problem hiding this comment.
Even if I changed the tagName to nbsp, it worked fine! Does tagName not have to be an element that is allowed as HTML?
There was a problem hiding this comment.
Not if we're not going to use it 😄
carstingaxion
pushed a commit
to carstingaxion/gutenberg
that referenced
this pull request
Mar 27, 2024
Unlinked contributors: rezeau, pierrefrance, burnuser, zackkatz. Co-authored-by: t-hamano <[email protected]> Co-authored-by: ellatrix <[email protected]> Co-authored-by: yankiara <[email protected]> Co-authored-by: tybor <[email protected]> Co-authored-by: fr-laurentn <[email protected]> Co-authored-by: TonyGravagno <[email protected]> Co-authored-by: noisysocks <[email protected]> Co-authored-by: benjaminpwarren <[email protected]>
This was referenced Apr 10, 2024
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.
Fix: #18523
What?
This PR adds non-breaking space shortcut (Ctrl + Shift Space) for Windows.
Why?
As seen in the source here, this shortcut is well known in Word and its compatibility software and should be introduced in Gutenberg as well.
How?
I added a format type (
core/non-breaking-space) and appliedRichTextShortcutin it. This shortcut is built-in on Mac and valid only on Windows.However, this shortcut is not for formatting. It might not be necessary to add a format type, but
RichTextShortcutis used in only format types or blocks, and I couldn't find a way to implement it directly into RichText.At the same time, I added it to the keyboard shortcut modal. I think this should be no problem to display without limiting the OS.
Testing Instructions
Press
Ctrl+Shift+Spacein rich text.This behavior follows the behavior of Microsoft Word.
Screenshots or screencast
bdad4fc264ebf129bf30d4e8f42f849e.mp4