Fix Space Bar in <summary> element #24558
Conversation
georgeh
requested review from
cameronvoell,
ellatrix,
etoledom and
guarani
as code owners
|
Size Change: -475 B (0%) Total Size: 1.16 MB
ℹ️ View Unchanged
|
ZebulanStanphill
added
[Feature] Rich Text
Co-authored-by: Zebulan Stanphill <[email protected]>
|
This code definitely works, but I'm not sure I understand the code well enough to feel confident that it's solving it the right way. Requesting a technical review from @ellatrix. |
| @@ -527,6 +527,12 @@ function RichText( { | |||
| function handleSpace( event ) { | |||
| const { keyCode, shiftKey, altKey, metaKey, ctrlKey } = event; | |||
|
|
|||
| // Allow space characters to be typed in a <summary>. | |||
| if ( keyCode === SPACE && TagName === 'summary' ) { | |||
| handleChange( insert( createRecord(), ' ' ) ); | |||
There was a problem hiding this comment.
I don't think RichText should be responsible for inserting characters, because this varies by keyboards and language and user settings. If the browser is blocking spaces to be inserted because of some other behaviour, the behaviour should be prevented some other way.
There was a problem hiding this comment.
Thanks for looking at this! Are you saying that the fix for this issue belongs somewhere else, or that we should be smarter about the locale of the character we are inserting on line 532? I want to do it the right way, and would appreciate your help understanding what that is.
I spent a lot of time on #23940 looking for a way to fix this at the block level but couldn't get it to work without any luck. Something about the way that Firefox handles the spacebar in a <summary contenteditable> element eats the space character, so the two solutions I came up with were:
- Add this space inserter
- Make the
RichTexta div child of the<summary>element, which means either rendering that on the client side or having separate CSS for the editor and client
I appreciate any pointers on how to make this better, and thanks again for the feedback.
There was a problem hiding this comment.
I don't know honestly. First, since this is a problem only with the summary element, we should fix it in the block and not in rich text imo. Maybe you can prevent default the event? Not sure. If the only way is to insert the character ourselves, maybe have a look at the data property in the event and insert that data?
|
Fixed in #30244. |
|
🔥 |
Description
This fixes an issue in Firefox where the if you create a
RichTextcomponent withtagName="summary"the spacebar event won't register.This does not address the issue where the
detailselement opens/closes with the spacebar keyUp event. That can be handled withpreventDefault()and event handlers.How has this been tested?
This was tested in Firefox, Chrome, and Safari in #23940
Here's some example code that exposes the issue, without the change the space character won't be added to a RichText
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist: