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

DYN-4734 Contrast ratio between a group background and group text fix #13682

Merged
merged 1 commit into from
Jan 20, 2023

Conversation

dnenov
Copy link
Collaborator

@dnenov dnenov commented Jan 17, 2023

Purpose

This PR addresses DYN-4734 'As a Dynamo user, I want clear contrast between group background and group text'. The requirements are:

  • Identify the contrast ratio between group color and group text (title, description)
  • Make sure the contrast is larger than 4.5, otherwise, switch the color of the group text
    • group text color between #F5F5F5(white) and #3C3C3C(dark charcoal)

workflow_test

Contrast ratio test

contrast_ratio

image

Using this tool to measure the contrast between the #3C3C3C text color and chosen background color.

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User-facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated

Release Notes

  • now measures the current group text-to-group background contrast ratio and adjusts text color according to the group background color, so good contrast is maintained
  • introduced a new color (#F5F5F5 white) in the DynamoColorsAndBrushes
  • will not change text on rename, as the background is consistently white during renaming

Reviewers

@reddyashish

FYIs

@Amoursol
@Jingyi-Wen

- now measures the current group text to group background contrast ratio and adjusts text color according to the group background color, so good contrast is maintained
@Amoursol
Copy link
Contributor

@dnenov looks awesome! Could we do one quick test with a 50% contrast color around the middle (As in close to when it switches between white and black) to check legibility?

@dnenov
Copy link
Collaborator Author

dnenov commented Jan 18, 2023

@dnenov looks awesome! Could we do one quick test with a 50% contrast color around the middle (As in close to when it switches between white and black) to check legibility?

Sure thing, I uploaded a test case in the body of the PR, I hope that's helpful.

@reddyashish reddyashish marked this pull request as ready for review January 19, 2023 15:12
@reddyashish reddyashish merged commit 08a2a32 into DynamoDS:master Jan 20, 2023
var lightColor = (System.Windows.Media.Color)SharedDictionaryManager.DynamoColorsAndBrushesDictionary["WhiteColor"];
var darkColor = (System.Windows.Media.Color)SharedDictionaryManager.DynamoColorsAndBrushesDictionary["DarkerGrey"];

var backgroundColor = (System.Windows.Media.Color)value;
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if the foreground color ( value ) can ever be null, but it won't hurt to do a check

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

Successfully merging this pull request may close these issues.

4 participants