Skip to content

Fix spinner button active color#6774

Merged
aduth merged 1 commit into
mainfrom
aduth-spinner-button-active
Aug 17, 2022
Merged

Fix spinner button active color#6774
aduth merged 1 commit into
mainfrom
aduth-spinner-button-active

Conversation

@aduth

@aduth aduth commented Aug 17, 2022

Copy link
Copy Markdown
Contributor

Fixes regression introduced in #6564

Why: Because the spinner button retains its original shape by setting the text color to transparent, which was being overridden by the new active styles introduced in #6564.

Before After
Screen Shot 2022-08-17 at 1 23 48 PM Screen Shot 2022-08-17 at 1 24 02 PM

**Why**: Because the spinner button retains its original shape by setting the text color to transparent, which was being overridden by the new active styles introduced in #6564.

changelog: Bug Fixes, Buttons, Fix spinner button active state appearance
Comment on lines +21 to +22
.usa-button:disabled.usa-button--active:not(.usa-button--unstyled, .usa-button--secondary, .usa-button--accent-cool, .usa-button--accent-warm, .usa-button--base, .usa-button--outline, .usa-button--inverse),
.usa-button--disabled.usa-button--active:not(.usa-button--unstyled, .usa-button--secondary, .usa-button--accent-cool, .usa-button--accent-warm, .usa-button--base, .usa-button--outline, .usa-button--inverse) {

@aduth aduth Aug 17, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not thrilled about this selector, but each of these :not's from the original selector increase specificity, so we must match them in order to override the styles. (We could use !important, but I try to avoid !important at all costs)

The plan is to avoid the :not selectors altogether by providing active, disabled styles for all button states, which may have helped avoid this issue in the first place, or at least will allow us to simplify the selector greatly.

That's tracked at LG-7269.

Specifically, this note in the ticket:

The IdP implementation is limited to the default button, but ideally the design system should include styles for all button variants

@zachmargolis zachmargolis left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yikes, good catch, glad to see the cleanup for those selectors is tracked

LGTM

@aduth aduth merged commit 555fc7d into main Aug 17, 2022
@aduth aduth deleted the aduth-spinner-button-active branch August 17, 2022 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants