-
Notifications
You must be signed in to change notification settings - Fork 120
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
LG-3865: Update BassCSS link buttons to USWDS #4825
Conversation
**Why**: Fix unstyled button link appearance
**Why**: We should be able to expect that the visual appearance of BassCSS `btn-link` and the USWDS unstyled button are identical. As such, migrating to the unstyled button can happen independently of the broader BassCSS migration to simplify future effort. Excludes SimpleForm buttons, since all SimpleForm buttons will apply the `btn btn-primary` class which may conflict with USWDS buttons. This is also why the class isn't yet deprecated in the erb-lint.yml file. See: https://github.com/18F/identity-idp/blob/ce7608a50ea9f7b8d8c747c63cc0c49549eac4a7/config/initializers/simple_form.rb#L5
See inline comment
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.
LGTM
**Why**: See inline code comment
**Why**: 18F/identity-design-system#202 and 18F/identity-design-system#201 intended to inherit font smoothing, but unfortunately miss these two states which are handled and inherited from USWDS.
.usa-button--unstyled { | ||
&:hover, | ||
&:active { | ||
// Temporary: These styles should be ported upstream to the design system, optionally as part of | ||
// future reconciliation effort with uswds/uswds#4077. | ||
-moz-osx-font-smoothing: inherit; | ||
-webkit-font-smoothing: inherit; | ||
} | ||
} |
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.
This is becoming a bit of an embarrassing game of whack-a-mole, since this was already tried to be fixed across two separate patch releases in 18F/identity-design-system#201 and 18F/identity-design-system#202 🤦
The problem now is that the first set of changes in 18F/identity-design-system#201 reset font smoothing for hover and active states, but notably only for the explicit --hover
and --active
class modifiers that USWDS doesn't handle (uswds/uswds#4077). The native :hover
and :active
states are inherited from USWDS, which applies its opinionated font smoothing (source), and takes precedent over the base styles that were addressed in 18F/identity-design-system#202.
For future follow-up:
- We could do yet another patch release
- Or, wait to reconcile this 'til if / when Button: Include explicit state classes in unstyled hover, active overrides uswds/uswds#4077 is merged and published
- Or, wait to reconcile this 'til if / when Buttons: Use "inherit" for font smoothing reset uswds/uswds#4115 is merged and published
- Or, propose an upstream change to USWDS to avoid the potentially-redundant hover/active state font smoothing styles, which would further simplify wrangling various states
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.
For future follow-up:
See: #206
Why: We should be able to expect that the visual appearance of BassCSS
btn-link
and the USWDS unstyled button are identical. As such, migrating to the unstyled button can happen independently of the broader BassCSS migration to simplify future effort.Excludes SimpleForm buttons, since all SimpleForm buttons will apply the
btn btn-primary
class which may conflict with USWDS buttons. This is also why the class isn't yet deprecated in the erb-lint.yml file.