chore(Badge/MenuToggle): updated implementation of badge toggle#6430
chore(Badge/MenuToggle): updated implementation of badge toggle#6430mcoker merged 3 commits intopatternfly:v6from
Conversation
| {{> breadcrumb-item-divider}} | ||
| {{#> breadcrumb-dropdown}} | ||
| {{> dropdown dropdown--id="dropdown-badge-toggle" dropdown--template--Breadcrumb="true" dropdown--IsExpanded="true"}} | ||
| {{#> menu-toggle menu-toggle--IsPlain=true}} |
There was a problem hiding this comment.
This example would still need to have Menu added in to fully replace the deprecated dropdown, but for now this resolves updating the example to show using our recommended badge toggle implementation.
There was a problem hiding this comment.
Per discussion off GH, we won't need to add a Menu to this example.
|
Preview: https://patternfly-pr-6430.surge.sh A11y report: https://patternfly-pr-6430-a11y.surge.sh |
There was a problem hiding this comment.
LGTM though I suspect we'll need to address the styling. Doesn't have to be part of this PR though. I'm curious what @andrew-ronaldson and @lboehling think.
Some things worth noting
- The menu toggle has a small variation that may work better?
- We should make sure the height of the breadcrumb doesn't change depending on whether there is a menu toggle in it or not (it changes currently)
- The toggle icon font-size is larger than the breadcrumb and badge text
|
I agree about that the height shouldn't change in breadcrumbs.
|
|
@andrew-ronaldson we have a breadcrumb issue to look at some things and I added a note about the spacing around the dividers #6097 For the menu toggle, maybe we can look at this spacing. This is effectively a plain text menu toggle (@thatblindgeye I think we should use that variant instead of the plain - wdyt?) and here's how that spacing is created. There is a space between the "text" and the toggle icon, then there is a bunch of space around the toggle icon to effectively maintain some sort of minimum width. For this toggle, how would you expect the spacing to look like? IMO this should be a follow-up, and we should at least review the way we're creating that spacing, I think the way it is will not create the spacing in figma. |
8961ad3 to
9713bed
Compare
|
Opened #6463 as a followup |
|
🎉 This PR is included in version 6.0.0-alpha.108 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Closes #6407