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 a Navigation Heading block. #33351

Closed
wants to merge 4 commits into from
Closed

Conversation

tellthemachines
Copy link
Contributor

Description

Fixes #18866. It should be possible to add a heading to a submenu list that is not itself a link.

Some questions to consider:

  • I made the text element a span for now but perhaps it should be an actual heading element. Looking for accessibility feedback on that one.
  • Because of Navigation: use generic css class names for navigation items #33048, and to avoid further duplication, I went with using the Navigation Link block classes for now; that's a temporary fix but the classname issue needs to be solved globally for all Navigation block children.
  • The other place where a lot of code duplication is happening is the index.php file. E.g. there are identical versions in everything but name of the ...build_css_colors and ...build_css_font_sizes functions across 4 files. Might be worth extracting some of these into a common file, but where should it live?

TODO:

  • Add ability to transform Navigation Heading to Navigation Link and vice-versa. (Maybe as a separate PR; this one's already pretty big.)
  • Allow this block to be used for menu items without URLs in the Navigation screen. Currently the experience for items with no URL is a bit buggy because their label gets deleted:

Screen Shot 2021-07-12 at 2 41 25 pm

How has this been tested?

Tested in the browser. Add an empty Navigation block to a post or page, then add a Navigation Heading block inside. Verify a submenu can be created from it and Navigation Links added as children. Also verify that Navigation Heading can only be added at the top level.

Screenshots

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.
  • I've tested my changes with keyboard and screen readers.
  • 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 (please manually search all *.native.js files for terms that need renaming or removal).

@github-actions
Copy link

github-actions bot commented Jul 12, 2021

Size Change: +3.41 kB (0%)

Total Size: 1.07 MB

Filename Size Change
build/annotations/index.min.js 2.93 kB -1 B (0%)
build/block-directory/index.min.js 6.62 kB +3 B (0%)
build/block-editor/index.min.js 126 kB +628 B (+1%)
build/block-editor/style-rtl.css 14 kB +50 B (0%)
build/block-editor/style.css 14 kB +49 B (0%)
build/block-library/blocks/latest-posts/style-rtl.css 526 B -8 B (-1%)
build/block-library/blocks/latest-posts/style.css 524 B -8 B (-2%)
build/block-library/blocks/navigation/style-rtl.css 1.67 kB +17 B (+1%)
build/block-library/blocks/navigation/style.css 1.67 kB +16 B (+1%)
build/block-library/blocks/tag-cloud/editor-rtl.css 0 B -118 B (removed) 🏆
build/block-library/blocks/tag-cloud/editor.css 0 B -118 B (removed) 🏆
build/block-library/blocks/tag-cloud/style-rtl.css 146 B +52 B (+55%) 🆘
build/block-library/blocks/tag-cloud/style.css 146 B +52 B (+55%) 🆘
build/block-library/editor-rtl.css 9.81 kB -33 B (0%)
build/block-library/editor.css 9.81 kB -37 B (0%)
build/block-library/index.min.js 147 kB +1.37 kB (+1%)
build/block-library/style-rtl.css 10.3 kB +54 B (+1%)
build/block-library/style.css 10.3 kB +62 B (+1%)
build/blocks/index.min.js 47.2 kB -4 B (0%)
build/components/index.min.js 194 kB -24 B (0%)
build/compose/index.min.js 10.2 kB -4 B (0%)
build/core-data/index.min.js 12.4 kB -10 B (0%)
build/customize-widgets/index.min.js 10.3 kB +20 B (0%)
build/data/index.min.js 7.22 kB +2 B (0%)
build/dom/index.min.js 4.78 kB +116 B (+2%)
build/edit-navigation/index.min.js 13.9 kB +40 B (0%)
build/edit-post/index.min.js 59.4 kB +844 B (+1%)
build/edit-site/index.min.js 25.9 kB +71 B (0%)
build/edit-widgets/index.min.js 16.1 kB +39 B (0%)
build/editor/index.min.js 38.6 kB +34 B (0%)
build/element/index.min.js 3.44 kB -1 B (0%)
build/nux/index.min.js 2.31 kB +1 B (0%)
build/primitives/index.min.js 1.06 kB +33 B (+3%)
build/redux-routine/index.min.js 2.82 kB +1 B (0%)
build/rich-text/index.min.js 10.8 kB +182 B (+2%)
build/warning/index.min.js 1.16 kB +4 B (0%)
build/widgets/index.min.js 6.48 kB +31 B (0%)
ℹ️ View Unchanged
Filename Size
build/a11y/index.min.js 1.12 kB
build/admin-manifest/index.min.js 1.41 kB
build/api-fetch/index.min.js 2.44 kB
build/autop/index.min.js 2.28 kB
build/blob/index.min.js 673 B
build/block-directory/style-rtl.css 1.02 kB
build/block-directory/style.css 1.02 kB
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 65 B
build/block-library/blocks/archives/style.css 65 B
build/block-library/blocks/audio/editor-rtl.css 58 B
build/block-library/blocks/audio/editor.css 58 B
build/block-library/blocks/audio/style-rtl.css 112 B
build/block-library/blocks/audio/style.css 112 B
build/block-library/blocks/audio/theme-rtl.css 125 B
build/block-library/blocks/audio/theme.css 125 B
build/block-library/blocks/block/editor-rtl.css 161 B
build/block-library/blocks/block/editor.css 161 B
build/block-library/blocks/button/editor-rtl.css 475 B
build/block-library/blocks/button/editor.css 474 B
build/block-library/blocks/button/style-rtl.css 603 B
build/block-library/blocks/button/style.css 602 B
build/block-library/blocks/buttons/editor-rtl.css 315 B
build/block-library/blocks/buttons/editor.css 315 B
build/block-library/blocks/buttons/style-rtl.css 375 B
build/block-library/blocks/buttons/style.css 375 B
build/block-library/blocks/calendar/style-rtl.css 208 B
build/block-library/blocks/calendar/style.css 208 B
build/block-library/blocks/categories/editor-rtl.css 84 B
build/block-library/blocks/categories/editor.css 83 B
build/block-library/blocks/categories/style-rtl.css 79 B
build/block-library/blocks/categories/style.css 79 B
build/block-library/blocks/code/style-rtl.css 90 B
build/block-library/blocks/code/style.css 90 B
build/block-library/blocks/code/theme-rtl.css 131 B
build/block-library/blocks/code/theme.css 131 B
build/block-library/blocks/columns/editor-rtl.css 190 B
build/block-library/blocks/columns/editor.css 190 B
build/block-library/blocks/columns/style-rtl.css 475 B
build/block-library/blocks/columns/style.css 476 B
build/block-library/blocks/cover/editor-rtl.css 670 B
build/block-library/blocks/cover/editor.css 670 B
build/block-library/blocks/cover/style-rtl.css 1.22 kB
build/block-library/blocks/cover/style.css 1.23 kB
build/block-library/blocks/embed/editor-rtl.css 486 B
build/block-library/blocks/embed/editor.css 486 B
build/block-library/blocks/embed/style-rtl.css 401 B
build/block-library/blocks/embed/style.css 400 B
build/block-library/blocks/embed/theme-rtl.css 124 B
build/block-library/blocks/embed/theme.css 124 B
build/block-library/blocks/file/editor-rtl.css 301 B
build/block-library/blocks/file/editor.css 300 B
build/block-library/blocks/file/style-rtl.css 255 B
build/block-library/blocks/file/style.css 255 B
build/block-library/blocks/file/view.min.js 780 B
build/block-library/blocks/freeform/editor-rtl.css 2.44 kB
build/block-library/blocks/freeform/editor.css 2.44 kB
build/block-library/blocks/gallery/editor-rtl.css 704 B
build/block-library/blocks/gallery/editor.css 705 B
build/block-library/blocks/gallery/style-rtl.css 1.06 kB
build/block-library/blocks/gallery/style.css 1.06 kB
build/block-library/blocks/gallery/theme-rtl.css 122 B
build/block-library/blocks/gallery/theme.css 122 B
build/block-library/blocks/group/editor-rtl.css 160 B
build/block-library/blocks/group/editor.css 160 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 93 B
build/block-library/blocks/group/theme.css 93 B
build/block-library/blocks/heading/editor-rtl.css 152 B
build/block-library/blocks/heading/editor.css 152 B
build/block-library/blocks/heading/style-rtl.css 76 B
build/block-library/blocks/heading/style.css 76 B
build/block-library/blocks/home-link/style-rtl.css 259 B
build/block-library/blocks/home-link/style.css 259 B
build/block-library/blocks/html/editor-rtl.css 281 B
build/block-library/blocks/html/editor.css 281 B
build/block-library/blocks/image/editor-rtl.css 729 B
build/block-library/blocks/image/editor.css 727 B
build/block-library/blocks/image/style-rtl.css 481 B
build/block-library/blocks/image/style.css 485 B
build/block-library/blocks/image/theme-rtl.css 124 B
build/block-library/blocks/image/theme.css 124 B
build/block-library/blocks/latest-comments/style-rtl.css 286 B
build/block-library/blocks/latest-comments/style.css 286 B
build/block-library/blocks/latest-posts/editor-rtl.css 137 B
build/block-library/blocks/latest-posts/editor.css 137 B
build/block-library/blocks/list/style-rtl.css 63 B
build/block-library/blocks/list/style.css 63 B
build/block-library/blocks/media-text/editor-rtl.css 263 B
build/block-library/blocks/media-text/editor.css 264 B
build/block-library/blocks/media-text/style-rtl.css 492 B
build/block-library/blocks/media-text/style.css 489 B
build/block-library/blocks/more/editor-rtl.css 434 B
build/block-library/blocks/more/editor.css 434 B
build/block-library/blocks/navigation-link/editor-rtl.css 474 B
build/block-library/blocks/navigation-link/editor.css 473 B
build/block-library/blocks/navigation-link/style-rtl.css 94 B
build/block-library/blocks/navigation-link/style.css 94 B
build/block-library/blocks/navigation/editor-rtl.css 1.69 kB
build/block-library/blocks/navigation/editor.css 1.69 kB
build/block-library/blocks/navigation/view.min.js 2.87 kB
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 310 B
build/block-library/blocks/page-list/editor.css 311 B
build/block-library/blocks/page-list/style-rtl.css 240 B
build/block-library/blocks/page-list/style.css 240 B
build/block-library/blocks/paragraph/editor-rtl.css 157 B
build/block-library/blocks/paragraph/editor.css 157 B
build/block-library/blocks/paragraph/style-rtl.css 247 B
build/block-library/blocks/paragraph/style.css 248 B
build/block-library/blocks/post-author/editor-rtl.css 209 B
build/block-library/blocks/post-author/editor.css 209 B
build/block-library/blocks/post-author/style-rtl.css 183 B
build/block-library/blocks/post-author/style.css 184 B
build/block-library/blocks/post-comments-form/style-rtl.css 140 B
build/block-library/blocks/post-comments-form/style.css 140 B
build/block-library/blocks/post-comments/style-rtl.css 360 B
build/block-library/blocks/post-comments/style.css 359 B
build/block-library/blocks/post-content/editor-rtl.css 139 B
build/block-library/blocks/post-content/editor.css 139 B
build/block-library/blocks/post-excerpt/editor-rtl.css 73 B
build/block-library/blocks/post-excerpt/editor.css 73 B
build/block-library/blocks/post-excerpt/style-rtl.css 69 B
build/block-library/blocks/post-excerpt/style.css 69 B
build/block-library/blocks/post-featured-image/editor-rtl.css 338 B
build/block-library/blocks/post-featured-image/editor.css 338 B
build/block-library/blocks/post-featured-image/style-rtl.css 141 B
build/block-library/blocks/post-featured-image/style.css 141 B
build/block-library/blocks/post-template/editor-rtl.css 100 B
build/block-library/blocks/post-template/editor.css 99 B
build/block-library/blocks/post-template/style-rtl.css 379 B
build/block-library/blocks/post-template/style.css 380 B
build/block-library/blocks/post-terms/style-rtl.css 73 B
build/block-library/blocks/post-terms/style.css 73 B
build/block-library/blocks/post-title/style-rtl.css 60 B
build/block-library/blocks/post-title/style.css 60 B
build/block-library/blocks/preformatted/style-rtl.css 103 B
build/block-library/blocks/preformatted/style.css 103 B
build/block-library/blocks/pullquote/editor-rtl.css 183 B
build/block-library/blocks/pullquote/editor.css 183 B
build/block-library/blocks/pullquote/style-rtl.css 318 B
build/block-library/blocks/pullquote/style.css 318 B
build/block-library/blocks/pullquote/theme-rtl.css 169 B
build/block-library/blocks/pullquote/theme.css 169 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 270 B
build/block-library/blocks/query-pagination/editor.css 262 B
build/block-library/blocks/query-pagination/style-rtl.css 168 B
build/block-library/blocks/query-pagination/style.css 168 B
build/block-library/blocks/query-title/editor-rtl.css 86 B
build/block-library/blocks/query-title/editor.css 86 B
build/block-library/blocks/query/editor-rtl.css 131 B
build/block-library/blocks/query/editor.css 132 B
build/block-library/blocks/quote/style-rtl.css 169 B
build/block-library/blocks/quote/style.css 169 B
build/block-library/blocks/quote/theme-rtl.css 221 B
build/block-library/blocks/quote/theme.css 221 B
build/block-library/blocks/rss/editor-rtl.css 201 B
build/block-library/blocks/rss/editor.css 202 B
build/block-library/blocks/rss/style-rtl.css 290 B
build/block-library/blocks/rss/style.css 290 B
build/block-library/blocks/search/editor-rtl.css 211 B
build/block-library/blocks/search/editor.css 211 B
build/block-library/blocks/search/style-rtl.css 359 B
build/block-library/blocks/search/style.css 362 B
build/block-library/blocks/search/theme-rtl.css 64 B
build/block-library/blocks/search/theme.css 64 B
build/block-library/blocks/separator/editor-rtl.css 99 B
build/block-library/blocks/separator/editor.css 99 B
build/block-library/blocks/separator/style-rtl.css 251 B
build/block-library/blocks/separator/style.css 251 B
build/block-library/blocks/separator/theme-rtl.css 172 B
build/block-library/blocks/separator/theme.css 172 B
build/block-library/blocks/shortcode/editor-rtl.css 476 B
build/block-library/blocks/shortcode/editor.css 476 B
build/block-library/blocks/site-logo/editor-rtl.css 465 B
build/block-library/blocks/site-logo/editor.css 465 B
build/block-library/blocks/site-logo/style-rtl.css 154 B
build/block-library/blocks/site-logo/style.css 154 B
build/block-library/blocks/site-tagline/editor-rtl.css 87 B
build/block-library/blocks/site-tagline/editor.css 87 B
build/block-library/blocks/site-title/editor-rtl.css 85 B
build/block-library/blocks/site-title/editor.css 85 B
build/block-library/blocks/social-link/editor-rtl.css 164 B
build/block-library/blocks/social-link/editor.css 165 B
build/block-library/blocks/social-links/editor-rtl.css 800 B
build/block-library/blocks/social-links/editor.css 799 B
build/block-library/blocks/social-links/style-rtl.css 1.34 kB
build/block-library/blocks/social-links/style.css 1.34 kB
build/block-library/blocks/spacer/editor-rtl.css 308 B
build/block-library/blocks/spacer/editor.css 308 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 478 B
build/block-library/blocks/table/editor.css 478 B
build/block-library/blocks/table/style-rtl.css 480 B
build/block-library/blocks/table/style.css 480 B
build/block-library/blocks/table/theme-rtl.css 188 B
build/block-library/blocks/table/theme.css 188 B
build/block-library/blocks/template-part/editor-rtl.css 551 B
build/block-library/blocks/template-part/editor.css 550 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/editor-rtl.css 90 B
build/block-library/blocks/term-description/editor.css 90 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 87 B
build/block-library/blocks/verse/style.css 87 B
build/block-library/blocks/video/editor-rtl.css 569 B
build/block-library/blocks/video/editor.css 570 B
build/block-library/blocks/video/style-rtl.css 173 B
build/block-library/blocks/video/style.css 173 B
build/block-library/blocks/video/theme-rtl.css 124 B
build/block-library/blocks/video/theme.css 124 B
build/block-library/common-rtl.css 1.29 kB
build/block-library/common.css 1.29 kB
build/block-library/reset-rtl.css 514 B
build/block-library/reset.css 515 B
build/block-library/theme-rtl.css 692 B
build/block-library/theme.css 693 B
build/block-serialization-default-parser/index.min.js 1.3 kB
build/block-serialization-spec-parser/index.min.js 3.06 kB
build/components/style-rtl.css 16.1 kB
build/components/style.css 16.1 kB
build/customize-widgets/style-rtl.css 1.48 kB
build/customize-widgets/style.css 1.48 kB
build/data-controls/index.min.js 830 B
build/date/index.min.js 31.8 kB
build/deprecated/index.min.js 738 B
build/dom-ready/index.min.js 576 B
build/edit-navigation/style-rtl.css 3.12 kB
build/edit-navigation/style.css 3.12 kB
build/edit-post/classic-rtl.css 483 B
build/edit-post/classic.css 483 B
build/edit-post/style-rtl.css 7.25 kB
build/edit-post/style.css 7.24 kB
build/edit-site/style-rtl.css 5.04 kB
build/edit-site/style.css 5.03 kB
build/edit-widgets/style-rtl.css 3.88 kB
build/edit-widgets/style.css 3.89 kB
build/editor/style-rtl.css 3.88 kB
build/editor/style.css 3.88 kB
build/escape-html/index.min.js 739 B
build/format-library/index.min.js 5.71 kB
build/format-library/style-rtl.css 668 B
build/format-library/style.css 669 B
build/hooks/index.min.js 1.76 kB
build/html-entities/index.min.js 628 B
build/i18n/index.min.js 3.73 kB
build/is-shallow-equal/index.min.js 709 B
build/keyboard-shortcuts/index.min.js 1.74 kB
build/keycodes/index.min.js 1.43 kB
build/list-reusable-blocks/index.min.js 2.07 kB
build/list-reusable-blocks/style-rtl.css 842 B
build/list-reusable-blocks/style.css 842 B
build/media-utils/index.min.js 3.08 kB
build/notices/index.min.js 1.07 kB
build/nux/style-rtl.css 745 B
build/nux/style.css 742 B
build/plugins/index.min.js 1.99 kB
build/priority-queue/index.min.js 790 B
build/react-i18n/index.min.js 924 B
build/reusable-blocks/index.min.js 2.56 kB
build/reusable-blocks/style-rtl.css 256 B
build/reusable-blocks/style.css 256 B
build/server-side-render/index.min.js 1.64 kB
build/shortcode/index.min.js 1.68 kB
build/token-list/index.min.js 847 B
build/url/index.min.js 1.95 kB
build/viewport/index.min.js 1.28 kB
build/widgets/style-rtl.css 1.04 kB
build/widgets/style.css 1.05 kB
build/wordcount/index.min.js 1.24 kB

compressed-size-action

@draganescu
Copy link
Contributor

👋🏻 why can't we use a heading block? Or just a paragraph?

@tellthemachines
Copy link
Contributor Author

👋🏻 why can't we use a heading block? Or just a paragraph?

There's some more context in the discussion starting here, but tl,dr: there are too many differences between what we need here (a block exactly like Navigation Link but without the link) and either of those blocks.

@tellthemachines
Copy link
Contributor Author

There's a conversation going over on Slack about the best element to use here, considering screen reader accessibility:

  • A heading can be confusing when navigating through headings on the page, as it won't be obvious this is a heading within a menu - not ideal.
  • A span with a tabindex will result in the text content being read out, but no other indication of what that item's role is.
  • A button is semantically closer to the item's role, but it might lead users to expect the dropdown will only open on click (it actually opens on hover or focus).

Looking at classic menu patterns in default themes, most of them expect the parent element that opens the dropdown to be a link; dropdowns open on hover or focus so screen readers are only aware that they are entering a nested list, the fact that it's housed in a dropdown isn't relevant. (Twenty Twenty One is an exception, with a dedicated button to open the submenu dropdown.)

The problem is that it's possible to set a menu item without a URL in classic menus, although it's a bit of a hack (it requires creating a custom link and then deleting its URL), and if that happens, the element displayed is an anchor tag without a href. The screen reader ignores it altogether, but it's still possible to tab to the submenu even if the anchor itself isn't focusable. This isn't great, but it's not clear what a more semantic alternative might be.

@talldan
Copy link
Contributor

talldan commented Jul 14, 2021

This is a really popular pattern online, but most sites seem to prefer to open a submenu on a click rather than a hover.

Some markup I've observed in the wild. These open on click and clicking only opens a submenu, the item doesn't act as a link to anywhere:
Screenshot 2021-07-14 at 2 26 20 pm
Screenshot 2021-07-14 at 2 17 07 pm
Screenshot 2021-07-14 at 2 16 28 pm
Screenshot 2021-07-14 at 2 15 50 pm

These open on hover and the same as above, even though the first one has a link href, clicking it just opens a submenu 🤔:
Screenshot 2021-07-14 at 2 21 33 pm
Screenshot 2021-07-14 at 2 24 01 pm
Screenshot 2021-07-14 at 2 33 37 pm

This one, the H&M site, opens on hover, but the item can also be clicked as a link. I couldn't figure out a way to open the submenu using the keyboard only, though there is an 'accessibility trigger' button element in there as well:
Screenshot 2021-07-14 at 2 51 37 pm

So while this is a limited subset, link or button seem the most popular options.

Some related issues:

I wonder if the idea of a submenu block could be explored. The submenu block could contain options for whether the submenu opens on click or hover. I feel like the following might work:

  • If the submenu opens on hover, the item can be a link or perhaps a span if it doesn't link anywhere. If it's a link, maybe there should be a separate icon button for opening the submenu?
  • If the submenu opens on click, the item can be a button, and there should be no option to link somewhere.

Introducing a submenu block would require a bit of work in the navigation editor, but I don't think that should be considered a reason not to do it.

@Ryokuhi
Copy link

Ryokuhi commented Jul 16, 2021

This pull request was reviewed today during the Accessibility Team's meeting.
The full discussion can be found on Slack, but here are the main takeaways of the discussion.

  • The best solution is probably to mark the heading block inside the navigation as a button opening the submenu, which behaves like a disclosure widget. This allows users to skip submenus if they are not interested in their content. Also, the submenu should not open on focus, as it can distract and confuse the user especially if there are multiple levels.
  • The biggest issue is that such a button should have an appropriately managed aria-expanded attribute, but this is impossible in Gutenberg right now, since there's no JavaScript output for Gutenberg blocks. In this situation, CSS alone is not enough, so adding a way to dynamically communicate the change of the button state is a blocker to the merge of this pull request.

@talldan
Copy link
Contributor

talldan commented Jul 20, 2021

The biggest issue is that such a button should have an appropriately managed aria-expanded attribute, but this is impossible in Gutenberg right now, since there's no JavaScript output for Gutenberg blocks. In this situation, CSS alone is not enough, so adding a way to dynamically communicate the change of the button state is a blocker to the merge of this pull request.

@Ryokuhi I think this is possible now, the navigation block has a frontend script for the responsive menu, so it should be possible to build on top of that:

"viewScript": "file:./view.min.js",

@tellthemachines
Copy link
Contributor Author

Thanks for the feedback, everyone!

We have two aspects to consider here: the front end UI (whether it opens on click, hover, etc.), and the editor UI (whether to open submenus directly from Link/Heading blocks, or to build a dedicated Submenu block).

Front End UI

Bearing in mind the accessibility feedback that the submenu shouldn't open on focus, I think our best bet is to follow a similar pattern to Twenty Twenty One, where submenus open on both hover and click. Then later on we can implement #18395 to make hover optional.

The difference in this implementation is that, unlike classic menus, where top-level items without a link still render as anchor tags, we can make these non-link items buttons instead.

So, if we have a top level item that is a link, the markup will be something like:

<li>
  <a href="http://example.com">about us</a>
  <button aria-expanded="false">open submenu</button>
  <ul></ul>
</li>

Whereas if the top-level item isn't a link, it will be:

<li>
  <button aria-expanded="false">about us</button>
  <ul></ul>
</li>

I'm wondering if the button content needs explicit text instructions for screen readers, or if the aria-expanded attribute is enough.

Editor UI

Should we build a Submenu block? I see two main advantages:

  • Decoupling submenu logic from the Link block will not only make for cleaner code, as @talldan suggested, but also potentially a better experience, considering that upcoming work on mega menus will introduce a bunch of new controls for submenus.
  • Avoids misuse of the Heading block proposed in this PR: this block only makes sense if there's a submenu attached to it, but as it stands it could be used as a standalone block. I think if we're introducing button and aria-expanded semantics, it makes sense to have it packaged with a submenu.

If we do go this route, we have to think about how to structure the Submenu block internally. @talldan are there any obvious advantages to either of these approaches for the Navigation editor?

Also, I'm wondering how the Link block deprecation will work. Can we auto-transform existing Link blocks with submenus into Submenu blocks?

An important question to consider is: would we ever want to use a Submenu block outside of the Navigation? As a child of Navigation, we'll want to limit its inner blocks to only Links for now, and then when we go on to building mega menus, we'll need some logic to render groups of links as lists, such as we currently have for the Navigation block. As we'll need a bunch of logic custom to Navigation, I'm not sure how viable it will be to make it a general use block.

I'm going to put this PR on hold and investigate what a Submenu block might look like, and see if we can answer some of these questions.

@talldan
Copy link
Contributor

talldan commented Jul 27, 2021

@talldan are there any obvious advantages to either of these approaches for the Navigation editor?

If the editor UI is cleaner, it'd be beneficial, but other than that there would be no real difference.

The code that maps menu items to blocks and blocks to menu items would need to be updated to handle submenu blocks. Some feature may also need to be removed in the nav editor.

Also, I'm wondering how the Link block deprecation will work. Can we auto-transform existing Link blocks with submenus into Submenu blocks?

Yeah, this is an interesting one. It could be that the nav link block still supports submenus for back compat, but it's not something exposed in the UI. An option to convert to a submenu block could be shown as well.

It might also be possible at some point in the future to remove submenus from the nav link block entirely, given the block is plugin only.

It might be interesting to find out if there have been other similar use cases in the past. I'm sure some blocks have been renamed, but can't recall which ones.

@jasmussen
Copy link
Contributor

Very cool work.

At first and due to the name of the block, I instinctually thought of this as a "subheading" type item, to help build mega menus, like "Highlights", "Clothing" or "Accessories" in this draft mockup:

04 Navigation item

However I understand that the goal at least for now is to create a menu item that doesn't link, so that you can create submenus whose parent item doesn't also link anywhere, is that correct? Here's what I see:

nav heading

To be clear, that's cool. As I expanded on in your "Dropdown Menu" PR here, a dropdown menu that opens on click in a wholly unambiguous way can be very useful and perhaps even more usable than hover-only dropdown menus. See also #18395.

Because of Navigation: use generic css class names for navigation items #33048, and to avoid further duplication, I went with using the Navigation Link block classes for now; that's a temporary fix but the classname issue needs to be solved globally for all Navigation block children.

💯

Allow this block to be used for menu items without URLs in the Navigation screen. Currently the experience for items with no URL is a bit buggy because their label gets deleted:

Fixing this might also fix #33091. See also #33849 (comment).

@tellthemachines
Copy link
Contributor Author

Closing this in favour of #33775.

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 Accessibility Feedback Need input from accessibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Navigation block: can't unlink items
5 participants