fix(menu): added display hidden to :not(.pf-m-current-path) .pf-c-menu

[mattnolting]

closes #4870

After testing, the removal of pf-m-hidden and pf-m-visible modifiers has no affect on the drilldown variant for .pf-c-menu__list-item. Neither class is present in the DOM or used in any of the interaction states. We need to retain the class to prevent a breaking change, but the mixin isn't necessary to use.

[patternfly-build]

Preview: https://patternfly-pr-4980.surge.sh

A11y report: https://patternfly-pr-4980-a11y.surge.sh

[mcoker]

TIL about this https://codepen.io/mcoker/pen/BarWpjy! I didn't know that was possible.

Traditionally we've required both display: none and visibility: hidden on an element to hide it across all the things we cared about - this will remove visibility from the hidden/visible classes on list items if users are hiding/showing them for other reasons. Is that OK?

Does display: none on the nested/hidden menus not work? Changing it in the react component like Eric mentioned seems to work - the parent menu height is still computed correctly.

[mattnolting]

@mcoker after some more testing, the issue appears to be with &:not(.pf-m-current-path) .pf-c-menu not also being set to display: none. Try this solution out @thatblindgeye and let me know what you think.

[mattnolting]

@mcoker @thatblindgeye There's also an issue with pf-c-divider being counted in child DOM elements, so VO announces the second li after a divider as "{title} item 3 of 6" instead of "{title} 2 of 5". I think we either need to aria-hidden="true" this, as we did with the nested table column headers, or hide otherwise.

[mcoker]

🥳

[thatblindgeye]

Tested on both the React menu drilldown and nav drilldown examples and it looks to work great! 🎉 I did initially notice at first on the nav drilldown when trying to apply the styles to the appropriate selector that the second level menu (after clicking the "Item 1" item) wasn't displaying, but after trying again I wasn't able to replicate the issue.

[patternfly-build]

🎉 This PR is included in version 4.204.0 🎉

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀