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

Classic Block: fix undo keyboard shortcut #23376

Merged
merged 5 commits into from
Jun 29, 2020
Merged

Conversation

glendaviesnz
Copy link
Contributor

@glendaviesnz glendaviesnz commented Jun 23, 2020

Description

Traps undo keyboard shortcut in Classic box to allow TinyMCE undo stack to work, otherwise cmd-z/ctrl-z causes all classic block content to be deleted in a way that is not re-doable.

Fixes #22797

To Test

Check out PR in local dev en
Add a Classic block
Add several paragraphs in the block
Use cmd-z/ctrl-z undo keyboard shortcuts, and ensure that the Classic block stays in place and just the changes in the block are undo.
Focus another block and check that cmd-z/ctrl-z still works as expected with other blocks

Screenshots

Before:

classic-before

After:

classic-after

Types of changes

Adds a keydown check in classic editor block

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code has proper inline documentation.

@glendaviesnz glendaviesnz added [Type] Bug An existing feature does not function as intended [Block] Classic Affects the Classic Editor Block labels Jun 23, 2020
@glendaviesnz glendaviesnz self-assigned this Jun 23, 2020
@github-actions
Copy link

github-actions bot commented Jun 23, 2020

Size Change: +25 B (0%)

Total Size: 1.13 MB

Filename Size Change
build/block-library/index.js 129 kB +25 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.62 kB 0 B
build/api-fetch/index.js 3.4 kB 0 B
build/autop/index.js 2.82 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 7.39 kB 0 B
build/block-directory/style-rtl.css 941 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 109 kB 0 B
build/block-editor/style-rtl.css 10.7 kB 0 B
build/block-editor/style.css 10.7 kB 0 B
build/block-library/editor-rtl.css 7.58 kB 0 B
build/block-library/editor.css 7.58 kB 0 B
build/block-library/style-rtl.css 8.04 kB 0 B
build/block-library/style.css 8.05 kB 0 B
build/block-library/theme-rtl.css 730 B 0 B
build/block-library/theme.css 732 B 0 B
build/block-serialization-default-parser/index.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 48.2 kB 0 B
build/components/index.js 198 kB 0 B
build/components/style-rtl.css 15.9 kB 0 B
build/components/style.css 15.9 kB 0 B
build/compose/index.js 9.65 kB 0 B
build/core-data/index.js 11.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.44 kB 0 B
build/date/index.js 5.47 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 569 B 0 B
build/dom/index.js 3.19 kB 0 B
build/edit-navigation/index.js 9.87 kB 0 B
build/edit-navigation/style-rtl.css 1.02 kB 0 B
build/edit-navigation/style.css 1.02 kB 0 B
build/edit-post/index.js 303 kB 0 B
build/edit-post/style-rtl.css 5.51 kB 0 B
build/edit-post/style.css 5.5 kB 0 B
build/edit-site/index.js 16.6 kB 0 B
build/edit-site/style-rtl.css 3.03 kB 0 B
build/edit-site/style.css 3.03 kB 0 B
build/edit-widgets/index.js 9.32 kB 0 B
build/edit-widgets/style-rtl.css 2.42 kB 0 B
build/edit-widgets/style.css 2.42 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/index.js 44.8 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.86 kB 0 B
build/element/index.js 4.65 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 7.72 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.13 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 711 B 0 B
build/keyboard-shortcuts/index.js 2.51 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 450 B 0 B
build/list-reusable-blocks/style.css 451 B 0 B
build/media-utils/index.js 5.29 kB 0 B
build/notices/index.js 1.79 kB 0 B
build/nux/index.js 3.4 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 788 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 14 kB 0 B
build/server-side-render/index.js 2.68 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.06 kB 0 B
build/viewport/index.js 1.85 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

@glendaviesnz glendaviesnz marked this pull request as draft June 23, 2020 02:29
@glendaviesnz glendaviesnz changed the title Classic Block: fix undo keyboard shortcut [WIP]Classic Block: fix undo keyboard shortcut Jun 23, 2020
@glendaviesnz glendaviesnz added the [Status] In Progress Tracking issues with work in progress label Jun 23, 2020
@glendaviesnz
Copy link
Contributor Author

Some glitches with this approach when there are other blocks on the page, working on that.

@glendaviesnz glendaviesnz marked this pull request as ready for review June 23, 2020 23:04
@glendaviesnz glendaviesnz removed the [Status] In Progress Tracking issues with work in progress label Jun 23, 2020
@glendaviesnz
Copy link
Contributor Author

glendaviesnz commented Jun 23, 2020

Not sure if it is the best approach to revert back to gutenberg undo if editor is empty, or to prevent the undo from propagating the while the classic block editor is focused whether it is empty or not?

Currently the PR will fall back to gutenberg undo once editor empty, but I think that maybe for anyone using the classic block it is likely to be the only block on the page/post, so trapping all undo keyboard shortcuts in the block may provide the best experience for the user and provide the least potential for data loss?

@glendaviesnz glendaviesnz changed the title [WIP]Classic Block: fix undo keyboard shortcut Classic Block: fix undo keyboard shortcut Jun 24, 2020
@glendaviesnz
Copy link
Contributor Author

glendaviesnz commented Jun 24, 2020

I tried two approaches with this:

  • While the Classic Block is focused trap the undo keyboard shortcuts within the block to prevent any gutenberg undo running. This limits the risk of a user unintentionally removing the block with a cmd-z and not being able to redo to get the content back. It does however mean that if there are other blocks on the page the undo keyboard shortcut won't have any affect on the those until the Classic block is unfocused.
  • The other approach was to revert back to the gutenberg undo when the TinyMCE undo hit an empty editor state. While this allowed a fallback to the gutenberg undo if there were other blocks present without having to unfocus the Classic block, with this approach it is easy to remove the Classic block unintentionally and then not have access to the TinyMCE redo stack to get content back.

Given that users with the Classic block are 99.9% of the time going to be focused on that single block, rather than multiple blocks on the page, I think that limiting the undo keyboard shortcut to the Classic block while focused is the easiest way to prevent this data loss bug.

It may be that we could better integrate the TinyMCE undo stack into the wider gutenberg undo stack, but given that the Classic block is not really intended to be used a part of a mulit-block page layout I don't think it is worth adding extra complexity this might require.

@creativecoder
Copy link
Contributor

Given that users with the Classic block are 99.9% of the time going to be focused on that single block, rather than multiple blocks on the page, I think that limiting the undo keyboard shortcut to the Classic block while focused is the easiest way to prevent this data loss bug.

I agree--keeping the undo keyboard shortcut trapped when the Classic block is focused seems like the best balance of usability while preventing content loss.

Copy link
Contributor

@scruffian scruffian left a comment

Choose a reason for hiding this comment

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

Works as described. I like the solution you went for.

Copy link
Contributor

@ntsekouras ntsekouras left a comment

Choose a reason for hiding this comment

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

Hey @glendaviesnz. Thanks for looking into this!

This LGTM and prevents content loss when using the Classic block.

@glendaviesnz glendaviesnz merged commit 6676de7 into master Jun 29, 2020
@glendaviesnz glendaviesnz deleted the fix/classic-block-undo branch June 29, 2020 21:51
@github-actions github-actions bot added this to the Gutenberg 8.5 milestone Jun 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Classic Affects the Classic Editor Block [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Classic Block removes content that cannot be "redo[ne]" when using keyboard shortcuts [Ctrl/Cmd + Z]
4 participants