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

feat(Menu): add footer, add view more demo #5791

Merged
merged 1 commit into from
May 27, 2021

Conversation

kmcfaul
Copy link
Contributor

@kmcfaul kmcfaul commented May 17, 2021

What: Closes #5699

In this PR:

  • Adds MenuFooter
  • Adds "view more" menu demo

Notes:

  • In the "view more" demo, when the complete options list is displayed, the "view more" button will disappear.
  • For the menu footer keyboard interaction, I followed a previous menu pattern. Tab shifts focus between the active menu item and footer (as in the breadcrumb drilldown interaction). The content of the footer has no pre-built keyboard handling.
  • For the "view more" menu demo keyboard interaction, ArrowUp/ArrowDown navigates through the options list and the "view more" button as usual. The demo hooks in and manages the Space/Enter keydown for the "view more" button, focusing the next valid option from where the "view more" button used to be.

@patternfly-build
Copy link
Contributor

patternfly-build commented May 17, 2021

@kmcfaul
Copy link
Contributor Author

kmcfaul commented May 17, 2021

@jessiehuff Let me know what you think of the keyboard interactions so far, especially regarding the footer.

The built in menu keyboard interaction currently only searches for all menu list items, so it would require adjustment to get the menu to cleanly navigate through outer elements with the arrow keys (if we wanted to go that way). It would be doable but a bit tricky.

Copy link
Contributor

@jessiehuff jessiehuff left a comment

Choose a reason for hiding this comment

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

The keyboard interaction looks really good with this!! You even shifted focus with the view more example which is awesome! 😄 I noticed some weirdness with VO on the view more example (when the view more button is pressed it didn't shift to the next item and said that the list is over). I'm going to try to investigate an aria-activedescendant approach that I wonder if would help this.

@kmcfaul kmcfaul requested a review from karelhala May 20, 2021 17:39
Copy link
Contributor

@karelhala karelhala left a comment

Choose a reason for hiding this comment

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

This looks really good!

Copy link
Member

@mcarrano mcarrano left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Copy link
Contributor

@mcoker mcoker left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@jessiehuff jessiehuff left a comment

Choose a reason for hiding this comment

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

I had difficulty getting my aria-activedescendant approach to work, and I don't want to hold up the PR for it so I created an issue for us to investigate that specific problem. Otherwise looks good! 🙂

@tlabaj tlabaj merged commit dfb53aa into patternfly:master May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add view more and footer to Menu
8 participants