-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[Admin] Use aria-disabled
for setting anchor buttons as disabled
#5287
Conversation
Codecov Report
@@ Coverage Diff @@
## nebulab/admin #5287 +/- ##
=================================================
- Coverage 88.52% 88.51% -0.01%
=================================================
Files 597 597
Lines 14523 14518 -5
=================================================
- Hits 12856 12851 -5
Misses 1667 1667
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
25af019
to
5f81491
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @elia 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯
<form> | ||
<%= render @tab_component.new( | ||
text: "All", | ||
'aria-current': true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that the implementation (using a button
under the hood) is hidden, can't we just treat the way we set is as active as an implementation detail? I think it makes more sense as it was before, with the active
and disabled
params determining the state of the button.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Active is still a state that cannot be controlled programmatically, so for that we still need another way to tap into it.
See https://developer.mozilla.org/en-US/docs/Web/CSS/:active#active_form_elements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, sorry. For active
, I meant selected
(the current tab that is selected), not the activation on click.
5f81491
to
5e359eb
Compare
5e359eb
to
8ad4a62
Compare
Summary
Rely on ARIA-attributes instead of custom
data-ui
attributes for showing anchors as active or disabled in theui/tab
andui/button
components.Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: