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 Anything that 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.

@shaunandrews shaunandrews moved this from 👀 PRs to review to 💻 Issues in progress in Navigation editor Oct 30, 2019
@shaunandrews shaunandrews moved this from 💻 Issues in progress to 👀 PRs to review in Navigation editor Oct 30, 2019
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
Navigation editor automation moved this from 👀 PRs to review to ✅ Done 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
CreativeDive pushed a commit to CreativeDive/gutenberg that referenced this pull request Nov 12, 2019
* Show appender only when item has submenu

* Address PR feedback.

* Insert block directly from toolbar

* updates submenu label, field css and shows toolbar button all the time
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

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