-
Notifications
You must be signed in to change notification settings - Fork 87
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
Refactor/pagination a11y #516
Conversation
ChrisPaj
commented
Aug 10, 2021
- @media screen and (max-width: 639px): width 100%
- no doubled button borders
- introduce prop 'small'
- button:focus with visible outlines
- adapt font size to 12px
- introduce button:active state
Your Render PR Server URL is https://scale-storybook-staging-pr-516.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-c498mbs6fj33r8n61i20. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ChrisPaj
I left some comments 🤓
Please make sure all checks pass (prettier is failing because of the index.html, but you can leave that one alone).
Also, we don't want to merge this in main
yet, we need the a11y approval first. So we keep the branch, and keep it up-to-date with main
so it's easier to merge when the time comes.
packages/storybook-vue/stories/3_components/pagination/Pagination.stories.mdx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @ChrisPaj, one more small thing 🙏
packages/storybook-vue/stories/3_components/pagination/Pagination.stories.mdx
Outdated
Show resolved
Hide resolved
@ChrisPaj the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are not using scale icons for next / prev? |
|
I left them all inlined because they behave differently in HCM, would it be possible to add the other "double arrow" icons added to the library? I did reduce the size of the inlined svgs to match the |
We're sending it out and closing for now until we get more feedback. (The visual tests need to be updated!) |
Your Render PR Server URL is https://scale-storybook-staging-pr-516.onrender.com. Follow its progress at https://dashboard.render.com/web/srv-c4k9i84objd2e88g9i50. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The visual tests need to be updated.