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.

Sign up for GitHub

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

Jump to bottom

feat(select): accessibility improvement on select and option component #588

Merged
merged 3 commits into from
Jul 14, 2023
Merged

feat(select): accessibility improvement on select and option component #588

merged 3 commits into from
Jul 14, 2023

Conversation

MehmetCanBOZ
Copy link
Contributor

This pull request improves the accessibility of select and option components.
Fixes: #587

muratcorlu
muratcorlu reviewed May 24, 2023
View reviewed changes
Copy link
Contributor

@muratcorlu muratcorlu left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! I have some remarks.

src/components/select/bl-select.ts Outdated Show resolved Hide resolved
src/components/select/bl-select.ts Outdated Show resolved Hide resolved
@MehmetCanBOZ MehmetCanBOZ force-pushed the 587-accesibility-improvement-on-select-and-option-component branch 2 times, most recently from a604137 to a02a69d Compare May 24, 2023 10:21
@MehmetCanBOZ MehmetCanBOZ force-pushed the 587-accesibility-improvement-on-select-and-option-component branch from a02a69d to 7f3e7a1 Compare May 24, 2023 13:20
@muratcorlu
Copy link
Contributor

I tried this work on published storybook with voiceover and noticed that we defined role definitions on the wrong elements. I think role and aria attributes should be defined on focusable elements, list-box role on fieldset and option role on the element that has focus-target class.

Can you change them and try with a screen reader to check if it reads select labels properly?

@MehmetCanBOZ MehmetCanBOZ force-pushed the 587-accesibility-improvement-on-select-and-option-component branch from 7f3e7a1 to 33db870 Compare May 26, 2023 09:49
feat(select): accessibility improvement on select and option component
ded78f3 
fix(select): role and ari-label attribute

fix(select): style issue

fix(select): aria-multiselectable attribute

fix(select): role and aria-label attribute for correct accessibility

fix(select): code style
@MehmetCanBOZ MehmetCanBOZ force-pushed the 587-accesibility-improvement-on-select-and-option-component branch from 33db870 to ded78f3 Compare May 26, 2023 09:55
@MehmetCanBOZ
Copy link
Contributor Author

MehmetCanBOZ commented May 26, 2023 •
edited
Loading

Yes, you are correct. I tried several options for the role attribute with VoiceOver, and the best solution that I found is to give role='button', aria-haspopup='listbox' and aria-expanded='{isOpened}' attribute to the fieldset and role='listbox' attribute for class='popover' element. Maybe you can try again with a screen reader and give feedback about it.

muratcorlu
muratcorlu approved these changes Jul 14, 2023
View reviewed changes
Copy link
Contributor

@muratcorlu muratcorlu left a comment

Choose a reason for hiding this comment

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

I checked with voiceover and it looks good to me. Thank you!

@muratcorlu muratcorlu merged commit 0d99a12 into Trendyol:next Jul 14, 2023
2 checks passed
@github-actions
Copy link

🎉 This PR is included in version 2.2.0-beta.17 🎉

The release is available on:

Your semantic-release bot 📦🚀

@muratcorlu muratcorlu mentioned this pull request Jul 14, 2023
Merged
@github-actions
Copy link

🎉 This PR is included in version 2.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@muratcorlu muratcorlu muratcorlu approved these changes

Assignees
No one assigned
Labels
released on @beta released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Accessibility improvements on Select and option component
2 participants
@MehmetCanBOZ @muratcorlu