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

Fix save shortcut in code editor #24151

Merged
merged 6 commits into from
Jul 27, 2020
Merged

Conversation

talldan
Copy link
Contributor

@talldan talldan commented Jul 23, 2020

Description

Fixes #24054

Edits made in the code editor were not being saved when using the shortcut (Cmd+S or Ctrl+S).

The issue appears to be that edits are only persisted (resetEditorBlocks called) when the text area is blurred in the code editor:
https://github.com/WordPress/gutenberg/blob/master/packages/editor/src/components/post-text-editor/index.js#L82

When using the save shortcut, that blur event wouldn't be called, so the code editor changes wouldn't be saved.

This change makes the save shortcut for the code editor also call resetEditorBlocks with the relevant changes to ensure that the changes are prepared for saving.

This meant working with code I've not looked at for a while, so I'm open to suggestions on improvements.

Another bit of clean up is removing the duplicate registration of the save shortcut in the VisualEditorShortcuts component.

How has this been tested?

Added e2e tests

  1. Add three paragraphs with the text 'First paragraph' in the first, 'Second paragraph' in the second, 'Third paragraph' in the third ...
  2. switch to Code editor via the keyboard shortcuts, on macOS: Shift+Alt+Cmd+M
  3. manually edit the third paragraph text and change it to "Hello"
  4. save as draft by pressing Cmd+S
  5. switch back to Visual editor by pressing Shift+Alt+Cmd+M
  6. observe the third paragraph content is now "Hello"
  7. Previously it would still be 'Third paragraph'

Types of changes

Bug fix (non-breaking change which fixes an issue)

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.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@talldan talldan added [Type] Bug An existing feature does not function as intended [Feature] Code Editor Handling the code view of the editing experience labels Jul 23, 2020
@talldan talldan self-assigned this Jul 23, 2020
@github-actions
Copy link

github-actions bot commented Jul 23, 2020

Size Change: +55 B (0%)

Total Size: 1.15 MB

Filename Size Change
build/editor/index.min.js 45.3 kB +55 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.min.js 1.14 kB 0 B
build/annotations/index.min.js 3.67 kB 0 B
build/api-fetch/index.min.js 3.43 kB 0 B
build/autop/index.min.js 2.82 kB 0 B
build/blob/index.min.js 620 B 0 B
build/block-directory/index.min.js 7.63 kB 0 B
build/block-directory/style-rtl.css 953 B 0 B
build/block-directory/style.css 952 B 0 B
build/block-editor/index.min.js 125 kB 0 B
build/block-editor/style-rtl.css 10.8 kB 0 B
build/block-editor/style.css 10.8 kB 0 B
build/block-library/editor-rtl.css 7.63 kB 0 B
build/block-library/editor.css 7.63 kB 0 B
build/block-library/index.min.js 132 kB 0 B
build/block-library/style-rtl.css 7.83 kB 0 B
build/block-library/style.css 7.83 kB 0 B
build/block-library/theme-rtl.css 728 B 0 B
build/block-library/theme.css 729 B 0 B
build/block-serialization-default-parser/index.min.js 1.88 kB 0 B
build/block-serialization-spec-parser/index.min.js 3.1 kB 0 B
build/blocks/index.min.js 48.3 kB 0 B
build/components/index.min.js 200 kB 0 B
build/components/style-rtl.css 15.7 kB 0 B
build/components/style.css 15.6 kB 0 B
build/compose/index.min.js 9.68 kB 0 B
build/core-data/index.min.js 11.5 kB 0 B
build/data-controls/index.min.js 1.29 kB 0 B
build/data/index.min.js 8.45 kB 0 B
build/date/index.min.js 5.38 kB 0 B
build/deprecated/index.min.js 772 B 0 B
build/dom-ready/index.min.js 568 B 0 B
build/dom/index.min.js 3.23 kB 0 B
build/edit-navigation/index.min.js 10.8 kB 0 B
build/edit-navigation/style-rtl.css 1.08 kB 0 B
build/edit-navigation/style.css 1.08 kB 0 B
build/edit-post/index.min.js 304 kB 0 B
build/edit-post/style-rtl.css 5.61 kB 0 B
build/edit-post/style.css 5.61 kB 0 B
build/edit-site/index.min.js 16.9 kB 0 B
build/edit-site/style-rtl.css 3.06 kB 0 B
build/edit-site/style.css 3.06 kB 0 B
build/edit-widgets/index.min.js 9.37 kB 0 B
build/edit-widgets/style-rtl.css 2.45 kB 0 B
build/edit-widgets/style.css 2.45 kB 0 B
build/editor/editor-styles-rtl.css 537 B 0 B
build/editor/editor-styles.css 539 B 0 B
build/editor/style-rtl.css 3.78 kB 0 B
build/editor/style.css 3.78 kB 0 B
build/element/index.min.js 4.65 kB 0 B
build/escape-html/index.min.js 733 B 0 B
build/format-library/index.min.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.min.js 2.13 kB 0 B
build/html-entities/index.min.js 621 B 0 B
build/i18n/index.min.js 3.56 kB 0 B
build/is-shallow-equal/index.min.js 711 B 0 B
build/keyboard-shortcuts/index.min.js 2.51 kB 0 B
build/keycodes/index.min.js 1.94 kB 0 B
build/list-reusable-blocks/index.min.js 3.12 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.min.js 5.33 kB 0 B
build/notices/index.min.js 1.79 kB 0 B
build/nux/index.min.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.min.js 2.56 kB 0 B
build/primitives/index.min.js 1.4 kB 0 B
build/priority-queue/index.min.js 789 B 0 B
build/redux-routine/index.min.js 2.85 kB 0 B
build/rich-text/index.min.js 13.9 kB 0 B
build/server-side-render/index.min.js 2.71 kB 0 B
build/shortcode/index.min.js 1.7 kB 0 B
build/token-list/index.min.js 1.27 kB 0 B
build/url/index.min.js 4.06 kB 0 B
build/viewport/index.min.js 1.85 kB 0 B
build/warning/index.min.js 1.14 kB 0 B
build/wordcount/index.min.js 1.17 kB 0 B

compressed-size-action

@talldan
Copy link
Contributor Author

talldan commented Jul 23, 2020

I tested WP 5.4.2 and this does seem to have also been an issue in that release.

@talldan talldan added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Jul 24, 2020
@talldan
Copy link
Contributor Author

talldan commented Jul 24, 2020

I'm labelling as backport to wp core. While this isn't something that has regressed from 5.4.2, I think the content loss aspect of this bug makes it particularly bad. Especially if the user has made lots of edits or complicated edits in the Code Editor mode. If this is something that we can get into WP5.5, that'd be great.

@talldan talldan force-pushed the fix/save-shortcut-in-code-editor branch from 791d604 to 3667c51 Compare July 24, 2020 04:49
Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Thanks for the test.

@youknowriad youknowriad merged commit 6954857 into master Jul 27, 2020
@youknowriad youknowriad deleted the fix/save-shortcut-in-code-editor branch July 27, 2020 08:12
@github-actions github-actions bot added this to the Gutenberg 8.7 milestone Jul 27, 2020
youknowriad pushed a commit that referenced this pull request Jul 27, 2020
@youknowriad youknowriad removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Jul 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Code Editor Handling the code view of the editing experience [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Content not saved correctly when switching visual / code editor modes via keyboard shortcuts
2 participants