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(components): [collapse] fix accessibility issues #13832

Merged
merged 6 commits into from
Sep 12, 2023
Merged

fix(components): [collapse] fix accessibility issues #13832

merged 6 commits into from
Sep 12, 2023

Conversation

Karolis-Stoncius
Copy link
Contributor

@Karolis-Stoncius Karolis-Stoncius commented Aug 2, 2023

Using tab related aria attributes results in accessibility issues

Please make sure these boxes are checked before submitting your PR, thank you!

  • Make sure you follow contributing guide English | (中文 | Español | Français).
  • Make sure you are merging your commits to dev branch.
  • Add some descriptions and refer to relative issues for your PR.

Description

🤖 Generated by Copilot at fbef87b

The pull request improves the accessibility and semantics of the collapse component by using <button> elements instead of <div> elements for the collapse item headers, and by adjusting the role and aria- attributes accordingly. It also refactors some code and adds some CSS styles to maintain the appearance of the component.

Related Issue

Fixes #___.

Explanation of Changes

🤖 Generated by Copilot at fbef87b

  • Replace <div> elements with <button> elements for collapse item headers to improve accessibility and semantics (link, link)
  • Remove unnecessary role and aria attributes from collapse component and collapse item content (link, link)
  • Move code from collapse.vue to collapse-item.vue to simplify component structure (link)

Using tab related aria attributes results in accessibility issues
@pull-request-triage
Copy link

👋 @Karolis-Stoncius, seems like this is your first time contribution to element-plus.

  • Please make sure that you have read our guidelines and code of conduct before making a contribution.
  • You can comment with /label Components:[component_name] to add a label for which component you are working on.
  • You may join our Discord for staying tuned.

@pull-request-triage pull-request-triage bot added 1st contribution Their very first contribution Needs Review labels Aug 2, 2023
@github-actions
Copy link

github-actions bot commented Aug 2, 2023

@github-actions
Copy link

github-actions bot commented Aug 2, 2023

Hello @Karolis-Stoncius, thank you for contributing to element-plus, please see our guideline to see how to make contribution

@Karolis-Stoncius Karolis-Stoncius marked this pull request as ready for review August 2, 2023 14:06
@github-actions
Copy link

github-actions bot commented Aug 9, 2023

🧪 Playground Preview: https://element-plus.run/?pr=13832
Please comment the example via this playground if needed.

@github-actions github-actions bot added the CommitMessage::Qualified Qualified commit message label Aug 23, 2023
@Karolis-Stoncius
Copy link
Contributor Author

@btea can you take a look?

@btea
Copy link
Collaborator

btea commented Sep 11, 2023

I have limited knowledge of the content related to a11y and may need you to review it. @tolking

Copy link
Member

@tolking tolking left a comment

Choose a reason for hiding this comment

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

LGTM

@btea btea merged commit d791229 into element-plus:dev Sep 12, 2023
10 checks passed
@element-bot element-bot mentioned this pull request Sep 14, 2023
3 tasks
@btea
Copy link
Collaborator

btea commented Sep 16, 2023

@Karolis-Stoncius Based on the problem with #14304, perhaps we should switch back to div.

@btea
Copy link
Collaborator

btea commented Sep 16, 2023

Yeah, this is a feature of form, but changing the div to a button will cause this kind of problem, so this may be a breaking change for previous users and cause problems with 14034.

@Karolis-Stoncius
Copy link
Contributor Author

Yeah, this is a feature of form, but changing the div to a button will cause this kind of problem, so this may be a breaking change for previous users and cause problems with 14034.

Yeah, you're right. Changing it back to div will bring back the a11y issues tho. Adding role="button" seems to fix it https://stackoverflow.com/a/66294321

@btea
Copy link
Collaborator

btea commented Sep 16, 2023

Could you create a new PR to fix it? Thank you!

consultation-applio pushed a commit to consultation-applio/element-plus that referenced this pull request Nov 10, 2023
)

Using tab related aria attributes results in accessibility issues
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants