Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: Accessibility - change color for WCAG AA #19472

Merged
merged 3 commits into from
Oct 25, 2023

Conversation

SphinxKnight
Copy link
Contributor

@SphinxKnight SphinxKnight commented Oct 20, 2023

Comprehensive Summary of your change

Changed the label token foreground and background to comply with WCAG AA (or AAA when possible).

Before:
image
After:
image

Using https://webaim.org/resources/contrastchecker/ as a testing tool, only label-medium was OK for WCAG AA for normal content.

Issue being fixed

No existing issue (I preferred to file a PR first). This could relate to #12327

Notes

I don't have any Sass skill so far and tried to apply a homogeneous patch where such labels were duplicated. Please let me know if I missed something.

result-tip-histogram.component.scss didn't have any issue, but I preferred to match the colors changed elsewhere.

Please indicate you've done the following:

  • Well Written Title and Summary of the PR
  • Label the PR as needed. "release-note/ignore-for-release, release-note/new-feature, release-note/update, release-note/enhancement, release-note/community, release-note/breaking-change, release-note/docs, release-note/infra, release-note/deprecation" << I'm not sure I have the rights to label the PR.
  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Made sure tests are passing and test coverage is added if needed.
  • Considered the docs impact and opened a new docs issue or PR with docs changes if needed in website repository.

@SphinxKnight SphinxKnight requested a review from a team as a code owner October 20, 2023 11:56
@Vad1mo Vad1mo enabled auto-merge (squash) October 23, 2023 09:47
@Vad1mo Vad1mo added the release-note/update Update or Fix label Oct 23, 2023
auto-merge was automatically disabled October 23, 2023 10:27

Head branch was pushed to by a user without write access

@SphinxKnight
Copy link
Contributor Author

SphinxKnight commented Oct 23, 2023

Thanks @Vad1mo for the swift review and sorry about the auto-merge, I just pushed to fix the reason of UI_UT CI job's failure.

@codecov
Copy link

codecov bot commented Oct 23, 2023

Codecov Report

Merging #19472 (3bf7872) into main (adb066c) will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #19472      +/-   ##
==========================================
+ Coverage   67.43%   67.46%   +0.03%     
==========================================
  Files         989      989              
  Lines      108836   108836              
  Branches     2752     2752              
==========================================
+ Hits        73394    73428      +34     
+ Misses      31480    31446      -34     
  Partials     3962     3962              
Flag Coverage Δ
unittests 67.46% <ø> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 7 files with indirect coverage changes

@AllForNothing
Copy link
Contributor

@SphinxKnight Can you update the background color according to this #19249? The new colors this suggested by our designer.

@SphinxKnight
Copy link
Contributor Author

SphinxKnight commented Oct 24, 2023

@SphinxKnight Can you update the background color according to this #19249? The new colors this suggested by our designer.

Thanks for the pointer: just did in fdaab42

Edit: screenshot with the new values updated in the description. Also included here for simplicity:
image

@OrlinVasilev
Copy link
Member

@SphinxKnight thank you for the PR :))

@OrlinVasilev OrlinVasilev added release-note/enhancement Label to mark PR to be added under release notes as enhancement and removed release-note/update Update or Fix labels Oct 24, 2023
SphinxKnight and others added 3 commits October 25, 2023 11:02
Signed-off-by: julieng <julien.gattelier@gmail.com>
Signed-off-by: julieng <julien.gattelier@gmail.com>
Signed-off-by: julieng <julien.gattelier@gmail.com>
@AllForNothing AllForNothing force-pushed the fix-inaccessible-contrast branch from fdaab42 to 3bf7872 Compare October 25, 2023 03:02
@AllForNothing AllForNothing enabled auto-merge (squash) October 25, 2023 03:04
@AllForNothing AllForNothing merged commit d3907f6 into goharbor:main Oct 25, 2023
@OrlinVasilev
Copy link
Member

@SphinxKnight congrats on merging your first PR here :)

@SphinxKnight
Copy link
Contributor Author

@SphinxKnight congrats on merging your first PR here :)

Thanks :) eager to learn a bit more SASS to factor those tokens when time allows :)

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
area/ui release-note/enhancement Label to mark PR to be added under release notes as enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants