Enhanced backspace#81
Conversation
|
|
||
|
|
||
| // Merge any text nodes we split | ||
| mergeInlines( startContainer, range ); |
There was a problem hiding this comment.
This is very irritating. There seems to be a bug where mergeInlines will actually screw up the range, giving getHTML dangerous side effects. Unsure why we need to mergeInlines instead of Node.normalize here, but if there is a real reason we will need to fix mergeInlines
There was a problem hiding this comment.
I think we should probably look at this anyway. I stepped through and noticed two things:
- The
{start,end}Offsetcalculation doesn't seem accurate - The actual process of detaching/re-appending the nodes seems to alter the selection (I guess possibly when the selection is 'dangling' it defaults to the end of the container node?)
There was a problem hiding this comment.
hmm, this is tricky. On master, if you have two adjacent bold tags backspace will merge them. In this PR that is no longer the case. Personally I like that behavior and it leads to cleaner content over time, but we can move the mergeinines into backspace to do the same thing.
There was a problem hiding this comment.
However I'm not sure there aren't other side effects. Will play with this more over the weekend.
There was a problem hiding this comment.
Cool, I will take another look at that mergeInlines, there should be a way to get it to properly 'restore' the selection.
Alternatively we can just avoid using withBookmarks here and figure out the cursor position by just looking at the range, and sidestep this issue entirely.
There was a problem hiding this comment.
I think the mergeInlines is desirable in some cases, but don't think a method called getHTML should modify the content
|
We should also add a mirror event for |
2 participants
Add special event for backspacing at the beginning of the editor