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

feat!: Add and rename Pagination props, remove use of aria-label #1366

Merged
merged 18 commits into from
Jul 26, 2024

Conversation

RubenSibon
Copy link
Contributor

@RubenSibon RubenSibon commented Jul 12, 2024

Describe the pull request

The Amsterdam Design System advices against using aria-label. We should then not use the bad practice in our own components.

What

This branch removes usage of aria-label and replaces it with the labels in spans with the ams-visually-hidden class name.

Additional notes

  • Should we also refactor the properties nextAriaLabel and previousAriaLabel and thus break the API? Yes, the props have been changed, thus breaking the API.
  • A new issue has been made for the Switch story which uses the bad practice. Addressing that in this PR was deemed out-of-scope.

@RubenSibon RubenSibon self-assigned this Jul 12, 2024
packages/react/src/Pagination/Pagination.tsx Outdated Show resolved Hide resolved
storybook/src/components/Switch/Switch.docs.mdx Outdated Show resolved Hide resolved
packages/react/src/Pagination/Pagination.tsx Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to demo-DES-849-aria-label-to-ams-visually-hidden July 12, 2024 14:52 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-849-aria-label-to-ams-visually-hidden July 22, 2024 15:06 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-849-aria-label-to-ams-visually-hidden July 24, 2024 07:28 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-849-aria-label-to-ams-visually-hidden July 24, 2024 07:41 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-849-aria-label-to-ams-visually-hidden July 24, 2024 08:41 Destroyed
Important: breaking change for the Pagination API

This renames Pagination properties to no longer refer to aria-label, but
to visually hidden labels.
…ub.com:Amsterdam/design-system into feat/DES-849-aria-label-to-ams-visually-hidden
@github-actions github-actions bot temporarily deployed to demo-DES-849-aria-label-to-ams-visually-hidden July 24, 2024 10:51 Destroyed
@RubenSibon RubenSibon requested review from alimpens and dlnr July 24, 2024 10:51
@github-actions github-actions bot temporarily deployed to demo-DES-849-aria-label-to-ams-visually-hidden July 24, 2024 10:54 Destroyed
@RubenSibon RubenSibon changed the title feat: Replace aria-label with ams-visually-hidden class name feat!: Replace aria-label with ams-visually-hidden class name Jul 24, 2024
@alimpens alimpens changed the title feat!: Replace aria-label with ams-visually-hidden class name feat!: Add and rename Pagination props, remove use of aria-label Jul 24, 2024
@github-actions github-actions bot temporarily deployed to demo-DES-849-aria-label-to-ams-visually-hidden July 24, 2024 11:33 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-849-aria-label-to-ams-visually-hidden July 24, 2024 11:57 Destroyed
@RubenSibon RubenSibon requested a review from alimpens July 24, 2024 11:58
@github-actions github-actions bot temporarily deployed to demo-DES-849-aria-label-to-ams-visually-hidden July 24, 2024 11:59 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-849-aria-label-to-ams-visually-hidden July 24, 2024 14:23 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-849-aria-label-to-ams-visually-hidden July 26, 2024 08:01 Destroyed
@alimpens alimpens merged commit ce09376 into develop Jul 26, 2024
6 checks passed
@alimpens alimpens deleted the feat/DES-849-aria-label-to-ams-visually-hidden branch July 26, 2024 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants