USWDS - Pagination: Updated text-decoration value to better show pagination links#5970
Conversation
cathybaptista
left a comment
There was a problem hiding this comment.
Hi, Rachel
I tested this, and your change does mean a color contrast of 5:1 versus 1:1 so I guess this passes? I'm not an alpha pro. Just eyeballing it, the change is much darker. I'll approve it if someone with a little more experience with contrast ratios can just verify that the new color contrast works. :)
mahoneycm
left a comment
There was a problem hiding this comment.
You're off to a good start here @RachelCorsino ! Left a change request with some additional context on the color adjust function and additional pitfalls to look out for.
Feel free to ping me if you want any additional help!
mahoneycm
left a comment
There was a problem hiding this comment.
Made a suggestion for a slightly more noticeable change though the suggestions still cause the the standard border to be quite a bit darker. I'm wondering if we want to improve the overall logic to maintain something closer to the current styles and offering a fallback instead.
It might be good to check in with the rest of the team to get a temperature check!
…ine to pagination border links
amyleadem
left a comment
There was a problem hiding this comment.
Thanks for making the quick update, @RachelCorsino. I added a couple of suggestions below.
Also, can you update the PR title and description to reflect the change in solution approach?
mahoneycm
left a comment
There was a problem hiding this comment.
Looks great! This new approach is simple and effective. One request to update the PR description to match this approach and I'll give it the green ✅
issue (blocking): The PR description doesn't match this new approach.
suggestion: Update PR title, description, and testing instructions to match new text decoration approach.
|
@mahoneycm @amyleadem please move to FFR once both have approved. |
|
PR description has been updated to account for |
amyleadem
left a comment
There was a problem hiding this comment.
Thanks @RachelCorsino.
I noticed there are two other text-decoration style rules in usa-pagination.scss that we should be able to safely remove. Can you check that it is safe to remove those style rules and if so, remove them?
Before approval, we'll also need to attach a changelog PR for this update. (Let me know if you need help creating that).
amyleadem
left a comment
There was a problem hiding this comment.
Looks good to me!
One last update: Can you update the PR description to be a bit clearer? It might seem nitpicky but this description will be pulled into the release notes and will also be used for future reference as part of the component's history. Maybe something like this?
Added text underline styles to pagination links. Pagination links now have the same visual indicators as other USWDS text links. This should improve accessibility for the component.
Can you also update the problem statement since it doesn't quite capture the root issue? Maybe something like this:
Pagination links do not provide accessible link styles. Pagination borders around the page number have poor color contrast. The light gray ( #d1d1d1 ) on the white background in the example has a color contrast ratio of 1.5:1. We should provide a clear visual indication that the pagination list items are links."
Feel free to edit these suggestions for clarity.
amyleadem
left a comment
There was a problem hiding this comment.
LGTM! Thanks @RachelCorsino
mahoneycm
left a comment
There was a problem hiding this comment.
LGTM! Thanks @RachelCorsino !
thisisdano
left a comment
There was a problem hiding this comment.
A nice, simple, effective improvement
Summary
Added text underline styles to pagination links. Pagination links now have the same visual indicators as other USWDS text links. This should improve accessibility for the component.
Breaking change
This is not a breaking change.
Related issue
Closes #5939
Related pull requests
Changelog: Text-Decoration Style Fix for Pagination Component Accessibility #2793
Preview link
Pagination Component Page
Problem statement
Pagination links do not provide accessible link styles. Pagination borders around the page number have poor color contrast. The light gray ( #d1d1d1 ) on the white background in the example has a color contrast ratio of 1.5:1. We should provide a clear visual indication that the pagination list items are links.
Solution
Removed
text-decoration: noneso that the "buttons" are more clearly formatted as links.Testing and review
Follow these steps:
Before opening this PR, make sure you’ve done whichever of these applies to you:
git pull origin [base branch]to pull in the most recent updates from your base and check for merge conflicts. (Often, the base branch isdevelop).npm run prettier:sassto format any Sass updates.npm testand confirm that all tests pass.