feat(menu-toggle): remove extra div, reorder examples#6321
feat(menu-toggle): remove extra div, reorder examples#6321mcoker merged 3 commits intopatternfly:v6from
Conversation
|
Preview: https://patternfly-pr-6321.surge.sh A11y report: https://patternfly-pr-6321-a11y.surge.sh |
There was a problem hiding this comment.
LGTM! Just one nit. Also we'll need to poke around for plain menu toggles and make sure they have the icon wrapper. If we use the suggested code in my nit comment, that will let us include/exclude the icon wrapper behind the scenes without needing to go through the code and include/exclude the nested {{> menu-toggle-icon ...}} child that's in the PR now.
| @@ -161,15 +161,15 @@ import './MenuToggle.css' | |||
| ### Plain | |||
| ```hbs | |||
| {{#> menu-toggle menu-toggle--IsPlain=true menu-toggle--attribute='aria-label="Actions"'}} | |||
There was a problem hiding this comment.
I haven't tried it, but the hbs looks like it might be wired up so you can write it this way?
| {{#> menu-toggle menu-toggle--IsPlain=true menu-toggle--attribute='aria-label="Actions"'}} | |
| {{> menu-toggle menu-toggle--IsPlain=true menu-toggle--attribute='aria-label="Actions"' menu-toggle--icon="ellipsis-v"}} |
There was a problem hiding this comment.
@mcoker the additional changes you made for this in the examples/demos all look good.
|
@thatblindgeye @tlabaj this PR updates the plain menu toggle to include the the icon wrapper With this update, looking at the update we'll need to make to the menu toggle component in react, I'm guessing that might mean that an icon is always passed as the |
|
Just wanted to show a screenshot from React after adding the icon wrapper around the plain toggle's svg icon and updating the styles to remove the margin and line-height from that wrapper:
Just to check that it looks as expected in React. As for any necessary updates in React, it may depend how much we want to restrict passing anything as children to MenuToggle. Right now a user could pass whatever: "Toggle text", So we could update prop descriptions and examples to convey that we recommend passing all icons to the Alternatively we could restrict the type that can be passed to children to just a string or creating a new "text" prop to replace children, neither of which sound all to intuitive or great. Here's what it may look like passing the icon to the
Depending where we land on this, I can use the open React issue for Penta MenuToggle updates to add this wrapper and make any other necessary updates if we want. |
|
🎉 This PR is included in version 6.0.0-alpha.87 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Fixes #6318
Removes the extra div around some examples.
Reorders within example sets so that the toggles are always collapsed, expanded, disabled
Styles disabled plain version to match the disabled plain button.