-
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
RichText: Fix browser formatting buttons #13833
Conversation
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.
I tested this in Safari 12 and confirmed that all of the Touch Bar formatting buttons either work or don't break anything.
Agree that it would be nice to get the Insert List buttons working in a follow-up PR.
The approach makes sense to me, but take my ✅ with a grain of salt as I'm not a RichText
expert! 🙂
*/ | ||
import { Component } from '@wordpress/element'; | ||
|
||
export class RichTextInputEvent extends Component { |
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.
A unit test for this component could be good.
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.
How would it work?
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.
Something like this pseudocode:
const onInput = jest.fn();
mount( <RichTextInputEvent inputType="ella-is-cool" onInput={ onInput } /> );
const event = new Event( 'input' );
event.inputType = 'ella-is-cool';
document.dispatchEvent( event );
expect( onInput ).toHaveBeenCalled();
I find these sorts of tests don't hurt and sometimes catch silly regressions e.g. typos.
|
||
onInput( event ) { | ||
if ( event.inputType === this.props.inputType ) { | ||
this.props.onInput(); |
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.
Is there a need to preventDefault
as well, to ensure it's fully controlled?
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.
There's not really anything to prevent. The input
event happens after the fact.
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.
There's not really anything to prevent. The
input
event happens after the fact.
And to that point, an input
event is in-fact not cancelable.
* RichText: Fix browser formatting buttons * Simplify * componentDidMount instead of componentWillMount
* RichText: Fix browser formatting buttons * Simplify * componentDidMount instead of componentWillMount
Description
This PR fixes any formatting buttons that are provided by the browser, e.g. formatting buttons in Safari and iOS Safari.
What's the problem at the moment?
<b>
tags.RichText
, such as the align and list buttons. They could be linked at a block level though. I've prevented them from doing anything, but ideally they should work at the block level (maybe a future PR). I've found no way to remove the buttons.ctrl+u
on a Mac, a shortcut that wasn't overridden by us.The most reliable solution I've found is checking the input event for
inputType
, and then resetting the DOM. After this a format in the format library may useRichTextInputEvent
to do something based on that information.How has this been tested?
Screenshots
Types of changes
Checklist: