Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(pagination): support accessible markup #1870

Merged
merged 3 commits into from
Dec 12, 2024

Conversation

dancormier
Copy link
Contributor

@dancormier dancormier commented Dec 11, 2024

STACKS-687, A11Y-122


This PR introduces support for more accessible markup of the pagination component. Existing implementations using the previous markup should render as they did before.

Notable changes

  • In order for pagination item heights to render as desired with the new markup, this PR adds display: inline-block to these items.
  • I've added a deprecation notice that recommends consumers migrate to the new markup.
  • In addition to modifying the markup used in visual tests, I've added legacy markup for child elements using the deprecated structure. I figure this will help us identify any issues that could arise when rendering the deprecated markup.

To test

Copy link

netlify bot commented Dec 11, 2024

Deploy Preview for stacks ready!

Name Link
🔨 Latest commit 2604c4e
🔍 Latest deploy log https://app.netlify.com/sites/stacks/deploys/675b550499a7a00008b53d6f
😎 Deploy Preview https://deploy-preview-1870--stacks.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@dancormier dancormier requested a review from a team December 11, 2024 23:24
Copy link
Contributor

@giamir giamir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @dancormier. I have only left a couple of suggestions but I would be ok to merge the PR even as it is. I will leave it up to you if you think it makes sense to follow up on them.

Thanks for taking care of this a11y bug. 🎉

docs/product/components/pagination.html Outdated Show resolved Hide resolved
screenshots/Chromium/baseline/s-pagination-dark-legacy.ico Outdated Show resolved Hide resolved
docs/product/components/pagination.html Outdated Show resolved Hide resolved
@dancormier
Copy link
Contributor Author

@giamir thank you for your review. I've made the changes you've suggested and I plan to merge this today. If you see anything you think could use revision, let me know!

@dancormier dancormier merged commit 001a452 into develop Dec 12, 2024
11 checks passed
@dancormier dancormier deleted the STACKS-687/accessible-pagination-markup branch December 12, 2024 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants