search

helpscout / hsds-react

Help Scout Design System (HSDS) — React Component Library
MIT License
86 stars 17 forks source link

HSDS-213 a11y pagination #987

Closed plbabin closed 3 years ago

plbabin commented 3 years ago

CleanShot 2021-10-21 at 13 20 43

We realized that there were some issues with the pagination component related to a11y. Axe core was throwing some warnings and they are no visual focus indicators when tabbing via keyboard on the navigation buttons

This update fixes those issues. We applied a role="navigation" to the component. We also added a focus ring on the pagination button. For some reason, the Button component has a focus ring only for primary, secondary and tertiary kind. To fix the issue, we brought the focusRing mixin from #975 to this PR. This mixin allows any component to gain a blue ring when focusing with a keyboard.

Others major-ish changes

While we were in that file, we refactor the component to be a functional one. That allows us to create smaller components (all within that file) and also we extracted the page/range data in a custom hook. Overall it feels cleaner and simpler

We also rewrote all tests with react-testing-library 🎉

netlify[bot] commented 3 years ago

✔️ Deploy Preview for hsds-react ready!

🔨 Explore the source changes: 13f9dacfcda45bcacfb61fce7618b06c547f1942

🔍 Inspect the deploy log: https://app.netlify.com/sites/hsds-react/deploys/6171a8b7725fb80008f72d9f

😎 Browse the preview: https://deploy-preview-987--hsds-react.netlify.app

digitaldesigner commented 3 years ago

Love it @cen10 @plbabin

plbabin commented 3 years ago

@malikalim do you mind having a quick look on the netlify environment, just in case you have a11y recommendations on things we could have missed (since you are the original reporter of the issue)

plbabin commented 3 years ago

looking really clean 😎 I like it. since this is HSDS, there isn't a way to run cy-tests. let me know when this is merged in so I can check the builds please? 👀

@cen10 do we have a PR where we could squeeze that HSDS update? Or does it needs to be within hs-app if we want to run cypress on it?

cen10 commented 3 years ago

@plbabin We don't have any cypress related pagination tests that I'm aware of in hs-app-ui, at least not for the convo list page (they'll be there eventually). We'd need to make sure it doesn't break things in hs-app anyway b/c at the very least the Customers app uses it, not sure where else. Although I don't know that we're actually testing that with Cypress off the top of my head.