-
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
[RNMobile] Split Para/Heading blocks on enter.KEY - step 1 #10553
Conversation
…omponent, when enter.KEY is detected. Para block for mobile now has onSplit defined, that in this first implementation tries to add a test block after.
…ckAfter if defined in props.
@@ -24,6 +24,25 @@ import './editor.scss'; | |||
const minHeight = 50; | |||
|
|||
class HeadingEdit extends Component { | |||
constructor() { | |||
super( ...arguments ); | |||
|
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.
nitpick: can we remove this extra line here?
|
||
splitContent( htmlText, start, end ) { | ||
const { onSplit } = this.props; | ||
|
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.
nitpick #2: can we remove this extra line here?
import { RichText } from '@wordpress/editor'; | ||
|
||
const minHeight = 50; | ||
|
||
class ParagraphEdit extends Component { | ||
constructor() { | ||
super( ...arguments ); | ||
|
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.
nitpick #3: let's remove the extra line here
// Descriptive placeholder: This logic still needs to be implemented. | ||
onHTMLContentWithCursor( htmlText, start, end ) { | ||
if ( ! this.props.onSplit ) { | ||
// insert the \n char instead? |
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.
Let's add a ToDo comment here to make sure this is something we come back to at a follow up stage
…rnmobile/split-para-on-enter
…rnmobile/split-para-on-enter
Ready for another round @mzorz! 🙇 |
…rnmobile/split-para-on-enter
} | ||
|
||
// eslint-disable-next-line no-unused-vars | ||
splitBlock( htmlText, start, end ) { |
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.
@daniloercoli , can we perhaps go closer to the interface of the web-side counterpart, wdyt? https://github.com/WordPress/gutenberg/blob/master/packages/block-library/src/heading/edit.js#L52.
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.
Oh yes, we will probably go toward that direction in the short future. For now i'd like to go with the current implementation, and get the ball rolling.
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.
Thanks for addressing the small-ish comments! LGTM
This PR does add a very simple implementation of
onHTMLContentWithCursor
method to RichText. The code doesn't check much at the moment, but just callonSplit
on the outer Block, if it hasonSplit
defined.onSplit
on both Para and Heading blocks, is very minimal, it just callsinsertAfter
without checking the position of the cursor, or any other condition.