Skip to content
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

fix(notice): improve contrast of inline code #1677

Merged
merged 2 commits into from
Mar 14, 2024

Conversation

hamvocke
Copy link
Collaborator

The s-notice component currently has insufficient color contrast when used in combination with inline code elements. Here's what it looks like in dark mode, for example:

image

Screenshot 2024-03-11 091710

I'm opening this PR to give us a playground to discuss and fix this problem. I tried to play around with various color combinations for the background and text colors myself but quickly realized that I've got no idea what we're aiming for from a visual perspective. I could tweak the colors until I hit the right color contrast, obviously, but I really need some guidance on the visual design, too.

@dancormier dancormier changed the title A11y: Improve contrast of inline code in s-notice fix(notice): Improve contrast of inline code Mar 11, 2024
@dancormier dancormier changed the title fix(notice): Improve contrast of inline code fix(notice): improve contrast of inline code Mar 11, 2024
@CGuindon
Copy link
Collaborator

I had previously figured out the color combos needed for the comment box — so these should work for the blue notice. To do the other colors, we'll some design time and that's not going to happen until the end of this quarter (we have bug bash I can assign this to someone). You can also verify if the the other colors work with the same color stops but I bet there will be issues with red and yellow.

Screenshot 2024-03-11 at 1 59 40 PM

…1678)

* refactor(notice): generate styles programmatically; improve contrast

* Update notice tests to include  `code`, `a.s-link.s-link__inherit` children

* Fix comment typo

Addresses https://github.com/StackExchange/Stacks/pull/1678/files#r1525084777
@dancormier
Copy link
Contributor

Merging this since the changes here are really just from #1678 which has been reviewed and approved. 🚢

@dancormier dancormier merged commit a2ee5d3 into develop Mar 14, 2024
9 of 10 checks passed
@dancormier dancormier deleted the ham/notice-inline-code branch March 14, 2024 16:40
Copy link

netlify bot commented Mar 14, 2024

Deploy Preview for stacks ready!

Name Link
🔨 Latest commit fa2e469
🔍 Latest deploy log https://app.netlify.com/sites/stacks/deploys/65f327946f41a40008ca16e7
😎 Deploy Preview https://deploy-preview-1677--stacks.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants