Docs-Blocks: Improve ArgsTable border visibility in dark mode#34756
Docs-Blocks: Improve ArgsTable border visibility in dark mode#34756kimjune01 wants to merge 1 commit into
Conversation
appBorderColor uses white at 10% opacity, which is nearly invisible against dark backgrounds. Boost opacity with polished/opacify so table borders are distinguishable. Also increase dark-mode drop-shadow strength from 0.20 to 0.40 for better contrast. Refs storybookjs#34000
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR updates ChangesArgsTable Dark Mode Table Border Styling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| // In dark mode, appBorderColor (white at 10% opacity) is nearly invisible. | ||
| // Boost opacity so table borders are distinguishable from the background. | ||
| const tableBorderColor = | ||
| theme.base === 'dark' ? opacify(0.1, theme.appBorderColor) : theme.appBorderColor; |
There was a problem hiding this comment.
This helps us move from a 1.36 contrast to a 1.91 contrast, both under the 3.0 requirement set by WCAG SC 1.4.11.
We'd need to go all the way to 34% opacity (compared to 10% in next and 20% here) to reach the required ratio.
@MichaelArestad @domyen I'll put the question to you: do we want to change contrast ratios all the way till we reach 3.0? Do we want to find other treatments (alternating background colours)? Do we want to argue that visual grouping and alignment are enough to identify the table element, and so SC 1.4.11 doesn't apply?
There was a problem hiding this comment.
Hey @kimjune01!!! Thanks for the PR. I took a look at this and the accessibility guidelines and, ultimately, I don't think I we should make this change for a few reasons:
- The specification largely refers to that contrast level being necessary on controls or important imagery. I don't think that quite applies here.
- It would have a pretty big visual impact on the base theme. That could result in a significant increase in visual noise across the UI requiring us to do some serious design work to reduce the number of borders used.
- If we made that change in the dark theme, we would probably want to bump contrast on the light theme.
I am hoping, in the future, to make it easier for users to bump the contrast of borders and more on their individual Storybooks (possible now by making a custom theme), but we need to push a bit further with work on Design Tokens and the elusive Theming 2.0. I am also hoping that work can unlock specific themes just for accessibility purposes like a high contrast theme.
There was a problem hiding this comment.
Thanks Michael. I'll close the PR for now. Thanks again for your contribution @kimjune01!
Sidnioulz
left a comment
There was a problem hiding this comment.
Thanks for the PR @kimjune01!
This change is an improvement to visibility, but if contrast requirements apply here, the change isn't strong enough to meet them.
I've asked our design team to provide guidance on how to best fix this.

Refs #34000
What I did
appBorderColoruses white at 10% opacity, which is nearly invisible against dark backgrounds. The ArgsTable borders disappear in dark mode.Boost the border opacity in dark mode using
polished/opacifyso table borders are distinguishable from the background. Also increase the dark-mode drop-shadow strength from 0.20 to 0.40 for better depth separation.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
yarn task --task sandbox --start-from auto --template react-vite/default-tsDocumentation
Summary by CodeRabbit