Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RichText: fix caret position when deleting a selected word #11965

Merged
merged 4 commits into from
Nov 16, 2018
Merged

RichText: fix caret position when deleting a selected word #11965

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e6a4094
Add test case
ellatrix Nov 16, 2018
5dd5310
Try native delete behaviour for uncollpased selection
ellatrix Nov 16, 2018
252142a
Handle deletion if rich text is entirely selected
ellatrix Nov 16, 2018
31637c0
Remove unnecessary code
ellatrix Nov 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion packages/dom/src/dom.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,11 +497,25 @@ export function isEntirelySelected( element ) {

const { startContainer, endContainer, startOffset, endOffset } = range;

return (
if (
startContainer === element &&
endContainer === element &&
startOffset === 0 &&
endOffset === element.childNodes.length
) {
return true;
}

const lastChild = element.lastChild;
const lastChildContentLength = lastChild.nodeType === TEXT_NODE ?
lastChild.data.length :
lastChild.childNodes.length;

return (
startContainer === element.firstChild &&
endContainer === element.lastChild &&
startOffset === 0 &&
endOffset === lastChildContentLength
);
}

Expand Down
10 changes: 2 additions & 8 deletions packages/editor/src/components/rich-text/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,8 +510,8 @@ export class RichText extends Component {
const start = getSelectionStart( value );
const end = getSelectionEnd( value );

// Always handle uncollapsed selections ourselves.
if ( ! isCollapsed( value ) ) {
// Always handle full content deletion ourselves.
if ( start === 0 && end !== 0 && end === value.text.length ) {
this.onChange( remove( value ) );
event.preventDefault();
return;
Expand Down Expand Up @@ -600,12 +600,6 @@ export class RichText extends Component {
* keyboard.
*/
onKeyUp( { keyCode } ) {
// The input event does not fire when the whole field is selected and
// BACKSPACE is pressed.
if ( keyCode === BACKSPACE ) {
this.onChange( this.createRecord() );
}

// `scrollToRect` is called on `nodechange`, whereas calling it on
// `keyup` *when* moving to a new RichText element results in incorrect
// scrolling. Though the following allows false positives, it results
Expand Down
9 changes: 6 additions & 3 deletions packages/editor/src/components/rich-text/tinymce.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import classnames from 'classnames';
*/
import { Component, createElement } from '@wordpress/element';
import { BACKSPACE, DELETE, ENTER, LEFT, RIGHT } from '@wordpress/keycodes';
import { isEntirelySelected } from '@wordpress/dom';

/**
* Internal dependencies
Expand Down Expand Up @@ -272,11 +273,13 @@ export default class TinyMCE extends Component {

onKeyDown( event ) {
const { keyCode } = event;
const { startContainer, startOffset, endContainer, endOffset } = getSelection().getRangeAt( 0 );
const isCollapsed = startContainer === endContainer && startOffset === endOffset;
const isDelete = keyCode === DELETE || keyCode === BACKSPACE;

// Disables TinyMCE behaviour.
if ( keyCode === ENTER || ( ! isCollapsed && ( keyCode === DELETE || keyCode === BACKSPACE ) ) ) {
if (
keyCode === ENTER ||
( isDelete && isEntirelySelected( this.editorNode ) )
) {
event.preventDefault();
// For some reason this is needed to also prevent the insertion of
// line breaks.
Expand Down
82 changes: 82 additions & 0 deletions test/e2e/specs/__snapshots__/writing-flow.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,52 @@ exports[`adding blocks should clean TinyMCE content 2`] = `
<!-- /wp:paragraph -->"
`;

exports[`adding blocks should create valid paragraph blocks when rapidly pressing Enter 1`] = `
"<!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->"
`;

exports[`adding blocks should insert line break at end 1`] = `
"<!-- wp:paragraph -->
<p>a<br><br></p>
Expand Down Expand Up @@ -75,3 +121,39 @@ exports[`adding blocks should navigate around inline boundaries 1`] = `
<p>BeforeThird</p>
<!-- /wp:paragraph -->"
`;

exports[`adding blocks should not delete surrounding space when deleting a selected word 1`] = `
"<!-- wp:paragraph -->
<p>alpha  gamma</p>
<!-- /wp:paragraph -->"
`;

exports[`adding blocks should not delete surrounding space when deleting a selected word 2`] = `
"<!-- wp:paragraph -->
<p>alpha beta gamma</p>
<!-- /wp:paragraph -->"
`;

exports[`adding blocks should not delete surrounding space when deleting a word with Alt+Backspace 1`] = `
"<!-- wp:paragraph -->
<p>alpha  gamma</p>
<!-- /wp:paragraph -->"
`;

exports[`adding blocks should not delete surrounding space when deleting a word with Alt+Backspace 2`] = `
"<!-- wp:paragraph -->
<p>alpha beta gamma</p>
<!-- /wp:paragraph -->"
`;

exports[`adding blocks should not delete surrounding space when deleting a word with Backspace 1`] = `
"<!-- wp:paragraph -->
<p>1  3</p>
<!-- /wp:paragraph -->"
`;

exports[`adding blocks should not delete surrounding space when deleting a word with Backspace 2`] = `
"<!-- wp:paragraph -->
<p>1 2 3</p>
<!-- /wp:paragraph -->"
`;
60 changes: 38 additions & 22 deletions test/e2e/specs/writing-flow.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,42 +224,58 @@ describe( 'adding blocks', () => {
expect( isInBlock ).toBe( true );
} );

it( 'should not delete trailing spaces when deleting a word with backspace', async () => {
it( 'should not delete surrounding space when deleting a word with Backspace', async () => {
await clickBlockAppender();
await page.keyboard.type( '1 2 3 4' );
await page.keyboard.type( '1 2 3' );
await pressTimes( 'ArrowLeft', ' 3'.length );
await page.keyboard.press( 'Backspace' );
await page.keyboard.type( '4' );
const blockText = await page.evaluate( () => document.activeElement.textContent );
expect( blockText ).toBe( '1 2 3 4' );

expect( await getEditedPostContent() ).toMatchSnapshot();

await page.keyboard.type( '2' );

expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'should not delete trailing spaces when deleting a word with alt + backspace', async () => {
it( 'should not delete surrounding space when deleting a word with Alt+Backspace', async () => {
await clickBlockAppender();
await page.keyboard.type( 'alpha beta gamma delta' );
await page.keyboard.type( 'alpha beta gamma' );
await pressTimes( 'ArrowLeft', ' gamma'.length );

if ( process.platform === 'darwin' ) {
await pressWithModifier( 'Alt', 'Backspace' );
} else {
await pressWithModifier( META_KEY, 'Backspace' );
}
await page.keyboard.type( 'delta' );
const blockText = await page.evaluate( () => document.activeElement.textContent );
expect( blockText ).toBe( 'alpha beta gamma delta' );

expect( await getEditedPostContent() ).toMatchSnapshot();

await page.keyboard.type( 'beta' );

expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'should not delete surrounding space when deleting a selected word', async () => {
await clickBlockAppender();
await page.keyboard.type( 'alpha beta gamma' );
await pressTimes( 'ArrowLeft', ' gamma'.length );
await page.keyboard.down( 'Shift' );
await pressTimes( 'ArrowLeft', 'beta'.length );
await page.keyboard.up( 'Shift' );
await page.keyboard.press( 'Backspace' );

expect( await getEditedPostContent() ).toMatchSnapshot();

await page.keyboard.type( 'beta' );

expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'should create valid paragraph blocks when rapidly pressing Enter', async () => {
await clickBlockAppender();
await page.keyboard.press( 'Enter' );
await page.keyboard.press( 'Enter' );
await page.keyboard.press( 'Enter' );
await page.keyboard.press( 'Enter' );
await page.keyboard.press( 'Enter' );
await page.keyboard.press( 'Enter' );
await page.keyboard.press( 'Enter' );
await page.keyboard.press( 'Enter' );
await page.keyboard.press( 'Enter' );
await page.keyboard.press( 'Enter' );
await pressTimes( 'Enter', 10 );

// Check that none of the paragraph blocks have <br> in them.
const postContent = await getEditedPostContent();
expect( postContent.indexOf( 'br' ) ).toBe( -1 );
expect( await getEditedPostContent() ).toMatchSnapshot();
} );
} );