RichText: Removing/merging richtext should only trigger if the selection is collapsed #8651
Conversation
youknowriad
added
the
[Feature] Rich Text
| @@ -437,8 +437,10 @@ export class RichText extends Component { | |||
| const { keyCode } = event; | |||
|
|
|||
| if ( | |||
| ( keyCode === BACKSPACE && isHorizontalEdge( rootNode, true ) ) || | |||
| ( keyCode === DELETE && isHorizontalEdge( rootNode, false ) ) | |||
| window.getSelection().isCollapsed && ( | |||
There was a problem hiding this comment.
Should we continue with setting it at the top as browser dependencies? const { getSelection } = window;
| ( keyCode === BACKSPACE && isHorizontalEdge( rootNode, true ) ) || | ||
| ( keyCode === DELETE && isHorizontalEdge( rootNode, false ) ) | ||
| window.getSelection().isCollapsed && ( | ||
| ( keyCode === BACKSPACE && isHorizontalEdge( rootNode, true ) ) || |
There was a problem hiding this comment.
For some reason I would have thought that isHorizontalEdge also does the check, as navigating also requires it.
There was a problem hiding this comment.
Yes, I noticed it was also checked in the WritingFlow. I didn't want to change isHorizontalEdge behavior as I wasn't sure about the implications
There was a problem hiding this comment.
Maybe. As it reads, I see no reason why isHorizontalEdge should have any concern about whether the selection is collapsed or not. This assumes there's a use case for wanting to know if there's an uncollapsed selection at the horizontal edge. We don't have this need in our project, which might bias our opinion on it. Or maybe "is horizontal edge" is meant to mean that pressing ArrowLeft/ArrowRight would have no effect (in a traditional input).
| @@ -87,4 +87,18 @@ describe( 'splitting and merging blocks', () => { | |||
|
|
|||
| expect( await getEditedPostContent() ).toMatchSnapshot(); | |||
| } ); | |||
|
|
|||
| it( 'Should not merge paragraphs if the selection is not collapsed', async () => { | |||
There was a problem hiding this comment.
We should have a rule but I think in general these should be lowercase. It's inconsistent now in the code though so whatever 😆
There was a problem hiding this comment.
Starting today (if I remember :P)
| @@ -12,6 +12,16 @@ exports[`splitting and merging blocks Should merge into inline boundary position | |||
| <!-- /wp:paragraph -->" | |||
| `; | |||
|
|
|||
| exports[`splitting and merging blocks Should not merge paragraphs if the selection is not collapsed 1`] = ` | |||
There was a problem hiding this comment.
As mentioned, it does make these specs read weirdly though.
|
This duplicated effort from #8306 |
closes #8639
closes #8638
Testing instructions
Repeat the testing instructions from the issue, it should work as intended.