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: a11y issues for expandable #138

Merged
merged 19 commits into from
Oct 26, 2023
Merged

Conversation

felicia-haggqvist
Copy link
Contributor

@felicia-haggqvist felicia-haggqvist commented Oct 23, 2023

Fix a11y issues for expandable component in WARP-326 :

  • Switch between ChevronDownIcon and ChevronUpIcon depending on if expanded is set to true or not. This will set the correct svg-title for the icons.
  • Move Icon in the "Box With Custom Icon" story to after the text.
  • Add a comma with sr-only to add a pause between the text and the svg-title for screen readers.
  • Change text "I am expandable" to a <h2> instead of a <h1>

@magnuh
Copy link
Collaborator

magnuh commented Oct 23, 2023

Looks like you are using tabs instead of spaces for indentation? 😅

* The state of the chevron-up icon, if set to true, chevron up should be displayed, if set to false then chevron-down icon should be displayed.
* @default false
*/
showChevronUpIcon?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why can't we use expanded for that purpose? If it's true, chevron down should be displayed and the opposite.

Copy link
Contributor Author

@felicia-haggqvist felicia-haggqvist Oct 25, 2023

Choose a reason for hiding this comment

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

I tried that. It displays the chevron down and vs. but the animation doesn't work then. That's why I added the showChevronUpIcon-prop. I also added it as a prop because I saw that we had an example in the expandable story "Animated Expanded" where we wanted the Expandable-component to already be expanded. :)

@github-actions
Copy link
Contributor

github-actions bot commented Oct 25, 2023

PR Preview Action v1.4.4
Preview removed because the pull request was closed.
2023-10-26 09:36 UTC

package.json Outdated Show resolved Hide resolved
@felicia-haggqvist felicia-haggqvist merged commit ff8fecc into next Oct 26, 2023
3 checks passed
@felicia-haggqvist felicia-haggqvist deleted the fix/a11y-for-expandable branch October 26, 2023 09:35
github-actions bot pushed a commit that referenced this pull request Oct 26, 2023
## [1.1.2-next.6](v1.1.2-next.5...v1.1.2-next.6) (2023-10-26)

### Bug Fixes

* a11y issues for expandable ([#138](#138)) ([ff8fecc](ff8fecc))
github-actions bot pushed a commit that referenced this pull request Nov 6, 2023
# [1.2.0](v1.1.1...v1.2.0) (2023-11-06)

### Bug Fixes

* a11y adjust attention ([#127](#127)) ([6fba30c](6fba30c))
* a11y fixes for select component ([#146](#146)) ([d4d8c18](d4d8c18))
* a11y issues for expandable ([#138](#138)) ([ff8fecc](ff8fecc))
* activate translations in steps ([#144](#144)) ([8eb603e](8eb603e))
* add a11y attributes for to define steps in more accessible way ([#140](#140)) ([47af229](47af229))
* Add role property to Box component ([#139](#139)) ([e46db48](e46db48))
* adjust further a11y issues attention ([#147](#147)) ([b668aeb](b668aeb))
* crowdin build step ([#142](#142)) ([f2ace37](f2ace37))

### Features

* **attention:** add close button ([#149](#149)) ([6a5c9d6](6a5c9d6))
* **attention:** add highlight variant ([#145](#145)) ([1b86124](1b86124))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants