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

Show appender only when item has submenu #18153

Merged
merged 4 commits into from
Oct 31, 2019
Merged

Conversation

tellthemachines
Copy link
Contributor

Description

Closes #18104.

Navigation menu items: Block appender should only be visible when the item already has a submenu.
If the item has no submenu, show a toolbar button that reveals the block appender on click.

Would love some a11y feedback on whether clicking the toolbar button should merely show/hide the appender, or if we should transfer focus to the appender, or announce its presence in any other way.
Currently clicking only shows/hides the appender so screen reader users won't get any feedback on what is happening.

How has this been tested?

Tested in browser:

  1. Enable navigation block in experiments
  2. Add a navigation block with some nav item blocks to a post
  3. Check that toolbar button is rendered for nav items that have no children, and appender button is not rendered.
  4. Check that toolbar button is not rendered for nav items that do have children, and appender button is rendered.

Screenshots

Scenario 1:

Screen Shot 2019-10-29 at 3 41 50 pm

Scenario 2:

Screen Shot 2019-10-29 at 3 42 29 pm

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.

@tellthemachines tellthemachines added the Needs Design Feedback Needs general design feedback. label Oct 29, 2019
@tellthemachines
Copy link
Contributor Author

Actually, now that #16708 has been merged, it makes more sense for the toolbar button to immediately insert a block. Will update.

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.

I have two minor comments, but otherwise this is good to go.

packages/block-library/src/navigation-menu-item/edit.js Outdated Show resolved Hide resolved
packages/block-library/src/navigation-menu-item/edit.js Outdated Show resolved Hide resolved
@draganescu draganescu merged commit 1338951 into master Oct 31, 2019
@draganescu draganescu deleted the add/submenu-toolbar-button branch October 31, 2019 09:22
@youknowriad youknowriad added this to the Gutenberg 6.9 milestone Nov 11, 2019
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 Needs Design Feedback Needs general design feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Navigation Menu should not show an appender on single root items
3 participants