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(image-upload): increase text contrast for accepted files string to meet accessibility standards #310

Closed
wants to merge 1 commit into from

Conversation

kristinalustig
Copy link
Collaborator

@kristinalustig kristinalustig commented Apr 24, 2024

Describe your changes

Updating the text contrast for the accepted file types string so that it meets accessibility standards for contrast.

PR Checklist

  • All new/changed functionality includes unit and (optionally) e2e tests as appropriate
  • All new/changed functions have /** ... */ docs
  • I've added the bug/enhancement and other labels as appropriate

Additional context

Matches a change in SO editor.

Copy link

netlify bot commented Apr 24, 2024

Deploy Preview for stacks-editor ready!

Name Link
🔨 Latest commit a4d7d2c
🔍 Latest deploy log https://app.netlify.com/sites/stacks-editor/deploys/662935af95db130008b5338a
😎 Deploy Preview https://deploy-preview-310--stacks-editor.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.

@giamir
Copy link
Contributor

giamir commented Apr 29, 2024

@kristinalustig the color contrast for the "caption" element is already passing our accessibility standard. This would be a cosmetic change rather than an accessibility fix.

Before (black-400 - APCA Lc76)

Screenshot 2024-04-29 at 11 18 23

After (black-500 - APCA Lc94)
Screenshot 2024-04-29 at 11 17 37

Lc76 already passes our Lc60 target for text.

@kristinalustig Could you add a reference to a ticket? I believe a designer was consulted for making this change.
If not perhaps @CGuindon could give us the thumbs up on the change.

Thank you.

@CGuindon
Copy link
Collaborator

Yup, black-400 passes on our background and I would actually prefer to keep it at 400 instead of changing to 500. That way we only have 2 different shades of black in that small area instead of 3 (primary and secondary text). @kristinalustig

@giamir
Copy link
Contributor

giamir commented Apr 30, 2024

@kristinalustig based on @CGuindon's last comment can we close this PR?

@kristinalustig
Copy link
Collaborator Author

@giamir whoops, sorry for the delayed response, this PR got lost in the shuffle. Looks like we don't have to make this change, then. It's fine to close this PR. thanks @CGuindon for weighing in.

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