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

Update JsonTree light theme to have better contrast #6448

Merged
merged 4 commits into from Sep 15, 2023

Conversation

turbochef
Copy link
Contributor

@turbochef turbochef commented Sep 15, 2023

What does this PR do?

  • Closes Slice 3.2: Fix Color Contrast #6321
  • Updates the color of the values and count to be #007124
  • Updates color of null and boolean values to be #B4183F
  • Stop nested count color change when expanded

Demo

Kapture 2023-09-14 at 19 21 38

Checklist

@@ -102,7 +102,6 @@ const VariablesTree: React.FunctionComponent<{
theme={popoverTheme}
postprocessValue={sortVarMapKeys}
shouldExpandNodeInitially={expandCurrentVariableLevel(vars, likelyVariable)}
invertTheme
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Inverting the theme calculated the colors so we lost control of them. I just set the background to white in the light theme.

@turbochef turbochef self-assigned this Sep 15, 2023
@codecov
Copy link

codecov bot commented Sep 15, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (6f055b6) 70.11% compared to head (f11b196) 70.12%.

❗ Current head f11b196 differs from pull request most recent head 481e720. Consider uploading reports for the commit 481e720 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6448   +/-   ##
=======================================
  Coverage   70.11%   70.12%           
=======================================
  Files        1169     1169           
  Lines       36512    36512           
  Branches     6877     6877           
=======================================
+ Hits        25602    25605    +3     
+ Misses      10910    10907    -3     
Files Changed Coverage Δ
...ds/schemaFields/widgets/varPopup/VariablesTree.tsx 84.61% <ø> (ø)
...elds/schemaFields/widgets/varPopup/popoverTheme.ts 100.00% <ø> (ø)
src/components/jsonTree/JsonTree.tsx 87.20% <ø> (ø)
src/themes/light.ts 100.00% <ø> (ø)

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@turbochef turbochef requested review from grahamlangford and removed request for mnholtz September 15, 2023 17:15
@@ -40,6 +40,9 @@ export const popoverTheme: Theme = {
alignItems: "center",
flexWrap: "wrap",
},
nestedNodeItemString: {
color: "#007124",
Copy link
Collaborator

Choose a reason for hiding this comment

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

NIT: I would prefer to avoid this magic string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed it to reference the light.ts theme.

@github-actions
Copy link

No loom links were found in the first post. Please add one there if you'd like to it to appear on Slack.

Do not edit this comment manually.

@turbochef turbochef enabled auto-merge (squash) September 15, 2023 18:48
@turbochef turbochef merged commit c99d659 into main Sep 15, 2023
11 checks passed
@turbochef turbochef deleted the feature/6321-fix-color-contrast-in-json-trees branch September 15, 2023 18:53
@grahamlangford grahamlangford added this to the 1.8.0 milestone Oct 4, 2023
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.

Slice 3.2: Fix Color Contrast
2 participants