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.

Sign up for GitHub

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

Jump to bottom

update the link colors on darkmode #4201

Merged
merged 6 commits into from
Oct 9, 2023
Merged

update the link colors on darkmode #4201

merged 6 commits into from
Oct 9, 2023

Conversation

zesuma
Copy link
Contributor

@zesuma zesuma commented Oct 5, 2023 •
edited
Loading

What are you changing in this pull request and why?

https://app.asana.com/0/1200099998847559/1204216689849797/f

Description of the Issue
Review pages and for any links on backgrounds with the color of #262a38 ($neutral100), change the hover color of those links to #1FA4A3 ($teal30) so that they have appropriate contrast per WebAIM requirements.

Links
Use the WebAIM web extension to check contrast
https://wave.webaim.org/extension/

NOTE: I limited the brightness on the links a bit since this was causing unanticipated issues on images washing out. I updated the link color and link hover color to #1FA4A3 on dark mode.

Preview:
https://docs-getdbt-com-git-fix-insufficient-contrast-o-3250d0-dbt-labs.vercel.app/docs/introduction

Checklist

  • Review the Content style guide and About versioning so my content adheres to these guidelines.
  • Add a checklist item for anything that needs to happen before this PR is merged, such as "needs technical review" or "change base branch."

Adding new pages (delete if not applicable):

  • Add page to website/sidebars.js
  • Provide a unique filename for the new page

Removing or renaming existing pages (delete if not applicable):

  • Remove page from website/sidebars.js
  • Add an entry website/static/_redirects
  • Ran link testing to update the links that point to the deleted page

@zesuma zesuma requested a review from a team as a code owner October 5, 2023 16:36
@vercel
Copy link

vercel bot commented Oct 5, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs-getdbt-com ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 9, 2023 4:04pm

@github-actions github-actions bot added the size: small This change will take 1 to 2 days to address label Oct 5, 2023
JKarlavige
JKarlavige requested changes Oct 5, 2023
View reviewed changes
Copy link
Collaborator

@JKarlavige JKarlavige left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The color adjustments look great in dark mode! Would it be possible to make the same edits for links within the blog? https://docs-getdbt-com-git-fix-insufficient-contrast-o-3250d0-dbt-labs.vercel.app/blog

Right now, the blog links go darker on hover and become hard to read in dark mode.

website/src/css/custom.css
Comment on lines +682 to +688
.alert--secondary,
.alert--secondary a,
.alert--secondary svg {
--ifm-alert-background-color: #474748;
color: white !important;
fill: white !important;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know of a page where this is used for me to checkout?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's one with the alert--secondary which the the note alert:
https://docs-getdbt-com-git-fix-insufficient-contrast-o-3250d0-dbt-labs.vercel.app/docs/cloud/connect-data-platform/connect-apache-spark

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch @JKarlavige. I've added style updates for the blog hrefs.

@github-actions github-actions bot added size: medium This change will take up to a week to address and removed size: small This change will take 1 to 2 days to address labels Oct 5, 2023
@zesuma zesuma requested a review from JKarlavige October 5, 2023 21:05
JKarlavige
JKarlavige approved these changes Oct 6, 2023
View reviewed changes
Copy link
Collaborator

@JKarlavige JKarlavige left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!! :shipit:

@zesuma zesuma requested a review from lfracalossiFA October 6, 2023 13:15
breezyfasano
breezyfasano requested changes Oct 9, 2023
View reviewed changes
Copy link
Member

@breezyfasano breezyfasano left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Trever! Woo this is a big one, thanks for submitting this. I love these accessibility updates but changes like these are tricky cuz they touch every page!

I noticed that on the Materializations page, at the very bottom, the Discourse widget CTA is now washed out and does not meet contrast guidelines.
Screenshot 2023-10-09 at 10 34 38 AM

After checking a fair amount of pages, that was the only issue I could find! Nice work.

@zesuma
Copy link
Contributor Author

zesuma commented Oct 9, 2023

Hi Trevor! Woo this is a big one, thanks for submitting this. I love these accessibility updates but changes like these are tricky cuz they touch every page!

I noticed that on the Materializations page, at the very bottom, the Discourse widget CTA is now washed out and does not meet contrast guidelines. Screenshot 2023-10-09 at 10 34 38 AM

After checking a fair amount of pages, that was the only issue I could find! Nice work.

great catch @breezyfasano ! Yeah it was very tricky! You fix one thing and find you've messed up another thing lol

@zesuma zesuma requested a review from breezyfasano October 9, 2023 16:29
@zesuma
Copy link
Contributor Author

zesuma commented Oct 9, 2023

Hi Trevor! Woo this is a big one, thanks for submitting this. I love these accessibility updates but changes like these are tricky cuz they touch every page!
I noticed that on the Materializations page, at the very bottom, the Discourse widget CTA is now washed out and does not meet contrast guidelines. Screenshot 2023-10-09 at 10 34 38 AM
After checking a fair amount of pages, that was the only issue I could find! Nice work.

great catch @breezyfasano ! Yeah it was very tricky! You fix one thing and find you've messed up another thing lol

should be fixed now: https://docs-getdbt-com-git-fix-insufficient-contrast-o-3250d0-dbt-labs.vercel.app/docs/build/materializations

breezyfasano
breezyfasano approved these changes Oct 9, 2023
View reviewed changes
Copy link
Member

@breezyfasano breezyfasano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!! Thanks for fixing Trever @zesuma ! I'll go ahead and merge now.

@breezyfasano breezyfasano merged commit 571abfc into current Oct 9, 2023
7 checks passed
@breezyfasano breezyfasano deleted the Fix-insufficient-contrast-on-dark-background branch October 9, 2023 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@breezyfasano breezyfasano breezyfasano approved these changes

@JKarlavige JKarlavige JKarlavige approved these changes

@lfracalossiFA lfracalossiFA Awaiting requested review from lfracalossiFA

Assignees
No one assigned
Labels
size: medium This change will take up to a week to address
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@zesuma @JKarlavige @breezyfasano