Stacks
Stacks copied to clipboard
refactor(button): style with pseudo-private custom properties
This PR refactors the CSS for .s-btn
to use pseudo-private custom properties.
Benefits
Reduced fragility, increased maintainability
By defining a subset of rules that are modified by custom properties on a per-variant-basis, there's lower likelihood of the cascade biting us. With a handful of exceptions, .s-btn
rules are set at the component level and only custom properties are modified within variants and modifiers without the need to rewrite boilerplate CSS.
Bugs fixes
I noticed a few issues that I resolved by preventing CSS rule conflicts.
High contrast primary button active state
In develop
, the high contrast primary button style doesn't change on :active
. This PR resolves that.
High contrast borders
Some buttons variants had inconsistent borders in high contrast (Primary style selected state, for example). This PR resolves those issues.
Bundle size 🤷♂️
This PR drops ~2KB from stacks.min.css
(though I suspect this could be a wash when gzipped. TBD)
Deploy Preview for stacks ready!
Name | Link |
---|---|
Latest commit | 77c425c9c1f68cb8d06015f89a58ba180b22ee6d |
Latest deploy log | https://app.netlify.com/sites/stacks/deploys/636e9e8edca3850009c90996 |
Deploy Preview | https://deploy-preview-1038--stacks.netlify.app |
Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site settings.
Excellent work! Visually, this PR doesn't appear to introduce any regressions (tested all combos between light/dark, standard/custom theme, highcontrast on/off). Code-wise, the organization of this file has been greatly improved (dropping ~200 lines of duplicated selectors/properties in the process).
🎉
I do have a few easily addressed concerns:
1. Looks like the removal of the documented `s-btn-group--container` class is a breaking change. I'd prefer to avoid this if at all possible. Any chance we can put a shim in here with a deprecation note in the changelog?
Yeah, I thought that it could be deprecated and any relevant CSS could be removed, but I guess I didn't compensate for this properly (bottom row includes form.s-btn-group--container
):
I'll have to reconsider removing this class before merging this PR.
2. Radio based buttons don't have focus rings. Since this syntax is new to this PR, we could do one of the following: * just add the checked selectors to the focused selectors, much like you did for `.is-selected` * change the markup to have the input _inside_ of the label, then drop the special selector casing for `:focus-within` (in addition to `:focus`)
Fixed in https://github.com/StackExchange/Stacks/pull/1038/commits/e946105efad6abb90de5368385556c9ffc415a01
3. I also spotted an existing bug you can fix while you're in here: Primary buttons don't change their BG color when hovering over them in _dark mode_
Ah, nice catch! I'll fix that. Update: fixed in e519ab4