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.

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

Writing Flow: allow undo of patterns with BACKSPACE and ESC #14776

Merged
merged 4 commits into from
Aug 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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( () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we polyfill requestIdleCallback? It doesn't seem ready for primetime. Elsewhere in the codebase we have a fallback for it:

const requestIdleCallback = window.requestIdleCallback ? window.requestIdleCallback : window.requestAnimationFrame;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in #17213

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' );

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