Replace text block on paste if empty #2271
Conversation
ellatrix
force-pushed
the
fix/paste-empty-paragraph
branch
from
3c94376 to
df273e9
Compare
Codecov Report
@@ Coverage Diff @@
## master #2271 +/- ##
==========================================
- Coverage 29.01% 28.99% -0.02%
==========================================
Files 167 167
Lines 5060 5063 +3
Branches 835 836 +1
==========================================
Hits 1468 1468
- Misses 3053 3055 +2
- Partials 539 540 +1
Continue to review full report at Codecov.
|
blocks/editable/index.js
Outdated
| } | ||
|
|
||
| // We must wait for TinyMCE to clean up paste containers after this event. |
There was a problem hiding this comment.
I don't think TinyMCE is doing this asynchronously, so setTimeout is a bit of a hack. Instead can we make sure this logic is only triggered after TinyMCE completes its cleaning?
There was a problem hiding this comment.
What do you suggest to use instead?
There was a problem hiding this comment.
What is it in TinyMCE doing the cleaning?
There was a problem hiding this comment.
blocks/editable/index.js
Outdated
| } | ||
|
|
||
| // We must wait for TinyMCE to clean up paste containers after this event. | ||
| window.setTimeout( () => { |
There was a problem hiding this comment.
setTimeout is a common global between DOM APIs and Node, we shouldn't need window.?
There was a problem hiding this comment.
I like being explicit sorry :)
blocks/editable/index.js
Outdated
|
|
||
| // We must wait for TinyMCE to clean up paste containers after this event. | ||
| window.setTimeout( () => { | ||
| const rootNode = this.editor.getBody(); |
There was a problem hiding this comment.
Will this work okay if component unmounts between time setTimeout is called and callback is invoked?
There was a problem hiding this comment.
Thanks for noting :) What's the best check for that? this.editor?
|
Repeating steps from #2180, except with two paragraphs instead of one from Lipsum, the initial paragraph is not stripped, and an error is logged:
|
ellatrix
force-pushed
the
fix/paste-empty-paragraph
branch
from
85612f2 to
69ccb98
Compare
|
What's the status here? We cannot really improve without a better MCE event. |
|
@iseulde how important is this? we could get a new event on the list to look at (sorry if this has been discussed elsewhere already) |
|
@androb I'm not sure... More important if it blocks this PR. :) |
|
This works for me 👍 👍 I'm seeing an issue with the cover image block being "externally modified" on paste, but I believe that's a separate issue. |
|
No idea how that's related. Cross link: https://wordpress.slack.com/archives/C02QB2JS7/p1503060598000155 |
ellatrix
force-pushed
the
fix/paste-empty-paragraph
branch
from
69ccb98 to
bb086d1
Compare
|
Concerns addressed. |
…_not_merged Fix merged of text blocks when a style is active.
This PR fixes pasting in an empty paragraph. At the moment this would "split" the empty paragraph and paste the blocks in between, resulting in an empty block above and below the pasted blocks. Instead, we should replace the current block.