ActionList: Fix leaky disabled description styles
- Reported in https://github.com/github/collaboration-workflows-flex/issues/927
Problem
Description in a disabled item should not have color: fg.muted but the disabled color. We set this by adding the disabled color on the item and adding color:inherit on the description. Source
const styles = {
'li[aria-disabled="true"] &': { color: 'inherit' }
}
return <Box as="span" sx={styles}/>{props.children</Box>
This feels safe because we are only targeting Description inside li[aria-disabled=true]. This is the generated css:
li[aria-disabled="true"] .Box-sc-g0xbh4-0 {
color: inherit;
}
The trouble here is that the generated className for '&' is not the className just for ActionList.Description but is passed to multiple elements within ActionList.Item. 🤔
This really threw me off!
Solution
As a quick fix, I added a data-component to make the selector more specific. Expecting no visual changes, only changes in the generated css.
| Before |
|
| After |
|
Rollout strategy
- [x] Patch release
- [ ] Minor release
- [ ] Major release; if selected, include a written rollout or migration plan
- [ ] None; if selected, include a brief description as to why
Testing & Reviewing
Merge checklist
- [ ] Added/updated tests
- [ ] Added/updated documentation
- [x] Added/updated previews (Storybook)
- [x] Changes are SSR compatible
- [x] Tested in Chrome
- [ ] Tested in Firefox
- [x] Tested in Safari
- [ ] Tested in Edge
- [ ] (GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)
🦋 Changeset detected
Latest commit: e995bb9b9eed1356669ec3a614d1575e6fc1767b
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 1 package
| Name | Type |
|---|---|
| @primer/react | Patch |
Not sure what this means? Click here to learn what changesets are.
Click here if you're a maintainer who wants to add another changeset to this PR
size-limit report 📦
| Path | Size |
|---|---|
| packages/react/dist/browser.esm.js | 88.07 KB (+0.04% 🔺) |
| packages/react/dist/browser.umd.js | 88.43 KB (+0.06% 🔺) |
wanted to ask if this is possible to capture in VRT or nah? (Or is it already?)
Maybe but not a clean one. The instance where this happens in gh/gh is also not the best use case for ActionList
:wave: Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/false