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

Navigation: Add the navigation selector to the block toolbar #54214

Closed
wants to merge 3 commits into from

Conversation

scruffian
Copy link
Contributor

@scruffian scruffian commented Sep 6, 2023

What?

This surfaces the interface for changing the active navigation to the block toolbar.

Closes #37068

Why?

This makes it easier for users to change the currently active navigation in the block.

How?

  • Adds the component to <BlockControls>
  • Adds three new props to the NavigationMenuSelector component so that we can reuse the same component.

Testing Instructions

  1. Add a Navigation block
  2. Select the block
  3. Confirm that you see a "Change" button in the toolbar and that it allows you to change the currently active navigation and create a new one.

Testing Instructions for Keyboard

  1. Insert/select a navigation block
  2. Press alt+F10 to move to the block toolbar
  3. Use the arrow keys to select the "Change" button
  4. Press enter to open the Drop down
  5. Press arrow down to select a different navigation
  6. Press enter to make the change

Screenshots or screencast

Screenshot 2023-09-06 at 11 47 49

Notes

This needs some design feedback:

  1. Is the chevron the correct icon?
  2. Is "Change" the correct text?

@scruffian scruffian added Needs Design Feedback Needs general design feedback. [Block] Navigation Affects the Navigation Block labels Sep 6, 2023
@scruffian scruffian self-assigned this Sep 6, 2023
@github-actions
Copy link

github-actions bot commented Sep 6, 2023

Size Change: +10.1 kB (+1%)

Total Size: 1.53 MB

Filename Size Change
build/block-editor/index.min.js 216 kB +1.23 kB (+1%)
build/block-editor/style-rtl.css 15.3 kB +247 B (+2%)
build/block-editor/style.css 15.3 kB +251 B (+2%)
build/block-library/index.min.js 204 kB +266 B (0%)
build/blocks/index.min.js 51.4 kB -1 B (0%)
build/commands/index.min.js 15.5 kB +4 B (0%)
build/components/index.min.js 255 kB +7.68 kB (+3%)
build/components/style-rtl.css 11.7 kB -69 B (-1%)
build/components/style.css 11.7 kB -68 B (-1%)
build/core-data/index.min.js 16.8 kB +21 B (0%)
build/customize-widgets/index.min.js 12 kB -20 B (0%)
build/customize-widgets/style-rtl.css 1.48 kB +24 B (+2%)
build/customize-widgets/style.css 1.48 kB +26 B (+2%)
build/data/index.min.js 8.84 kB +456 B (+5%) 🔍
build/date/index.min.js 17.8 kB +30 B (0%)
build/edit-post/index.min.js 35.3 kB -41 B (0%)
build/edit-post/style-rtl.css 7.6 kB -26 B (0%)
build/edit-post/style.css 7.59 kB -25 B (0%)
build/edit-site/index.min.js 90.9 kB +63 B (0%)
build/edit-site/style-rtl.css 13.2 kB -23 B (0%)
build/edit-site/style.css 13.2 kB -22 B (0%)
build/edit-widgets/index.min.js 16.9 kB -17 B (0%)
build/format-library/index.min.js 7.71 kB -23 B (0%)
build/interactivity/index.min.js 11.3 kB +15 B (0%)
build/style-engine/index.min.js 1.97 kB +126 B (+7%) 🔍
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 955 B
build/annotations/index.min.js 2.69 kB
build/api-fetch/index.min.js 2.28 kB
build/autop/index.min.js 2.1 kB
build/blob/index.min.js 451 B
build/block-directory/index.min.js 7.01 kB
build/block-directory/style-rtl.css 1.02 kB
build/block-directory/style.css 1.02 kB
build/block-editor/content-rtl.css 4.25 kB
build/block-editor/content.css 4.24 kB
build/block-editor/default-editor-styles-rtl.css 381 B
build/block-editor/default-editor-styles.css 381 B
build/block-library/blocks/archives/editor-rtl.css 61 B
build/block-library/blocks/archives/editor.css 60 B
build/block-library/blocks/archives/style-rtl.css 90 B
build/block-library/blocks/archives/style.css 90 B
build/block-library/blocks/audio/editor-rtl.css 150 B
build/block-library/blocks/audio/editor.css 150 B
build/block-library/blocks/audio/style-rtl.css 122 B
build/block-library/blocks/audio/style.css 122 B
build/block-library/blocks/audio/theme-rtl.css 126 B
build/block-library/blocks/audio/theme.css 126 B
build/block-library/blocks/avatar/editor-rtl.css 116 B
build/block-library/blocks/avatar/editor.css 116 B
build/block-library/blocks/avatar/style-rtl.css 104 B
build/block-library/blocks/avatar/style.css 104 B
build/block-library/blocks/block/editor-rtl.css 305 B
build/block-library/blocks/block/editor.css 305 B
build/block-library/blocks/button/editor-rtl.css 584 B
build/block-library/blocks/button/editor.css 582 B
build/block-library/blocks/button/style-rtl.css 629 B
build/block-library/blocks/button/style.css 628 B
build/block-library/blocks/buttons/editor-rtl.css 337 B
build/block-library/blocks/buttons/editor.css 337 B
build/block-library/blocks/buttons/style-rtl.css 332 B
build/block-library/blocks/buttons/style.css 332 B
build/block-library/blocks/calendar/style-rtl.css 239 B
build/block-library/blocks/calendar/style.css 239 B
build/block-library/blocks/categories/editor-rtl.css 113 B
build/block-library/blocks/categories/editor.css 112 B
build/block-library/blocks/categories/style-rtl.css 124 B
build/block-library/blocks/categories/style.css 124 B
build/block-library/blocks/code/editor-rtl.css 53 B
build/block-library/blocks/code/editor.css 53 B
build/block-library/blocks/code/style-rtl.css 121 B
build/block-library/blocks/code/style.css 121 B
build/block-library/blocks/code/theme-rtl.css 124 B
build/block-library/blocks/code/theme.css 124 B
build/block-library/blocks/columns/editor-rtl.css 108 B
build/block-library/blocks/columns/editor.css 108 B
build/block-library/blocks/columns/style-rtl.css 421 B
build/block-library/blocks/columns/style.css 421 B
build/block-library/blocks/comment-author-avatar/editor-rtl.css 125 B
build/block-library/blocks/comment-author-avatar/editor.css 125 B
build/block-library/blocks/comment-content/style-rtl.css 92 B
build/block-library/blocks/comment-content/style.css 92 B
build/block-library/blocks/comment-template/style-rtl.css 199 B
build/block-library/blocks/comment-template/style.css 198 B
build/block-library/blocks/comments-pagination-numbers/editor-rtl.css 123 B
build/block-library/blocks/comments-pagination-numbers/editor.css 121 B
build/block-library/blocks/comments-pagination/editor-rtl.css 222 B
build/block-library/blocks/comments-pagination/editor.css 209 B
build/block-library/blocks/comments-pagination/style-rtl.css 235 B
build/block-library/blocks/comments-pagination/style.css 231 B
build/block-library/blocks/comments-title/editor-rtl.css 75 B
build/block-library/blocks/comments-title/editor.css 75 B
build/block-library/blocks/comments/editor-rtl.css 840 B
build/block-library/blocks/comments/editor.css 839 B
build/block-library/blocks/comments/style-rtl.css 637 B
build/block-library/blocks/comments/style.css 636 B
build/block-library/blocks/cover/editor-rtl.css 647 B
build/block-library/blocks/cover/editor.css 650 B
build/block-library/blocks/cover/style-rtl.css 1.63 kB
build/block-library/blocks/cover/style.css 1.62 kB
build/block-library/blocks/details/editor-rtl.css 65 B
build/block-library/blocks/details/editor.css 65 B
build/block-library/blocks/details/style-rtl.css 98 B
build/block-library/blocks/details/style.css 98 B
build/block-library/blocks/embed/editor-rtl.css 293 B
build/block-library/blocks/embed/editor.css 293 B
build/block-library/blocks/embed/style-rtl.css 410 B
build/block-library/blocks/embed/style.css 410 B
build/block-library/blocks/embed/theme-rtl.css 126 B
build/block-library/blocks/embed/theme.css 126 B
build/block-library/blocks/file/editor-rtl.css 316 B
build/block-library/blocks/file/editor.css 316 B
build/block-library/blocks/file/style-rtl.css 280 B
build/block-library/blocks/file/style.css 281 B
build/block-library/blocks/file/view-interactivity.min.js 317 B
build/block-library/blocks/file/view.min.js 375 B
build/block-library/blocks/footnotes/style-rtl.css 201 B
build/block-library/blocks/footnotes/style.css 199 B
build/block-library/blocks/freeform/editor-rtl.css 2.61 kB
build/block-library/blocks/freeform/editor.css 2.61 kB
build/block-library/blocks/gallery/editor-rtl.css 947 B
build/block-library/blocks/gallery/editor.css 952 B
build/block-library/blocks/gallery/style-rtl.css 1.53 kB
build/block-library/blocks/gallery/style.css 1.53 kB
build/block-library/blocks/gallery/theme-rtl.css 108 B
build/block-library/blocks/gallery/theme.css 108 B
build/block-library/blocks/group/editor-rtl.css 654 B
build/block-library/blocks/group/editor.css 654 B
build/block-library/blocks/group/style-rtl.css 57 B
build/block-library/blocks/group/style.css 57 B
build/block-library/blocks/group/theme-rtl.css 78 B
build/block-library/blocks/group/theme.css 78 B
build/block-library/blocks/heading/style-rtl.css 76 B
build/block-library/blocks/heading/style.css 76 B
build/block-library/blocks/html/editor-rtl.css 336 B
build/block-library/blocks/html/editor.css 337 B
build/block-library/blocks/image/editor-rtl.css 834 B
build/block-library/blocks/image/editor.css 833 B
build/block-library/blocks/image/style-rtl.css 1.42 kB
build/block-library/blocks/image/style.css 1.41 kB
build/block-library/blocks/image/theme-rtl.css 126 B
build/block-library/blocks/image/theme.css 126 B
build/block-library/blocks/image/view-interactivity.min.js 1.83 kB
build/block-library/blocks/latest-comments/style-rtl.css 357 B
build/block-library/blocks/latest-comments/style.css 357 B
build/block-library/blocks/latest-posts/editor-rtl.css 213 B
build/block-library/blocks/latest-posts/editor.css 212 B
build/block-library/blocks/latest-posts/style-rtl.css 478 B
build/block-library/blocks/latest-posts/style.css 478 B
build/block-library/blocks/list/style-rtl.css 88 B
build/block-library/blocks/list/style.css 88 B
build/block-library/blocks/media-text/editor-rtl.css 266 B
build/block-library/blocks/media-text/editor.css 263 B
build/block-library/blocks/media-text/style-rtl.css 505 B
build/block-library/blocks/media-text/style.css 503 B
build/block-library/blocks/more/editor-rtl.css 431 B
build/block-library/blocks/more/editor.css 431 B
build/block-library/blocks/navigation-link/editor-rtl.css 712 B
build/block-library/blocks/navigation-link/editor.css 711 B
build/block-library/blocks/navigation-link/style-rtl.css 115 B
build/block-library/blocks/navigation-link/style.css 115 B
build/block-library/blocks/navigation-submenu/editor-rtl.css 296 B
build/block-library/blocks/navigation-submenu/editor.css 295 B
build/block-library/blocks/navigation/editor-rtl.css 2.26 kB
build/block-library/blocks/navigation/editor.css 2.26 kB
build/block-library/blocks/navigation/style-rtl.css 2.23 kB
build/block-library/blocks/navigation/style.css 2.22 kB
build/block-library/blocks/navigation/view-interactivity.min.js 988 B
build/block-library/blocks/navigation/view-modal.min.js 2.85 kB
build/block-library/blocks/navigation/view.min.js 469 B
build/block-library/blocks/nextpage/editor-rtl.css 395 B
build/block-library/blocks/nextpage/editor.css 395 B
build/block-library/blocks/page-list/editor-rtl.css 401 B
build/block-library/blocks/page-list/editor.css 401 B
build/block-library/blocks/page-list/style-rtl.css 175 B
build/block-library/blocks/page-list/style.css 175 B
build/block-library/blocks/paragraph/editor-rtl.css 235 B
build/block-library/blocks/paragraph/editor.css 235 B
build/block-library/blocks/paragraph/style-rtl.css 335 B
build/block-library/blocks/paragraph/style.css 335 B
build/block-library/blocks/post-author/style-rtl.css 175 B
build/block-library/blocks/post-author/style.css 176 B
build/block-library/blocks/post-comments-form/editor-rtl.css 96 B
build/block-library/blocks/post-comments-form/editor.css 96 B
build/block-library/blocks/post-comments-form/style-rtl.css 508 B
build/block-library/blocks/post-comments-form/style.css 508 B
build/block-library/blocks/post-date/style-rtl.css 61 B
build/block-library/blocks/post-date/style.css 61 B
build/block-library/blocks/post-excerpt/editor-rtl.css 71 B
build/block-library/blocks/post-excerpt/editor.css 71 B
build/block-library/blocks/post-excerpt/style-rtl.css 141 B
build/block-library/blocks/post-excerpt/style.css 141 B
build/block-library/blocks/post-featured-image/editor-rtl.css 588 B
build/block-library/blocks/post-featured-image/editor.css 586 B
build/block-library/blocks/post-featured-image/style-rtl.css 319 B
build/block-library/blocks/post-featured-image/style.css 319 B
build/block-library/blocks/post-navigation-link/style-rtl.css 215 B
build/block-library/blocks/post-navigation-link/style.css 214 B
build/block-library/blocks/post-template/editor-rtl.css 99 B
build/block-library/blocks/post-template/editor.css 98 B
build/block-library/blocks/post-template/style-rtl.css 314 B
build/block-library/blocks/post-template/style.css 314 B
build/block-library/blocks/post-terms/style-rtl.css 96 B
build/block-library/blocks/post-terms/style.css 96 B
build/block-library/blocks/post-time-to-read/style-rtl.css 69 B
build/block-library/blocks/post-time-to-read/style.css 69 B
build/block-library/blocks/post-title/style-rtl.css 100 B
build/block-library/blocks/post-title/style.css 100 B
build/block-library/blocks/preformatted/style-rtl.css 125 B
build/block-library/blocks/preformatted/style.css 125 B
build/block-library/blocks/pullquote/editor-rtl.css 135 B
build/block-library/blocks/pullquote/editor.css 135 B
build/block-library/blocks/pullquote/style-rtl.css 335 B
build/block-library/blocks/pullquote/style.css 335 B
build/block-library/blocks/pullquote/theme-rtl.css 168 B
build/block-library/blocks/pullquote/theme.css 168 B
build/block-library/blocks/query-pagination-numbers/editor-rtl.css 122 B
build/block-library/blocks/query-pagination-numbers/editor.css 121 B
build/block-library/blocks/query-pagination/editor-rtl.css 221 B
build/block-library/blocks/query-pagination/editor.css 211 B
build/block-library/blocks/query-pagination/style-rtl.css 302 B
build/block-library/blocks/query-pagination/style.css 299 B
build/block-library/blocks/query-title/style-rtl.css 63 B
build/block-library/blocks/query-title/style.css 63 B
build/block-library/blocks/query/editor-rtl.css 450 B
build/block-library/blocks/query/editor.css 449 B
build/block-library/blocks/query/style-rtl.css 370 B
build/block-library/blocks/query/style.css 368 B
build/block-library/blocks/query/view.min.js 559 B
build/block-library/blocks/quote/style-rtl.css 222 B
build/block-library/blocks/quote/style.css 222 B
build/block-library/blocks/quote/theme-rtl.css 223 B
build/block-library/blocks/quote/theme.css 226 B
build/block-library/blocks/read-more/style-rtl.css 132 B
build/block-library/blocks/read-more/style.css 132 B
build/block-library/blocks/rss/editor-rtl.css 149 B
build/block-library/blocks/rss/editor.css 149 B
build/block-library/blocks/rss/style-rtl.css 289 B
build/block-library/blocks/rss/style.css 288 B
build/block-library/blocks/search/editor-rtl.css 178 B
build/block-library/blocks/search/editor.css 178 B
build/block-library/blocks/search/style-rtl.css 607 B
build/block-library/blocks/search/style.css 607 B
build/block-library/blocks/search/theme-rtl.css 114 B
build/block-library/blocks/search/theme.css 114 B
build/block-library/blocks/search/view.min.js 631 B
build/block-library/blocks/separator/editor-rtl.css 146 B
build/block-library/blocks/separator/editor.css 146 B
build/block-library/blocks/separator/style-rtl.css 234 B
build/block-library/blocks/separator/style.css 234 B
build/block-library/blocks/separator/theme-rtl.css 194 B
build/block-library/blocks/separator/theme.css 194 B
build/block-library/blocks/shortcode/editor-rtl.css 323 B
build/block-library/blocks/shortcode/editor.css 323 B
build/block-library/blocks/site-logo/editor-rtl.css 754 B
build/block-library/blocks/site-logo/editor.css 754 B
build/block-library/blocks/site-logo/style-rtl.css 204 B
build/block-library/blocks/site-logo/style.css 204 B
build/block-library/blocks/site-tagline/editor-rtl.css 86 B
build/block-library/blocks/site-tagline/editor.css 86 B
build/block-library/blocks/site-title/editor-rtl.css 116 B
build/block-library/blocks/site-title/editor.css 116 B
build/block-library/blocks/site-title/style-rtl.css 57 B
build/block-library/blocks/site-title/style.css 57 B
build/block-library/blocks/social-link/editor-rtl.css 184 B
build/block-library/blocks/social-link/editor.css 184 B
build/block-library/blocks/social-links/editor-rtl.css 682 B
build/block-library/blocks/social-links/editor.css 681 B
build/block-library/blocks/social-links/style-rtl.css 1.44 kB
build/block-library/blocks/social-links/style.css 1.43 kB
build/block-library/blocks/spacer/editor-rtl.css 348 B
build/block-library/blocks/spacer/editor.css 348 B
build/block-library/blocks/spacer/style-rtl.css 48 B
build/block-library/blocks/spacer/style.css 48 B
build/block-library/blocks/table/editor-rtl.css 432 B
build/block-library/blocks/table/editor.css 432 B
build/block-library/blocks/table/style-rtl.css 639 B
build/block-library/blocks/table/style.css 639 B
build/block-library/blocks/table/theme-rtl.css 146 B
build/block-library/blocks/table/theme.css 146 B
build/block-library/blocks/tag-cloud/style-rtl.css 251 B
build/block-library/blocks/tag-cloud/style.css 253 B
build/block-library/blocks/template-part/editor-rtl.css 403 B
build/block-library/blocks/template-part/editor.css 403 B
build/block-library/blocks/template-part/theme-rtl.css 101 B
build/block-library/blocks/template-part/theme.css 101 B
build/block-library/blocks/term-description/style-rtl.css 111 B
build/block-library/blocks/term-description/style.css 111 B
build/block-library/blocks/text-columns/editor-rtl.css 95 B
build/block-library/blocks/text-columns/editor.css 95 B
build/block-library/blocks/text-columns/style-rtl.css 166 B
build/block-library/blocks/text-columns/style.css 166 B
build/block-library/blocks/verse/style-rtl.css 99 B
build/block-library/blocks/verse/style.css 99 B
build/block-library/blocks/video/editor-rtl.css 552 B
build/block-library/blocks/video/editor.css 555 B
build/block-library/blocks/video/style-rtl.css 185 B
build/block-library/blocks/video/style.css 185 B
build/block-library/blocks/video/theme-rtl.css 126 B
build/block-library/blocks/video/theme.css 126 B
build/block-library/classic-rtl.css 179 B
build/block-library/classic.css 179 B
build/block-library/common-rtl.css 1.1 kB
build/block-library/common.css 1.1 kB
build/block-library/editor-elements-rtl.css 75 B
build/block-library/editor-elements.css 75 B
build/block-library/editor-rtl.css 12.2 kB
build/block-library/editor.css 12.1 kB
build/block-library/elements-rtl.css 54 B
build/block-library/elements.css 54 B
build/block-library/reset-rtl.css 478 B
build/block-library/reset.css 478 B
build/block-library/style-rtl.css 13.8 kB
build/block-library/style.css 13.8 kB
build/block-library/theme-rtl.css 688 B
build/block-library/theme.css 693 B
build/block-serialization-default-parser/index.min.js 1.12 kB
build/block-serialization-spec-parser/index.min.js 2.87 kB
build/commands/style-rtl.css 932 B
build/commands/style.css 929 B
build/compose/index.min.js 12.1 kB
build/core-commands/index.min.js 2.6 kB
build/data-controls/index.min.js 640 B
build/deprecated/index.min.js 451 B
build/dom-ready/index.min.js 324 B
build/dom/index.min.js 4.64 kB
build/edit-post/classic-rtl.css 544 B
build/edit-post/classic.css 545 B
build/edit-widgets/style-rtl.css 4.52 kB
build/edit-widgets/style.css 4.52 kB
build/editor/index.min.js 45.5 kB
build/editor/style-rtl.css 3.53 kB
build/editor/style.css 3.52 kB
build/element/index.min.js 4.82 kB
build/escape-html/index.min.js 537 B
build/format-library/style-rtl.css 554 B
build/format-library/style.css 553 B
build/hooks/index.min.js 1.55 kB
build/html-entities/index.min.js 448 B
build/i18n/index.min.js 3.58 kB
build/is-shallow-equal/index.min.js 527 B
build/keyboard-shortcuts/index.min.js 1.72 kB
build/keycodes/index.min.js 1.87 kB
build/list-reusable-blocks/index.min.js 2.2 kB
build/list-reusable-blocks/style-rtl.css 836 B
build/list-reusable-blocks/style.css 836 B
build/media-utils/index.min.js 2.9 kB
build/notices/index.min.js 948 B
build/nux/index.min.js 1.99 kB
build/nux/style-rtl.css 735 B
build/nux/style.css 732 B
build/patterns/index.min.js 2.71 kB
build/patterns/style-rtl.css 240 B
build/patterns/style.css 240 B
build/plugins/index.min.js 1.79 kB
build/preferences-persistence/index.min.js 1.84 kB
build/preferences/index.min.js 1.24 kB
build/primitives/index.min.js 943 B
build/priority-queue/index.min.js 1.52 kB
build/private-apis/index.min.js 958 B
build/react-i18n/index.min.js 615 B
build/react-refresh-entry/index.min.js 9.47 kB
build/react-refresh-runtime/index.min.js 7.31 kB
build/redux-routine/index.min.js 2.7 kB
build/reusable-blocks/index.min.js 2.7 kB
build/reusable-blocks/style-rtl.css 243 B
build/reusable-blocks/style.css 243 B
build/rich-text/index.min.js 11 kB
build/router/index.min.js 1.78 kB
build/server-side-render/index.min.js 1.94 kB
build/shortcode/index.min.js 1.39 kB
build/sync/index.min.js 53.8 kB
build/token-list/index.min.js 582 B
build/url/index.min.js 3.73 kB
build/vendors/inert-polyfill.min.js 2.48 kB
build/vendors/react-dom.min.js 41.8 kB
build/vendors/react.min.js 4.02 kB
build/viewport/index.min.js 958 B
build/warning/index.min.js 249 B
build/widgets/index.min.js 7.16 kB
build/widgets/style-rtl.css 1.15 kB
build/widgets/style.css 1.16 kB
build/wordcount/index.min.js 1.02 kB

compressed-size-action

@github-actions
Copy link

github-actions bot commented Sep 6, 2023

Flaky tests detected in 7107f34.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6096127761
📝 Reported issues:

@draganescu
Copy link
Contributor

As far as I remember we specifically downgraded in UI proeminence the availability of multiple "navigations". I am curious what design feedback looks like today for this.

Copy link
Contributor

@alexstine alexstine left a comment

Choose a reason for hiding this comment

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

One note.

@getdave
Copy link
Contributor

getdave commented Sep 8, 2023

As far as I remember we specifically downgraded in UI proeminence the availability of multiple "navigations". I am curious what design feedback looks like today for this.

Just for context, we've received a lot of feedback (in FSE outreach and elsewhere on PRs and Issues) that users do not know where/how to switch menus. Many users don't open the sidebar and this is causing confusion.

icon={ moreVertical }
toggleProps={ { isSmall: true } }
icon={ icon }
toggleProps={ toggleProps }
Copy link
Contributor

Choose a reason for hiding this comment

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

Augmenting toggle props with aria* attribute to address Alex's feedback re: text label content could work here.

@@ -150,6 +152,27 @@ const MenuInspectorControls = ( props ) => {
>
{ __( 'Menu' ) }
</Heading>
<BlockControls group="inline">
<NavigationMenuSelector
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about only showing this control if there is more than 1 menu? That might limit any potential confusion for users who only have a single menu (i.e. fresh install one is created by fallback algorithm).

One counter argument is that having a toolbar option that shows/hides depending on context could be confusing to users of assistive tech. Continuity of tools is probably a major factor here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It also offers the import from classic option...

isManageMenusButtonDisabled
}
icon={ chevronDown }
text={ __( 'Change' ) }
Copy link
Contributor

Choose a reason for hiding this comment

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

I was envisaging show the name of the menu here truncated so as not to be overly long. Something like this...

Screen Shot 2023-09-08 at 11 59 19

Copy link
Contributor

@getdave getdave left a comment

Choose a reason for hiding this comment

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

I am unsurprisingly in favour of this change (albeit with the caveats of the comments I left on the code itself - apologies I didn't submit those as part of this review 🤦 ).

As expressed in the Issue users have become very confused with where to find the place to change Navigation Menu.

The right hand sidebar is very hidden and many users won't know about it. Something as important as changing your menu should be prominent.

Note that this feature did originally exist, but was removed as contributors felt it was a 20% use case and risked confusing or distracting users.

I would argue that this is not the case and it's pretty common for folk to want to change menus.

I have suggested a middle ground of only showing this control if the user has more than a single menu but we should be conscious of a11y concerns around that approach.

@scruffian
Copy link
Contributor Author

I pushed an update to the labels. Also the navigation menu selector only shows if you have more than one navigation menu.

@alexstine
Copy link
Contributor

Is there a reason we're not simply adding this to the block itself? Why not a simple select that appears when editing the block? Seems like the toolbar and sidebar are always used for controls, interested as to why they are never UI elements in the Placeholder component. It wouldn't be super accessible at the moment but there are PRs in the works to fix it.

@scruffian
Copy link
Contributor Author

Is there a reason we're not simply adding this to the block itself?

The block is never in a placeholder state, since we now have started creating a fallback navigation when you use the block. Also I think this action is something users might want to do after they have made changes to the block, so it would need to be accessible outside of the placeholder.

@alexstine
Copy link
Contributor

@scruffian I am thinking about when the block is currently selected? E.g. the isSelected prop is true? The Placeholder component could be used to render the field within the block itself vs. needing to take up room in the toolbar/sidebar.

@richtabor
Copy link
Member

As far as I remember we specifically downgraded in UI proeminence the availability of multiple "navigations". I am curious what design feedback looks like today for this.

I'm a bit hesitant. Is it often you want to change the navigation source? Or is it just once, when the block is added?

@getdave
Copy link
Contributor

getdave commented Sep 11, 2023

Is it often you want to change the navigation source?

This is a good question - thank you.

I don't think it's necessarily when the block is added. Most Themes, for example, come with a Nav block already in the "header" and this will have a Navigation Menu auto-selected by way of the fallback algorithm.

Thus the most likely use case is a user who sees a given Navigation selected and wishes to change it to another Navigation which they previously created.

Note that the fallback does it's best to pick the "most appropriate" Navigation, but this may not always be the one you expected/wanted. That's simply a facet of having an "automated" system.

Also consider that we've been repeatly optimising for a linear flow whereby when the block is added we don't ask a user to make any upfront choices. Rather the flow is optimised for the block to "just work". However, if we've optimise for this route we need to improve the UI for the group of users who want to configure their Navigation after the fact. Currently I do not believe the block does a good enough job in this regard. This is somewhat supported by the feedback we've received to that effect.

There has been a lot of feedback that this menu switching control is too hard to find. We don't necessarily have to add it into the block toolbar, but it needs to be given more prominence than it is now (witness that it is hidden in a sidebar under an options menu). Whether it's a less common action or not, I believe it is sufficiently important/common to warrant some extra prominence.

If we're unsure about the block toolbar then what other options could Design suggest that might help to address this issue?

@jasmussen
Copy link
Contributor

If we're unsure about the block toolbar then what other options could Design suggest that might help to address this issue?

#37068 is from December 2021, before the 6.3 site editor interface was created to solve exactly menu management issues. I would go back to the original issue and ask whether it was really relevant. Mainly, the proliferation of multiple menus, and subsequently the need to switch between them and delete them, it can be argued this was due to a lack of a central management location. To that end, I would still think the inspector is the right place for it. Though if need be, we could adapt this interface a bit:

inspector menu

The "Menu" on the left could be the title of the menu, and have a chevron right next to it, and the ellipsis on the right could have just "delete menu" and "create new menu", potentially.

We still need to handle the fallback behavior in a better way, right now I can only imagine that this error message itself is exacerbating things:

Screenshot 2023-09-11 at 10 31 25

IMO it should not be the primary action to create a new menu when an existing one was deleted. Ideally it should fall back invisibly, mark the document dirty if need be, and inform of the bugfix in the multi entity saving dialog. But anything would be better than what we have there, even a promt to choose your fallback.

@getdave
Copy link
Contributor

getdave commented Sep 11, 2023

The "Menu" on the left could be the title of the menu, and have a chevron right next to it, and the ellipsis on the right could have just "delete menu" and "create new menu", potentially.

If we could de-couple:

  • Selecting a menu
  • Taking actions on the current menu (e.g. Delete, Rename...etc)

...that would make a big difference and also allow us to address some other UX issues with the block.

If I understand correctly you are suggesting the word Menu becomes {{Title of current menu}} and then there is an arrow to indicate this is a interactive "switcher".

Then the options menu becomes contextual options for the currently selected_ menu.

Did I get that right?

@jasmussen
Copy link
Contributor

If I understand correctly you are suggesting the word Menu becomes {{Title of current menu}} and then there is an arrow to indicate this is a interactive "switcher".

Right, it's a dropdown, and the panel title at the same time. It's an early Monday and jetlagged thought, so it's entirely possible I'm missing important nuance, though!

@jameskoster
Copy link
Contributor

Iirc there used to be a menu selector immediately below that "Menu" heading in the Inspector. That might be a nice middle ground that doesn't involve a unique UI?

@jasmussen
Copy link
Contributor

Iirc there used to be a menu selector immediately below that "Menu" heading in the Inspector. That might be a nice middle ground that doesn't involve a unique UI?

Yep, I think we intentionally moved that to the ellipsis to reduce the prominence. This goes back to some of the higher level thoughts on optimizing for fewer menus, which most people are likely to have if we avoid proliferating them.

To that end, the dropdown will take already valuable space of the inspector. That said, if it addresses the initial issue, and avoids us having to add it to the block toolbar, I'd be open to going back to this, knowing this is something that'll need to contiously be iterated regardless.

@jameskoster
Copy link
Contributor

Yeah it's a tough one to balance and I'm also keen to avoid going in circles. Perhaps "Create new" stays in the ellipsis, and the dropdown appears only when multiple menus exist? That might address the feedback about switching being too buried, without exposing anything more.

@getdave
Copy link
Contributor

getdave commented Sep 12, 2023

So in summary of the above:

  • we wish to avoid putting the menu selector in the block toolbar because it places too great an importance on an action that is perceived as being "less common". Instead the selector should remain in the sidebar controls.
  • nonetheless menu switching is currently too hidden away. We should move the switching out of the 3-dots options menu and instead show a <select> style dropdown above the list view to allow users to switch out the menu without having to "discover" the 3-dots options. The menu selection should only appear if the user has multiple menus.
  • the existing 3-dots menu should be repurpose as the options for the currently assigned Navigation [Menu]. Options should be:
    • Rename (triggering Modal - a la sidebar) - note this could be in a follow up PR.
    • Delete
    • Create new

Please correct any inconsistencies 🙏

@jameskoster
Copy link
Contributor

I always struggle a bit with navigation due to the relationship between the wrapping block and the menu within. In that regard I see "Menu" as a label for the dropdown (or just as a generic heading when there's only one menu).

The ellipsis feels like an element of the block, not the chosen menu, so I wouldn't expect to find rename/delete actions there. "Create new menu" makes sense, and I could see "Manage menus" living there (when there are multiple).

@getdave
Copy link
Contributor

getdave commented Sep 13, 2023

I always struggle a bit with navigation due to the relationship between the wrapping block and the menu within. In that regard I see "Menu" as a label for the dropdown (or just as a generic heading when there's only one menu).

The ellipsis feels like an element of the block, not the chosen menu, so I wouldn't expect to find rename/delete actions there. "Create new menu" makes sense, and I could see "Manage menus" living there (when there are multiple).

I felt that when you are in the "List View" tab you are clearly editing the menu items (and thus the Navigation Menu) so it would be ok to have those menu items in the ellipsis menu. But I take your point.

Also, the concept of "this is your selected menu" isn't always apparent - I'm not clear if we're trying to intentionally obfuscate this from the user. Are we?

Either way, we've had feedback that the following actions are not obvious enough:

  • finding where to switch the menu used on a given block
  • finding where to rename a given menu
  • finding where to delete a given menu

You could now make an argument that the Site View Navigation section handles the later two and that those items are more related to the Navigation Menu anyway so less relevant on the block itself.

This leaves only the first item as the problem - this one seems highly relevant to the specific block and we can't handle this interaction in Site View. So are we happy with the revised proposal?

@jameskoster
Copy link
Contributor

Also, the concept of "this is your selected menu" isn't always apparent - I'm not clear if we're trying to intentionally obfuscate this from the user. Are we?

In the "Menu" tab of the Navigation block? I don't see any need to obfuscate that when there are already multiple menus. I think we want to avoid the proliferation of menus which seems to occur when the 'create new' actions are too prominent.

So are we happy with #54214 (comment)?

I think so. We might include a 'Manage menus' link alongside 'Create new', but that can be tried separately.

@scruffian
Copy link
Contributor Author

Thanks for the feedback all. I tried the ideas in a new PR: #54436

@scruffian scruffian closed this Sep 13, 2023
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 Needs Design Feedback Needs general design feedback. [Type] Enhancement A suggestion for improvement.
Projects
Development

Successfully merging this pull request may close these issues.

Navigation Block: consider adding back in ability to change menus in a more visible place
7 participants