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

Add save shortcut for navigation menu screen #21342

Merged
merged 3 commits into from Apr 3, 2020

Conversation

talldan
Copy link
Contributor

@talldan talldan commented Apr 2, 2020

Description

Related #21338

Adds a save shortcut for the navigation menu screen.

It does this via a MenuEditorShortcuts component that might be useful for registering any other shortcuts.

How has this been tested?

  1. Edit a menu on the Navigation Menu page.
  2. Press Cmd+S
  3. Reload the page and observe that change is present

Note - when saving there's no success/error notifications. I've created #21344 to track that.

Types of changes

New feature (non-breaking change which adds functionality)

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] Enhancement A suggestion for improvement. [Block] Navigation Affects the Navigation Block labels Apr 2, 2020
@talldan talldan self-assigned this Apr 2, 2020
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.

Code wise, this looks good (I didn't test).
It would be good to extract at some point the keyboard shortcuts modal to the keyboard shortcuts package in order to reuse in multiple screen. It is still 100% generated dynamically yet though.

return null;
}

MenuEditorShortcuts.Register = RegisterMenuEditorShortcuts;
Copy link
Contributor

Choose a reason for hiding this comment

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

I know we used this pattern in the other packages too, but I wonder if a hook is better than a component for the registration :) (not specific to this PR though)

@github-actions
Copy link

github-actions bot commented Apr 2, 2020

Size Change: +449 B (0%)

Total Size: 884 kB

Filename Size Change
build/block-editor/index.js 102 kB -3 B (0%)
build/block-editor/style-rtl.css 10.7 kB -496 B (4%)
build/block-editor/style.css 10.7 kB -493 B (4%)
build/block-library/editor-rtl.css 7.22 kB +6 B (0%)
build/block-library/editor.css 7.22 kB +7 B (0%)
build/block-library/index.js 110 kB +86 B (0%)
build/block-library/style-rtl.css 7.53 kB +25 B (0%)
build/block-library/style.css 7.54 kB +27 B (0%)
build/components/index.js 195 kB -1 B
build/components/style-rtl.css 16.6 kB +506 B (3%)
build/components/style.css 16.5 kB +499 B (3%)
build/compose/index.js 6.21 kB +1 B
build/core-data/index.js 10.7 kB +19 B (0%)
build/data/index.js 8.23 kB +1 B
build/edit-navigation/index.js 2.71 kB +228 B (8%) 🔍
build/edit-post/index.js 92.3 kB +2 B (0%)
build/edit-post/style-rtl.css 12 kB -9 B (0%)
build/edit-post/style.css 12 kB -9 B (0%)
build/edit-site/index.js 9.12 kB +28 B (0%)
build/edit-site/style-rtl.css 4.61 kB -10 B (0%)
build/edit-site/style.css 4.61 kB -10 B (0%)
build/editor/index.js 42.8 kB -5 B (0%)
build/editor/style-rtl.css 3.49 kB +24 B (0%)
build/editor/style.css 3.49 kB +24 B (0%)
build/notices/index.js 1.57 kB +1 B
build/rich-text/index.js 14.5 kB +2 B (0%)
build/shortcode/index.js 1.69 kB -1 B
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.02 kB 0 B
build/annotations/index.js 3.45 kB 0 B
build/api-fetch/index.js 3.8 kB 0 B
build/autop/index.js 2.59 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 6.03 kB 0 B
build/block-directory/style-rtl.css 760 B 0 B
build/block-directory/style.css 760 B 0 B
build/block-library/theme-rtl.css 669 B 0 B
build/block-library/theme.css 671 B 0 B
build/block-serialization-default-parser/index.js 1.65 kB 0 B
build/block-serialization-spec-parser/index.js 3.1 kB 0 B
build/blocks/index.js 57.5 kB 0 B
build/data-controls/index.js 1.03 kB 0 B
build/date/index.js 5.37 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.05 kB 0 B
build/edit-navigation/style-rtl.css 239 B 0 B
build/edit-navigation/style.css 241 B 0 B
build/edit-widgets/index.js 4.43 kB 0 B
build/edit-widgets/style-rtl.css 3.74 kB 0 B
build/edit-widgets/style.css 3.74 kB 0 B
build/editor/editor-styles-rtl.css 423 B 0 B
build/editor/editor-styles.css 426 B 0 B
build/element/index.js 4.44 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/index.js 6.95 kB 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/hooks/index.js 1.93 kB 0 B
build/html-entities/index.js 622 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.3 kB 0 B
build/keycodes/index.js 1.7 kB 0 B
build/list-reusable-blocks/index.js 2.99 kB 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/media-utils/index.js 4.84 kB 0 B
build/nux/index.js 3.01 kB 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/plugins/index.js 2.54 kB 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 780 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/server-side-render/index.js 2.54 kB 0 B
build/token-list/index.js 1.28 kB 0 B
build/url/index.js 4.01 kB 0 B
build/viewport/index.js 1.6 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.17 kB 0 B

compressed-size-action

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.

Tested and worked as described.

@talldan
Copy link
Contributor Author

talldan commented Apr 2, 2020

I need to update the package-lock file by the looks of it. Will do that tomorrow and then merge. Thanks for the speedy reviews!

@talldan talldan merged commit 052c835 into master Apr 3, 2020
@talldan talldan deleted the add/navigation-page-save-shortcut branch April 3, 2020 08:08
@github-actions github-actions bot added this to the Gutenberg 7.9 milestone Apr 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants