Quote Block: Fixing focus transfer between citation and content #5100
Conversation
| this.props.value !== this.savedContent | ||
| this.props.value !== this.savedContent && | ||
| ! isEqual( this.props.value, prevProps.value ) && | ||
| ! isEqual( this.props.value, this.savedContent ) |
There was a problem hiding this comment.
This is the real fix. I removed this as part of #4955 but it's necessary to avoid unnecessarily content updates. I don't see any performance loss in adding this and it could even improve performance because we don't use those TinyMCE functions setBookmark and setContent constantly
There was a problem hiding this comment.
@youknowriad I feel like we removed and added this a few times I the last year now. :) Maybe worth leaving a comment?
There was a problem hiding this comment.
(I was surprised to see these lines again.)
| @@ -11,6 +11,10 @@ | |||
| } | |||
|
|
|||
| .wp-block-pullquote { | |||
| cite { | |||
| display: block; | |||
There was a problem hiding this comment.
This just increases the click area of the citation to the whole line (width: 100%)
There was a problem hiding this comment.
This kind of tells me that we shouldn't be using cite for this... :)
|
Seeing an issue:
Content from the first paragraph disappears. |
|
It seems like this solved the problems with the quote. Regarding the problem reported by @aduth I'm also able to replicate it seems to only happen in paragraph other blocks e.g: list works well. |
|
Good catch @aduth It's fixed now. It was happening because when you hit "Enter", TinyMCE updates the content of the rich text before even we call onSplit. And since we want to keep the same content as before in this case, we have to call Previous |
blocks/rich-text/index.js
Outdated
|
|
||
| // Since we need to abort the change done by TinyMCE | ||
| // we need to call updateContent to restore the initial content before calling onSplit. | ||
| this.updateContent(); |
There was a problem hiding this comment.
If it's the case that we must call updateContent before every this.props.onSplit, could be wise to create a separate class function for consistency/safety's sake.
youknowriad
force-pushed
the
fix/quote-focus
branch
from
f224da7 to
9360dc8
Compare
| */ | ||
| restoreContentAndSplit( before, after, blocks ) { | ||
| this.updateContent(); | ||
| this.props.onSplit( before, after, ...blocks ); |
There was a problem hiding this comment.
Hm, now we have (1) this.props.onSplit checks in several places and (2) none here.
| @@ -796,6 +799,19 @@ export class RichText extends Component { | |||
| } ) ); | |||
| } | |||
|
|
|||
| /** | |||
| * Calling onSplit means we need to abort the change done by TinyMCE. | |||
There was a problem hiding this comment.
I don't really understand this function. In most places where it's used, we're also calling preventDefault. Shouldn't that be enough to "abort the change" that would otherwise be effected by TinyMCE ?
closes #5081
This PR fixes moving focus between citation and content in quote blocks.
Testing instructions