fix(pagination): fixed display for per page menu toggle#6586
fix(pagination): fixed display for per page menu toggle#6586dlabaj merged 4 commits intopatternfly:mainfrom
Conversation
|
Preview: https://patternfly-pr-6586.surge.sh A11y report: https://patternfly-pr-6586-a11y.surge.sh |
There was a problem hiding this comment.
Just one comment below. This would also close out #6094 I assume?
| menu-toggle--IsPlain=true | ||
| menu-toggle--IsText=true | ||
| menu-toggle--modifier=pagination-menu-toggle--modifier menu-toggle--id=pagination-menu-toggle--id | ||
| menu-toggle--attribute='aria-label="Items per page"'}} |
There was a problem hiding this comment.
Not sure if we want to override the accessible name here. "Items per page" is accurate, but the context of the "item range of total items" is something we probably want to keep exposed.
There was a problem hiding this comment.
@thatblindgeye nice, that aria-label was borrowed from the old options menu - took it out.
re: #6094, at least one difference I'd love to see for v6 (since it's breaking) is a .pf-[v]-c-pagination__per-page-menu (or something named better than that anyways!) to hold that menu/menu-toggle and we show/hide that element instead. wdyt? Otherwise it's probably the same update. WDYT?
There was a problem hiding this comment.
Wow, fabulous explanation of the changes. 🌟 Thank you for that!
remove aria-label
|
🎉 This PR is included in version 5.4.0-prerelease.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
* fix(release): updated code to release patch to npmjs * fix(TreeView): improved a11y experience (#6480) * fix(TreeView): improved a11y experience * Updated aria-label to match React PR * Updated tabindices * Reverted changes affecting tabindices * fix(pagination): fixed display for per page menu toggle (#6586) * fix(pagination): fixed display for per page menu toggle * chore(pagination): s/pagination-options-menupagination-menu-toggle * chore(pagination): remove top expanded, add toggle aria-label * Update pagination-menu-toggle.hbs remove aria-label --------- Co-authored-by: Eric Olkowski <70952936+thatblindgeye@users.noreply.github.com> Co-authored-by: Michael Coker <35148959+mcoker@users.noreply.github.com>
|
🎉 This PR is included in version 5.4.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fixes #6582
Also spotted when working on patternfly/patternfly-react#9783
What was broken?
display: block, which happened when we swapped out the options menu css for menu toggle for the v5 release - fix(Pagination): updated styles to target menu toggle #5619. Settingdisplay: blockon the options menu worked fine, but the menu toggle is a flex layout. Surprisingly though, that didn't cause an issue until...__controlselement was updated todisplay: flex- fix(menu-toggle): added support for validation/status #6434. When the menu toggle is a flex layout (which it is by default), that's fine - however with the menu toggle as a block layout, that change forces the controls element (holds the toggle arrow) onto a new line causing this bug.Changes in this PR
CSS
We hide/show the menu toggle quite a bit in pagination depending on viewport size and variations/modifiers used. This PR adds a base var that holds the default display type for the menu toggle (
--#{$pagination}--c-menu-toggle--Display--base: inline-flex;) and replaces all of the places we're showing the menu toggle viadisplay: inline-flexordisplay: blockto use that var.HBS
Updates pagination to use the menu toggle, and various updates to support that change, more info on that below called out in the individual commits.
First commit is the styling update and pulling in the menu toggle to the example.
Second commit is just a
s/pagination-options-menu/pagination-menu-toggle/gfrom/src/patternfly. That puts a lot of noise in the "files changed" tab, but it's all just that search/replace.Last commit
aria-labelto the pagination menu toggleidstrings from "options-menu" to "menu-toggle"