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

We need to be able to set the role attribute on pf-button #2805

Closed
kelsS opened this issue Jul 9, 2024 · 4 comments
Closed

We need to be able to set the role attribute on pf-button #2805

kelsS opened this issue Jul 9, 2024 · 4 comments
Labels
accessibility Improve accessibility bug needs AT updates Please make updates to the automated testing. needs discussion needs to be discussed outside the PR needs: prioritization To be prioritized

Comments

@kelsS
Copy link
Contributor

kelsS commented Jul 9, 2024

Description of the issue

Devs need to be able to set the role attribute on pf-button. For example, I am using pf-button as a tab control and I need to set the role=tab directly on pf-button in order to follow the proper design pattern for the carousel slider I am working on.

Please see the following link for more info about the carousel design pattern

Impacted component(s)

Steps to reproduce

  1. Open a Codepen in Chrome
  2. Import pf-button into a pen
  3. Try to add role="tab" to the pf-button
  4. Inspect the devtools and observe that the role does not get added

Expected behavior

The role attribute is added to the button in the shadow root with whatever the user needs the role to be, in my case "tab".

This is a sev-2 issue at least because this is causing a very inaccessible experience for cp-slider

@kelsS kelsS added priority: low Severity level: 3 bug accessibility Improve accessibility needs AT updates Please make updates to the automated testing. needs: prioritization To be prioritized labels Jul 9, 2024
@kelsS
Copy link
Contributor Author

kelsS commented Jul 9, 2024

This issue is blocking a CP Ticket for high priority a11y fixes

@kelsS kelsS added priority: high Severity level: 1 and removed priority: low Severity level: 3 labels Jul 9, 2024
@kelsS kelsS added needs discussion needs to be discussed outside the PR and removed priority: high Severity level: 1 labels Jul 9, 2024
@bennypowers
Copy link
Member

#internals = InternalsController.of(this, { role: 'button' });

Button uses ElementInternals to set the role, which should not override user role attr. Perhaps theres an issue with the controller

@bennypowers
Copy link
Member

Please see #2806

@kelsS
Copy link
Contributor Author

kelsS commented Jul 9, 2024

This issue was resolved with @bennypowers PR. Further discussion is needed for how to handle tabs buttons within a carousel slider.

@kelsS kelsS closed this as completed Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility Improve accessibility bug needs AT updates Please make updates to the automated testing. needs discussion needs to be discussed outside the PR needs: prioritization To be prioritized
Projects
None yet
Development

No branches or pull requests

2 participants