fix/1273: Delete or backspace in a empty classic text block removes it #2482
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2482 +/- ##
==========================================
+ Coverage 26.93% 32.57% +5.63%
==========================================
Files 158 171 +13
Lines 4897 5744 +847
Branches 814 1027 +213
==========================================
+ Hits 1319 1871 +552
- Misses 3035 3253 +218
- Partials 543 620 +77
Continue to review full report at Codecov.
|
| @@ -67,6 +70,16 @@ export default class OldEditor extends Component { | |||
| } ); | |||
| } ); | |||
|
|
|||
| editor.on( 'keydown', ( event ) => { | |||
| if ( ( event.keyCode === BACKSPACE || event.keyCode === DELETE ) && | |||
| /^\n?$/.test( editor.getContent( { format: 'text' } ) ) ) { | |||
There was a problem hiding this comment.
How performant is { format: 'text' }? Does it need to run TinyMCE's serializer? If so, might be worryingly non-performant to occur on every keypress.
There was a problem hiding this comment.
It is literally a call to body.innerText falling back to body.textContent so no it does not run the serializer. I think it should be fairly low cost.
There was a problem hiding this comment.
Though it does fire the 'BeforeGetContent' and 'GetContent' event by default. I could turn that off by passing { format: 'text', no_events: true } .
There was a problem hiding this comment.
I rewrote the empty check to test the node counts first just in case.
| // <p><br data-mce-bogus="1"></p> | ||
| // avoid expensive checks for large documents | ||
| const body = editor.getBody(); | ||
| if ( body.childNodes > 1 ) { |
There was a problem hiding this comment.
Isn't childNodes a NodeList. In other words, shouldn't this be checking childNodes.length, not childNodes itself?
There was a problem hiding this comment.
Yes it should! Working on a fix.
There was a problem hiding this comment.
@EphoxJames Could editor.dom.isEmpty not be used?
There was a problem hiding this comment.
If I'm reading the source correctly I don't believe it would do what we want. If we called it passing in the paragraph tag of <p><br/></p> then it would return true but if we called it with the body of the editor then it would return false (effectively passing the div tag of <div><p><br/></p></div> ) because it would consider the p tag to be content.
It would also do a lot more work internally.
This pull request fixes issue #1273
Pressing delete or backspace inside an empty classic text block will now remove it.