Skip to content

Commit

Permalink
Writing Flow: allow undo of patterns with BACKSPACE and ESC (#14776)
Browse files Browse the repository at this point in the history
* RichText/Patterns/Input Interaction: allow undo of patterns with BACKSPACE and ESC

* Only run input rules when inserting text

* Fix typo

* Simplify
  • Loading branch information
ellatrix committed Aug 22, 2019
1 parent 47dfd82 commit 6ecff18
Show file tree
Hide file tree
Showing 10 changed files with 222 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,18 @@ _Returns_

- `boolean`: Whether the given block type is allowed to be inserted.

<a name="didAutomaticChange" href="#didAutomaticChange">#</a> **didAutomaticChange**

Returns true if the last change was an automatic change, false otherwise.

_Parameters_

- _state_ `Object`: Global application state.

_Returns_

- `boolean`: Whether the last change was automatic.

<a name="getAdjacentBlockClientId" href="#getAdjacentBlockClientId">#</a> **getAdjacentBlockClientId**

Returns the client ID of the block adjacent one at the given reference
Expand Down
26 changes: 26 additions & 0 deletions packages/block-editor/src/components/rich-text/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ class RichTextWrapper extends Component {
this.onPaste = this.onPaste.bind( this );
this.onDelete = this.onDelete.bind( this );
this.inputRule = this.inputRule.bind( this );
this.markAutomaticChange = this.markAutomaticChange.bind( this );
}

onEnter( { value, onChange, shiftKey } ) {
Expand All @@ -81,6 +82,7 @@ class RichTextWrapper extends Component {
onReplace( [
transformation.transform( { content: value.text } ),
] );
this.markAutomaticChange();
}
}

Expand Down Expand Up @@ -273,6 +275,7 @@ class RichTextWrapper extends Component {
const block = transformation.transform( content );

onReplace( [ block ] );
this.markAutomaticChange();
}

getAllowedFormats() {
Expand All @@ -293,6 +296,16 @@ class RichTextWrapper extends Component {
return formattingControls.map( ( name ) => `core/${ name }` );
}

/**
* Marks the last change as an automatic change at the next idle period to
* ensure all selection changes have been recorded.
*/
markAutomaticChange() {
window.requestIdleCallback( () => {
this.props.markAutomaticChange();
} );
}

render() {
const {
children,
Expand All @@ -313,6 +326,10 @@ class RichTextWrapper extends Component {
onExitFormattedText,
isSelected: originalIsSelected,
onCreateUndoLevel,
// eslint-disable-next-line no-unused-vars
markAutomaticChange,
didAutomaticChange,
undo,
placeholder,
// eslint-disable-next-line no-unused-vars
allowedFormats,
Expand Down Expand Up @@ -375,6 +392,9 @@ class RichTextWrapper extends Component {
__unstableOnEnterFormattedText={ onEnterFormattedText }
__unstableOnExitFormattedText={ onExitFormattedText }
__unstableOnCreateUndoLevel={ onCreateUndoLevel }
__unstableMarkAutomaticChange={ this.markAutomaticChange }
__unstableDidAutomaticChange={ didAutomaticChange }
__unstableUndo={ undo }
>
{ ( { isSelected, value, onChange, Editable } ) =>
<>
Expand Down Expand Up @@ -435,6 +455,7 @@ const RichTextContainer = compose( [
getSelectionStart,
getSelectionEnd,
getSettings,
didAutomaticChange,
} = select( 'core/block-editor' );

const selectionStart = getSelectionStart();
Expand All @@ -453,6 +474,7 @@ const RichTextContainer = compose( [
selectionStart: isSelected ? selectionStart.offset : undefined,
selectionEnd: isSelected ? selectionEnd.offset : undefined,
isSelected,
didAutomaticChange: didAutomaticChange(),
};
} ),
withDispatch( ( dispatch, {
Expand All @@ -465,7 +487,9 @@ const RichTextContainer = compose( [
enterFormattedText,
exitFormattedText,
selectionChange,
__unstableMarkAutomaticChange,
} = dispatch( 'core/block-editor' );
const { undo } = dispatch( 'core/editor' );

This comment has been minimized.

Copy link
@youknowriad

youknowriad Nov 28, 2019

Contributor

I saw this, ideally, we don't use core/editor or the editor package in the block editor module as it's a generic WP agnostic package. We have some open PRs to remove all existing usage, we should try to find a solution for this one as well.


return {
onCreateUndoLevel: __unstableMarkLastChangeAsPersistent,
Expand All @@ -474,6 +498,8 @@ const RichTextContainer = compose( [
onSelectionChange( start, end ) {
selectionChange( clientId, identifier, start, end );
},
markAutomaticChange: __unstableMarkAutomaticChange,
undo,
};
} ),
withFilters( 'experimentalRichText' ),
Expand Down
14 changes: 14 additions & 0 deletions packages/block-editor/src/store/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,20 @@ export function __unstableMarkLastChangeAsPersistent() {
return { type: 'MARK_LAST_CHANGE_AS_PERSISTENT' };
}

/**
* Returns an action object used in signalling that the last block change is
* an automatic change, meaning it was not performed by the user, and can be
* undone using the `Escape` and `Backspace` keys. This action must be called
* after the change was made, and any actions that are a consequence of it, so
* it is recommended to be called at the next idle period to ensure all
* selection changes have been recorded.
*
* @return {Object} Action object.
*/
export function __unstableMarkAutomaticChange() {
return { type: 'MARK_AUTOMATIC_CHANGE' };
}

/**
* Returns an action object used to enable or disable the navigation mode.
*
Expand Down
13 changes: 13 additions & 0 deletions packages/block-editor/src/store/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1251,6 +1251,18 @@ export function lastBlockAttributesChange( state, action ) {
return null;
}

/**
* Reducer returning automatic change state.
*
* @param {boolean} state Current state.
* @param {Object} action Dispatched action.
*
* @return {boolean} Updated state.
*/
export function didAutomaticChange( state, action ) {
return action.type === 'MARK_AUTOMATIC_CHANGE';
}

export default combineReducers( {
blocks,
isTyping,
Expand All @@ -1264,4 +1276,5 @@ export default combineReducers( {
preferences,
lastBlockAttributesChange,
isNavigationMode,
didAutomaticChange,
} );
11 changes: 11 additions & 0 deletions packages/block-editor/src/store/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1415,3 +1415,14 @@ function getReusableBlocks( state ) {
export function isNavigationMode( state ) {
return state.isNavigationMode;
}

/**
* Returns true if the last change was an automatic change, false otherwise.
*
* @param {Object} state Global application state.
*
* @return {boolean} Whether the last change was automatic.
*/
export function didAutomaticChange( state ) {
return state.didAutomaticChange;
}
10 changes: 10 additions & 0 deletions packages/e2e-tests/specs/__snapshots__/rich-text.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ exports[`RichText should not lose selection direction 1`] = `
<!-- /wp:paragraph -->"
`;

exports[`RichText should not undo backtick transform with backspace after selection change 1`] = `""`;

exports[`RichText should not undo backtick transform with backspace after typing 1`] = `""`;

exports[`RichText should only mutate text data on input 1`] = `
"<!-- wp:paragraph -->
<p>1<strong>2</strong>34</p>
Expand Down Expand Up @@ -77,3 +81,9 @@ exports[`RichText should update internal selection after fresh focus 1`] = `
<p>1<strong>2</strong></p>
<!-- /wp:paragraph -->"
`;

exports[`RichText should undo backtick transform with backspace 1`] = `
"<!-- wp:paragraph -->
<p>\`a\`</p>
<!-- /wp:paragraph -->"
`;
16 changes: 16 additions & 0 deletions packages/e2e-tests/specs/blocks/__snapshots__/list.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,10 @@ exports[`List should not transform lines in block when transforming multiple blo
<!-- /wp:list -->"
`;
exports[`List should not undo asterisk transform with backspace after selection change 1`] = `""`;
exports[`List should not undo asterisk transform with backspace after typing 1`] = `""`;
exports[`List should outdent with children 1`] = `
"<!-- wp:list -->
<ul><li>a<ul><li>b<ul><li>c</li></ul></li></ul></li></ul>
Expand Down Expand Up @@ -267,3 +271,15 @@ exports[`List should split into two with paragraph and merge lists 3`] = `
<ul><li>two</li></ul>
<!-- /wp:list -->"
`;
exports[`List should undo asterisk transform with backspace 1`] = `
"<!-- wp:paragraph -->
<p>* </p>
<!-- /wp:paragraph -->"
`;
exports[`List should undo asterisk transform with escape 1`] = `
"<!-- wp:paragraph -->
<p>* </p>
<!-- /wp:paragraph -->"
`;
41 changes: 41 additions & 0 deletions packages/e2e-tests/specs/blocks/list.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,47 @@ describe( 'List', () => {
expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'should undo asterisk transform with backspace', async () => {
await clickBlockAppender();
await page.keyboard.type( '* ' );
await page.evaluate( () => new Promise( window.requestIdleCallback ) );
await page.keyboard.press( 'Backspace' );

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

it( 'should undo asterisk transform with escape', async () => {
await clickBlockAppender();
await page.keyboard.type( '* ' );
await page.evaluate( () => new Promise( window.requestIdleCallback ) );
await page.keyboard.press( 'Escape' );

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

it( 'should not undo asterisk transform with backspace after typing', async () => {
await clickBlockAppender();
await page.keyboard.type( '* a' );
await page.evaluate( () => new Promise( window.requestIdleCallback ) );
await page.keyboard.press( 'Backspace' );
await page.keyboard.press( 'Backspace' );

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

it( 'should not undo asterisk transform with backspace after selection change', async () => {
await clickBlockAppender();
await page.keyboard.press( 'Enter' );
await page.keyboard.type( '* ' );
await page.evaluate( () => new Promise( window.requestIdleCallback ) );
await page.keyboard.press( 'ArrowUp' );
await page.keyboard.press( 'ArrowDown' );
await page.keyboard.press( 'Backspace' );

// Expect list to be deleted
expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'can be created by typing "/list"', async () => {
// Create a list with the slash block shortcut.
await clickBlockAppender();
Expand Down
36 changes: 36 additions & 0 deletions packages/e2e-tests/specs/rich-text.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,42 @@ describe( 'RichText', () => {
expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'should undo backtick transform with backspace', async () => {
await clickBlockAppender();
await page.keyboard.type( '`a`' );
await page.evaluate( () => new Promise( window.requestIdleCallback ) );
await page.keyboard.press( 'Backspace' );

// Expect "`a`" to be restored.
expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'should not undo backtick transform with backspace after typing', async () => {
await clickBlockAppender();
await page.keyboard.type( '`a`' );
await page.evaluate( () => new Promise( window.requestIdleCallback ) );
await page.keyboard.type( 'b' );
await page.keyboard.press( 'Backspace' );
await page.keyboard.press( 'Backspace' );

// Expect "a" to be deleted.
expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'should not undo backtick transform with backspace after selection change', async () => {
await clickBlockAppender();
await page.keyboard.type( '`a`' );
await page.evaluate( () => new Promise( window.requestIdleCallback ) );
// Move inside format boundary.
await page.keyboard.press( 'ArrowLeft' );
await page.keyboard.press( 'ArrowLeft' );
await page.keyboard.press( 'ArrowRight' );
await page.keyboard.press( 'Backspace' );

// Expect "a" to be deleted.
expect( await getEditedPostContent() ).toMatchSnapshot();
} );

it( 'should not format text after code backtick', async () => {
await clickBlockAppender();
await page.keyboard.type( 'A `backtick` and more.' );
Expand Down
Loading

0 comments on commit 6ecff18

Please sign in to comment.