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

Conversation

ellatrix
Copy link
Member

@ellatrix ellatrix commented Apr 2, 2019

Description

Fixes #8942.
Fixes #16649.
Fixes #17038.

Sets a flag when an automatic change (input rule) happened, so it can be used to undo when Backspace, Delete or Escape is pressed. The flag is removed as soon any action follows.

This will hopefully alleviate the frustration when something automatically changes based on an input rule. It doesn't restore the selection though, only the content. For that, we'll need #16428.

How has this been tested?

See e2e tests.

Screenshots

Types of changes

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@ellatrix ellatrix added [Status] In Progress Tracking issues with work in progress [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable [Feature] History History, undo, redo, revisions, autosave. labels Apr 2, 2019
@ellatrix ellatrix added [Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... and removed [Status] In Progress Tracking issues with work in progress labels Apr 2, 2019
@ellatrix ellatrix force-pushed the try/rich-text-pattern-undo-backspace branch 2 times, most recently from 1408408 to 02b7dc7 Compare April 3, 2019 09:15
@draganescu
Copy link
Contributor

Hi @ellatrix I have tested the branch and it worked but it behaved in a strange way:

undo-with-backspace-focus-lost

When the Undo is applied the focus is lost and I can't continue typing.

@draganescu draganescu assigned draganescu and unassigned draganescu Apr 3, 2019
@ellatrix
Copy link
Member Author

ellatrix commented Apr 3, 2019

@draganescu That's "normal". We don't have the ability yet to restore selection after undo, for that we'll need #14640. :) To test: try to undo anything in the editor, like typing a word, and you'll see that selection will be lost.

Thanks for reviewing!

@ellatrix ellatrix added the Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code label Apr 3, 2019
@ellatrix ellatrix force-pushed the try/rich-text-pattern-undo-backspace branch from 02b7dc7 to b17757a Compare April 4, 2019 07:40
Copy link
Contributor

@draganescu draganescu left a comment

Choose a reason for hiding this comment

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

@ellatrix you are correct, that is the "normal" behavior, which makes this change set good to go. There are some conflicts though, but that it all

@ellatrix ellatrix force-pushed the try/rich-text-pattern-undo-backspace branch from b17757a to 72da05d Compare April 12, 2019 12:11
@ellatrix ellatrix added the [Type] Bug An existing feature does not function as intended label Apr 12, 2019
@ellatrix ellatrix force-pushed the try/rich-text-pattern-undo-backspace branch from 72da05d to f0d654a Compare April 24, 2019 11:49
@ellatrix ellatrix added the [Status] In Progress Tracking issues with work in progress label Apr 24, 2019
@mapk
Copy link
Contributor

mapk commented Jul 24, 2019

I'm really hoping this is the solution for #16649, but it appears there are some conflicts. Can this get rebased? I'd love to test against #16649.

@ellatrix
Copy link
Member Author

Last time I checked I ran into some issues. I'll have to look into this again.

@ellatrix ellatrix force-pushed the try/rich-text-pattern-undo-backspace branch from f0d654a to 3a06020 Compare August 21, 2019 19:11
@ellatrix ellatrix force-pushed the try/rich-text-pattern-undo-backspace branch from 3a06020 to f4019b0 Compare August 21, 2019 19:34
@ellatrix ellatrix removed the [Status] In Progress Tracking issues with work in progress label Aug 21, 2019
@ellatrix ellatrix force-pushed the try/rich-text-pattern-undo-backspace branch from be27d13 to 5cfbb51 Compare August 22, 2019 07:12
@ellatrix ellatrix changed the title RichText/Patterns/Input Interaction: allow undo of patterns with BACKSPACE and ESC Writing Flow: allow undo of patterns with BACKSPACE and ESC Aug 22, 2019
@ellatrix
Copy link
Member Author

When the Undo is applied the focus is lost and I can't continue typing.

For that we'll need #16428, which is ready for final review.

@ellatrix ellatrix merged commit 6ecff18 into master Aug 22, 2019
@ellatrix ellatrix deleted the try/rich-text-pattern-undo-backspace branch August 22, 2019 07:47
@ellatrix ellatrix added this to the Gutenberg 6.4 milestone Aug 22, 2019
@mapk
Copy link
Contributor

mapk commented Aug 23, 2019

🎉 😄

donmhico pushed a commit to donmhico/gutenberg that referenced this pull request Aug 27, 2019
…s#14776)

* RichText/Patterns/Input Interaction: allow undo of patterns with BACKSPACE and ESC

* Only run input rules when inserting text

* Fix typo

* Simplify
* 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

gziolo pushed a commit that referenced this pull request Aug 29, 2019
* RichText/Patterns/Input Interaction: allow undo of patterns with BACKSPACE and ESC

* Only run input rules when inserting text

* Fix typo

* Simplify
gziolo pushed a commit that referenced this pull request Aug 29, 2019
* RichText/Patterns/Input Interaction: allow undo of patterns with BACKSPACE and ESC

* Only run input rules when inserting text

* Fix typo

* Simplify
daniloercoli added a commit that referenced this pull request Sep 4, 2019
 into rnmobile/add-autosave-to-mobile

* 'rnmobile/master' of https://github.com/WordPress/gutenberg: (52 commits)
  [RNMobile] DarkMode improvements (#17309)
  Remove redundant bg color within button appender (#17325)
  Support group block on mobile (#17251)
  [RNMobile] Insure tapping at end of post inserts at end
  Recover border colors (#17269)
  [RNMobile] Fix dismiss keyboard button for the post title (#17260)
  Unify media placeholder and upload props within media-text (#17268)
  MediaUpload and MediaPlaceholder unify props (#17145)
  Add native support for the MediaText block (#16305)
  Activate Travis CI on rnmobile/master branch (#17229)
  [RNMobile] Native mobile release v1.11.0 (#17181)
  Apply box-sizing border-box properly to the notices components (#17066)
  Writing Flow: allow undo of patterns with BACKSPACE and ESC (#14776)
  Project automation: Rewrite actions using JavaScript (#17080)
  Build: remove global install of latest npm since we want to use the paired node/npm version (#17134)
  Writing Flow/Quote: allow splitting (#17121)
  Use `400` as a valid `font-weight`
  Add: Disabled block count in the block manager (#17103)
  Update video player style on mobile - Add a new gridicon play icon, from: https://github.com/Automattic/gridicons/blob/87c9fce08b4a9f184b9fb4963228757fdd4f4e74/svg-min/gridicons-play.svg - Replace the Dashicon play by this one - Update icon size and icon color - Update the overlay color
  [RNMobile] Hide replaceable block when adding block (#16931)
  ...

# Conflicts:
#	packages/block-editor/src/components/block-list/index.native.js
#	packages/block-editor/src/components/inserter/index.native.js
#	packages/block-editor/src/components/inserter/menu.native.js
#	packages/block-editor/src/components/media-placeholder/index.native.js
#	packages/block-editor/src/components/warning/index.native.js
#	packages/block-library/src/code/edit.native.js
#	packages/block-library/src/image/edit.native.js
#	packages/block-library/src/missing/edit.native.js
#	packages/block-library/src/more/edit.native.js
#	packages/block-library/src/nextpage/edit.native.js
#	packages/block-library/src/video/edit.native.js
#	packages/components/src/mobile/bottom-sheet/cell.native.js
#	packages/components/src/mobile/bottom-sheet/index.native.js
#	packages/components/src/mobile/dark-mode/index.native.js
#	packages/components/src/mobile/html-text-input/index.native.js
#	packages/components/src/toolbar/toolbar-container.native.js
#	packages/edit-post/src/components/header/header-toolbar/index.native.js
#	packages/edit-post/src/components/layout/index.native.js
#	packages/edit-post/src/components/visual-editor/index.native.js
#	packages/rich-text/src/component/index.native.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] History History, undo, redo, revisions, autosave. [Feature] Rich Text Related to the Rich Text component that allows developers to render a contenteditable [Feature] Writing Flow Block selection, navigation, splitting, merging, deletion... Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code [Type] Bug An existing feature does not function as intended
Projects
None yet
4 participants