Skip to content

Conversation

@thatblindgeye
Copy link
Contributor

@thatblindgeye thatblindgeye commented Jul 28, 2022

What: Closes #7461

Composable flyout menu demo
Nav flyout example

  • Because VO's VO modifier + space registers as a click event, I added a check for a "click" mouse event for the handleFlyout method.
    • If there are no issues with this addition, this could also open up the possibility for allowing flyout menus to be opened vis mouse hover or mouse click as well.
  • I updated the logic for adding a role of "menuitem" to the menu buttons so that it does not get applied if the menu item has a flyout menu. Previously menu items with flyouts weren't announcing that there was a popup, now they do (and VO still registers the item as part of the menu list)
  • I added the same aria-expanded and aria-haspopup props to the Nav Item, following the logic from Menu Item. This will at least provide context that the nav item a) has a popup menu, and b) is expanded/collapsed, and you can choose between clicking the link with VO + space, or opening the flyout with just space. I believe this is about as close as we can get to resolving the flyout issue for the Nav component unless one of the following is done:
    • We recommend that flyouts not be put on nav items that are also links that can be clicked (I don't think this would really be feasible, though it would align with the drilldown nav example)
    • We move the toggle icon out of the <a> element to make it a sibling instead, and make it an actual button that can be used to click on and open the flyout. Similar to the W3C flyout button as toggle example (this would require an update in Core).

Additional issues:
N/A

@patternfly-build
Copy link
Collaborator

patternfly-build commented Jul 28, 2022

@nicolethoen
Copy link
Contributor

As far as I can tell on an initial pass through, Menu works great with VO! Good job!

I'm not sure based on your description in the issue if you're expecting the VO for Nav flyouts to work an be a little less than ideal - but I cannot get a submenu nav item to flyout at all
Jul-29-2022 07-50-27

@thatblindgeye
Copy link
Contributor Author

I'm not sure based on your description in the issue if you're expecting the VO for Nav flyouts to work an be a little less than ideal - but I cannot get a submenu nav item to flyout at all

Yeah the Nav flyout example I don't think makes it entirely clear how to navigate/operate things. It also doesn't help that with VO active (and I believe NVDA is the same as well), the Left/Right Arrow keys on their own navigate through the content of the page, so I don't believe we'd be able to rely on pressing those keys to open/close flyout menus. I should have included that in the original issue content, as what I wrote does make it sound like pressing Right Arrow with VO active would open the flyout.

I'm able to get the Nav flyout to open by pressing only Space, then pressing VO + Space or just Enter acts as clicking the link. My initial thought was that the text "menu popup collapsed" might indicate to users that it acts somewhat like a button and to press Space to open the menu, and the "[un]visited link" text would indicate that pressing Enter would click the link. But that is also a big assumption to make on my part.

Copy link
Collaborator

@wise-king-sullyman wise-king-sullyman left a comment

Choose a reason for hiding this comment

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

It seems that in Firefox the nav flyout is missing the "menu pop up" announcement doesn't work with VO, which leaves the a11y for it about how it was before this change.

The composable flyout does seem substantially improved though! Awesome work on that! If you would want to open a followup issue for the nav portion of this I'd be happy to approve this now 🙂

Copy link
Collaborator

@wise-king-sullyman wise-king-sullyman left a comment

Choose a reason for hiding this comment

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

🚀

@nicolethoen nicolethoen merged commit 6ad76cc into patternfly:main Aug 1, 2022
@patternfly-build
Copy link
Collaborator

Your changes have been released in:

  • eslint-plugin-patternfly-react@4.67.1
  • @patternfly/react-catalog-view-extension@4.79.1
  • @patternfly/react-charts@6.81.1
  • @patternfly/react-code-editor@4.69.1
  • @patternfly/react-console@4.79.1
  • @patternfly/react-core@4.228.1
  • @patternfly/react-docs@5.89.1
  • @patternfly/react-icons@4.79.1
  • @patternfly/react-inline-edit-extension@4.73.1
  • demo-app-ts@4.188.1
  • @patternfly/react-integration@4.190.1
  • @patternfly/react-log-viewer@4.73.1
  • @patternfly/react-styles@4.78.1
  • @patternfly/react-table@4.97.1
  • @patternfly/react-tokens@4.80.1
  • @patternfly/react-topology@4.75.1
  • @patternfly/react-virtualized-extension@4.75.1
  • transformer-cjs-imports@4.66.1

Thanks for your contribution! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug -Menu - composable flyout isn't accessible via VO

4 participants