-
Notifications
You must be signed in to change notification settings - Fork 67
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(button): changed aria-live to be separate form aria-disabled hover #1645
Conversation
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.
Nice.
0f06cd6
to
358aa34
Compare
@@ -79,7 +79,7 @@ a.fake-btn--primary { | |||
} | |||
|
|||
/* show hover states only for non-disabled or non-loading state button */ | |||
button.btn--primary:not([disabled]):not([aria-live="polite"][aria-disabled="true"]), | |||
button.btn--primary:not([disabled]):not([aria-disabled="true"]), | |||
a.fake-btn--primary[href] { | |||
&:focus, | |||
&:hover, |
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.
should we drop the active here (line 86)? I think maybe I should've with my last PR. Maybe could do in a separate PR if needed, but probably shouldn't be here. Other than that LGTM
Description
Since partially disabled relies on
aria-disabled
to disable hover, this code was picking up hover together witharia-live
. Changed it so they're separate checks.References
#1637