-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[Writing Flow]: Fix browser formatting with shortcuts on multiple selection #41207
[Writing Flow]: Fix browser formatting with shortcuts on multiple selection #41207
Conversation
Size Change: +32 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
fd657e2
to
864169d
Compare
864169d
to
a3d3ed5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clear diff, and good choice with beforeinput
. LGTM!
return; | ||
} | ||
// Prevent the browser to format something when we have multiselection. | ||
if ( event?.inputType?.startsWith( 'format' ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
event?.inputType?.
// ^
Can event
ever be falsy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to clarify where these format*
input events come from? Or is this easy enough to understand?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can event ever be falsy?
I guess not :). I made some more tests and I don't think it's possible to get here without an event.
Do we need to clarify where these format* input events come from? Or is this easy enough to understand?
I think the Prevent the browser
is enough.
1927812
to
4f91031
Compare
What?
Fixes: #41164
Some common shortcuts for formatting like
cmd + b (bold), cmd + i(italic), etc...
are implemented by browsers. In GB we handle such events in RichText for multiple reasons(example would be different markup applied per browser). With the current multiple selection implementation we make conditionally the content beingcontentEditable
which in turn enabled the browsers to add markup with such shortcuts.At first this seemed to work well, but we keep all formatting internally in RichText, and that's why when you would select a single block afterwards, the formatting was 'gone'.
So, until we implement this(issue) we need to disable these formatting from browsers.
Testing Instructions
Before
before.mov
After
after.mov