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 Component: Semantically create nested lists #25280

Merged
merged 3 commits into from
Sep 14, 2020

Conversation

psealock
Copy link
Contributor

@psealock psealock commented Sep 14, 2020

Fixes: #25248

Description

Semantically render nested lists. NavigationMenu accepts both NavigationGroup and NavigationItem (or any combination thereof) and will always render correct HTML structure:

<h2></h2>
<ul>
	<li></li>
	<li></li>
	<li>
		<h3></h3>
		<ul>
			<li></li>
			<li></li>
			<li></li>
		</ul>
	</li>
</ul>

See more on nesting lists here: https://developer.mozilla.org/en-US/docs/Learn/HTML/Introduction_to_HTML/HTML_text_fundamentals#Lists

#25248 hypothesises HTML manipulation may be well suited to automation once a way to analyze Navigation children. I don't think that will be required because these changes ensure a correct structure.

How has this been tested?

  1. npm run storybook:dev.
  2. Look through the markup of the example to ensure structure is correct.
  3. Make sure no visual CSS regressions occurred.

Screenshots

Types of changes

Changes make rendering a ul element unneeded. Technically this is breaking, but Navigation code is unreleased and behind __experimental flag.

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.

@psealock psealock requested a review from Copons September 14, 2020 01:25
@psealock psealock added the [Feature] Navigation Component A navigational waterfall component for hierarchy of items. label Sep 14, 2020
@github-actions
Copy link

Size Change: +3 B (0%)

Total Size: 1.2 MB

Filename Size Change
build/components/index.js 202 kB +3 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.67 kB 0 B
build/api-fetch/index.js 3.41 kB 0 B
build/autop/index.js 2.83 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/index.js 8.53 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.js 128 kB 0 B
build/block-editor/style-rtl.css 11.1 kB 0 B
build/block-editor/style.css 11.1 kB 0 B
build/block-library/editor-rtl.css 8.68 kB 0 B
build/block-library/editor.css 8.68 kB 0 B
build/block-library/index.js 139 kB 0 B
build/block-library/style-rtl.css 7.6 kB 0 B
build/block-library/style.css 7.59 kB 0 B
build/block-library/theme-rtl.css 754 B 0 B
build/block-library/theme.css 754 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 47.8 kB 0 B
build/components/style-rtl.css 15.5 kB 0 B
build/components/style.css 15.5 kB 0 B
build/compose/index.js 9.67 kB 0 B
build/core-data/index.js 12.4 kB 0 B
build/data-controls/index.js 1.29 kB 0 B
build/data/index.js 8.55 kB 0 B
build/date/index.js 31.9 kB 0 B
build/deprecated/index.js 772 B 0 B
build/dom-ready/index.js 568 B 0 B
build/dom/index.js 4.47 kB 0 B
build/edit-navigation/index.js 10.7 kB 0 B
build/edit-navigation/style-rtl.css 868 B 0 B
build/edit-navigation/style.css 871 B 0 B
build/edit-post/index.js 305 kB 0 B
build/edit-post/style-rtl.css 6.25 kB 0 B
build/edit-post/style.css 6.23 kB 0 B
build/edit-site/index.js 19.3 kB 0 B
build/edit-site/style-rtl.css 3.14 kB 0 B
build/edit-site/style.css 3.14 kB 0 B
build/edit-widgets/index.js 12.2 kB 0 B
build/edit-widgets/style-rtl.css 2.55 kB 0 B
build/edit-widgets/style.css 2.55 kB 0 B
build/editor/editor-styles-rtl.css 492 B 0 B
build/editor/editor-styles.css 493 B 0 B
build/editor/index.js 45.6 kB 0 B
build/editor/style-rtl.css 3.81 kB 0 B
build/editor/style.css 3.81 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.71 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 621 B 0 B
build/i18n/index.js 3.56 kB 0 B
build/is-shallow-equal/index.js 710 B 0 B
build/keyboard-shortcuts/index.js 2.52 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 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.32 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.41 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 13.9 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/token-list/index.js 1.27 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

Copy link
Contributor

@Copons Copons left a comment

Choose a reason for hiding this comment

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

It was so simple! ✨

Thanks for taking care of this @psealock

@psealock psealock merged commit fa04050 into master Sep 14, 2020
@psealock psealock deleted the fix/Nav-component-semantic-html branch September 14, 2020 18:53
@github-actions github-actions bot added this to the Gutenberg 9.1 milestone Sep 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Navigation Component A navigational waterfall component for hierarchy of items.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Navigation Component: Add ul Automatically Where Needed
2 participants