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.

Sign up for GitHub

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

RichText: add non breaking space shortcut on Windows #43150

Merged
merged 11 commits into from
Mar 20, 2024
Merged

RichText: add non breaking space shortcut on Windows #43150

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
2b459da
RichText: add non breaking space shortcut on Windows
t-hamano Aug 11, 2022
79e8afd
Update snapshot
t-hamano Aug 11, 2022
0d9d4ee
Fix local changes
t-hamano Aug 11, 2022
2697e83
Change function name
t-hamano Aug 12, 2022
824462a
Use unicode character
t-hamano Aug 12, 2022
a7c1a47
Override built-in shortcut on Mac OS
t-hamano Aug 12, 2022
1075635
Merge branch 'trunk' into add/richtext-shortcut-nbsp
t-hamano Dec 28, 2023
c16b857
Revert unintended changes
t-hamano Dec 28, 2023
bab769d
Revert unintended changes
t-hamano Dec 28, 2023
dd6cca7
Merge branch 'trunk' into add/richtext-shortcut-nbsp
t-hamano Mar 2, 2024
758ceb6
use nbsp as tagName
t-hamano Mar 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/customize-widgets/src/components/keyboard-shortcut-help-modal/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
description: __( 'Add non breaking space.' ),
},
];
4 changes: 4 additions & 0 deletions packages/edit-post/src/components/keyboard-shortcut-help-modal/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
description: __( 'Add non breaking space.' ),
},
];
4 changes: 4 additions & 0 deletions packages/edit-site/src/components/keyboard-shortcut-help-modal/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
Copy link
Member

Choose a reason for hiding this comment

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

Can we use a literal space here as you did below for the rich text shortcut?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean to use \u00a0 instead of SPACE? In that case, the shortcut modal will display as below.

image

Copy link
Member

Choose a reason for hiding this comment

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

Ah, this is for displaying the shortcut? Sorry it wasn't clear from the code.

description: __( 'Add non breaking space.' ),
},
];
4 changes: 4 additions & 0 deletions packages/edit-widgets/src/components/keyboard-shortcut-help-modal/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
description: __( 'Add non breaking space.' ),
},
];
2 changes: 2 additions & 0 deletions packages/format-library/src/default-formats.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { superscript } from './superscript';
import { keyboard } from './keyboard';
import { unknown } from './unknown';
import { language } from './language';
import { nonBreakingSpace } from './non-breaking-space';

export default [
bold,
Expand All @@ -29,4 +30,5 @@ export default [
keyboard,
unknown,
language,
nonBreakingSpace,
];
29 changes: 29 additions & 0 deletions packages/format-library/src/non-breaking-space/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/**
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { insert } from '@wordpress/rich-text';
import { RichTextShortcut } from '@wordpress/block-editor';

const name = 'core/non-breaking-space';
const title = __( 'Non breaking space' );

export const nonBreakingSpace = {
name,
title,
tagName: 'span',
ellatrix marked this conversation as resolved.
Show resolved Hide resolved
className: 'nbsp',
Copy link
Member

Choose a reason for hiding this comment

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

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. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even if I changed the tagName to nbsp, it worked fine! Does tagName not have to be an element that is allowed as HTML?

Copy link
Member

Choose a reason for hiding this comment

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

Not if we're not going to use it 😄

edit( { value, onChange } ) {
function addNonBreakingSpace() {
onChange( insert( value, '\u00a0' ) );
}

return (
<RichTextShortcut
type="primaryShift"
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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):

chrome

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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)

character=" "
onUse={ addNonBreakingSpace }
/>
);
},
};
Loading