feat(ConditionalFormatting): allow inverse color scaling and add color picker #20353

stevetracvc · 2022-06-11T19:46:59Z

SUMMARY

Currently, for the conditional color scaling, alpha is directly proportional to the value. This PR adds a checkbox to the Conditional Formatting modal that allows the designer to invert this scale, ie, as the value increases, the alpha decreases

Additionally, this fixes an issue I noticed with #20016 that broke the conditional formatting feature (need to convert hex string to 'rgb(R,G,B)' string).

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Broken color formatting:

Regular conditional formatting:

Inverted for Rank

TESTING INSTRUCTIONS

Create a table with numeric values
Add a conditional formatting under the Customize tab
Toggle the checkbox to see the effect

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

stevetracvc · 2022-06-11T19:55:19Z

The hexToRGBString function might be useful elsewhere, so please let me know if there's a good place to move it.

codecov · 2022-06-11T20:43:21Z

Codecov Report

Merging #20353 (ca1296a) into master (ab9f72f) will increase coverage by 0.00%.
The diff coverage is 77.27%.

@@           Coverage Diff           @@
##           master   #20353   +/-   ##
=======================================
  Coverage   66.71%   66.71%           
=======================================
  Files        1739     1739           
  Lines       65149    65190   +41     
  Branches     6899     6919   +20     
=======================================
+ Hits        43462    43493   +31     
- Misses      19934    19939    +5     
- Partials     1753     1758    +5

Flag	Coverage Δ
javascript	`51.79% <77.27%> (+0.02%)`	⬆️

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

Impacted Files	Coverage Δ
...d/packages/superset-ui-chart-controls/src/types.ts	`100.00% <ø> (ø)`
...explore/components/controls/ColorPickerControl.jsx	`88.23% <ø> (ø)`
...onalFormattingControl/FormattingPopoverContent.tsx	`46.34% <0.00%> (-5.01%)`	⬇️
...nts/controls/ConditionalFormattingControl/types.ts	`100.00% <ø> (ø)`
superset-frontend/src/utils/colorUtils.ts	`40.90% <66.66%> (+4.06%)`	⬆️
...-ui-chart-controls/src/utils/getColorFormatters.ts	`98.78% <85.71%> (-1.22%)`	⬇️
...ntend/packages/superset-ui-core/src/color/utils.ts	`94.02% <86.66%> (-5.98%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab9f72f...ca1296a. Read the comment docs.

michael-s-molina

Thanks for the PR @stevetracvc and for adding tests! I left some comments.

superset-frontend/packages/superset-ui-chart-controls/test/utils/getColorFormatters.test.ts

...nd/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.tsx

...c/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.test.tsx

…default in color picker

...nd/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.tsx

kgabryje · 2022-06-14T13:50:52Z

Love the change with color picker!
A few comments:

Maybe it would look better if we left the checkbox without any validation errors? If we changed copy in the tooltip to sth like "Ignored when '=' operator is selected", maybe that would be sufficient and we could simply ignore the state of the checkbox?
Is it possible to make the color picker popover a bit wider, so that the input fields for rgb values are wide enough to hold 3 digits? See video

Screen.Recording.2022-06-14.at.15.44.03.mov

stevetracvc · 2022-06-14T14:35:45Z

Love the change with color picker! A few comments:

Maybe it would look better if we left the checkbox without any validation errors? If we changed copy in the tooltip to sth like "Ignored when '=' operator is selected", maybe that would be sufficient and we could simply ignore the state of the checkbox?

I'll defer to you.

Is it possible to make the color picker popover a bit wider, so that the input fields for rgb values are wide enough to hold 3 digits? See video

I added an extra prop to make it easier to set the color picker width. Here's 300 and 250 (default is 200), I think 250 is probably good.

superset-frontend/packages/superset-ui-chart-controls/src/types.ts

superset-frontend/packages/superset-ui-chart-controls/src/utils/getColorFormatters.ts

stevetracvc · 2022-06-14T14:41:17Z

superset-frontend/src/explore/components/controls/ColorPickerControl.jsx

@@ -26,10 +26,12 @@ import ControlHeader from '../ControlHeader';
 const propTypes = {
  onChange: PropTypes.func,
  value: PropTypes.object,
+  sketchPickerWidth: PropTypes.number,


not sure if you'd rather just hard code in a bigger width, rather than having it as a prop

there are a lot of issues associated with moving away from color strings like `rgb(255, 255, 255)` in favor of `#FFFFFF` (and similar with alpha values). This fixes a lot of the issues presented, and also provides some extra functions to help with the transition. Unfortunately there are still too many other components that are using these rgb strings to convert the color picker right now.

stevetracvc · 2022-06-18T23:18:15Z

switching from colors stored as 'rgb(r, g, b)' to hex strings is going to affect a lot of different parts of the code

rusackas · 2024-02-02T17:13:00Z

Hey @stevetracvc @kgabryje @michael-s-molina - it looks like this effort fell by the wayside somehow. Is there any interest in rekindling/rebasing this effort to get it across the finish line?

stevetracvc · 2024-02-07T15:37:12Z

@rusackas I'd love to, not sure how much work it'll be. I'm still running v2.0.x something, and I haven't had a chance to play with any of the newer updates. I don't think I'll be able to look at it till March though. I'll have to see what you all did with the colors themselves, since I know there was a weird transition period between rgb objects and hex strings.

rusackas · 2024-02-07T16:04:35Z

Cool... I'll just set this to Draft mode for now then, and hit us up here or on Slack if you want to catch up on things when the time comes.

codecov-commenter · 2024-04-08T17:00:01Z

Codecov Report

Attention: Patch coverage is 77.27273% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 66.71%. Comparing base (ab9f72f) to head (ca1296a).
Report is 3248 commits behind head on master.

Files	Patch %	Lines
...ntend/packages/superset-ui-core/src/color/utils.ts	86.66%	1 Missing and 3 partials ⚠️
...onalFormattingControl/FormattingPopoverContent.tsx	0.00%	4 Missing ⚠️
...-ui-chart-controls/src/utils/getColorFormatters.ts	85.71%	0 Missing and 1 partial ⚠️
superset-frontend/src/utils/colorUtils.ts	66.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #20353   +/-   ##
=======================================
  Coverage   66.71%   66.71%           
=======================================
  Files        1739     1739           
  Lines       65149    65190   +41     
  Branches     6899     6919   +20     
=======================================
+ Hits        43462    43493   +31     
- Misses      19934    19939    +5     
- Partials     1753     1758    +5

Flag	Coverage Δ
javascript	`51.79% <77.27%> (+0.02%)`	⬆️

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

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

rusackas · 2024-06-06T19:47:14Z

@stevetracvc Since this still seems to be accumulating conflicts and whatnot, I'll go ahead and close it. Please feel free to reopen it if you ever want to rekindle this effort. Thanks!

stevetracvc added 2 commits June 11, 2022 13:31

fix: converted theme colors to 'rgb(r,g,b)' string

a368106

added option to invert the color scale

e8d9f32

pull-request-size bot added the size/L label Jun 11, 2022

fix: lint error

3273fc5

stevetracvc added 4 commits June 12, 2022 07:24

fix: use minOpacity on inverse

9e0e498

new tests for getColorFunction

1dec457

inverse doesn't work with equals, added validator

9457c02

added test for hexToRGBString

321e073

michael-s-molina reviewed Jun 13, 2022

View reviewed changes

michael-s-molina requested a review from kgabryje June 13, 2022 11:55

stevetracvc added 4 commits June 13, 2022 12:57

switched to hexToRgb util

74b3a4e

switched to color picker

af5171a

fixed tests and opacity rounding issue

3b62d53

added rgbStringToRgbaObject function to utils so themed green can be …

65034ca

…default in color picker

pull-request-size bot added size/XL and removed size/L labels Jun 14, 2022

kgabryje reviewed Jun 14, 2022

View reviewed changes

...nd/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.tsx Outdated Show resolved Hide resolved

stevetracvc added 3 commits June 14, 2022 08:25

added sketchPickerWidth prop to make it easier to set popup width

1be3b45

fix error on extra escaping

54fae32

removed inverse validator, ignored on equals operator

b78c576

pull-request-size bot added size/L and removed size/XL labels Jun 14, 2022

stevetracvc commented Jun 14, 2022

View reviewed changes

superset-frontend/packages/superset-ui-chart-controls/src/types.ts Outdated Show resolved Hide resolved

stevetracvc commented Jun 14, 2022

View reviewed changes

superset-frontend/packages/superset-ui-chart-controls/src/utils/getColorFormatters.ts Outdated Show resolved Hide resolved

stevetracvc commented Jun 14, 2022

View reviewed changes

stevetracvc mentioned this pull request Jun 18, 2022

fix(plugin-chart-pivot-table): color weight of Conditional formatting metrics not work #20396

Merged

9 tasks

pull-request-size bot added size/XL and removed size/L labels Jun 18, 2022

stevetracvc added 2 commits June 18, 2022 16:38

copied rgbToHex to avoid dependency issue

202160c

fixed lint errors

7cfdea5

removed optional text on modal

ca1296a

stevetracvc changed the title ~~feat(ConditionalFormatting): allow inverse color scaling~~ feat(ConditionalFormatting): allow inverse color scaling and add color picker Aug 10, 2022

rusackas marked this pull request as draft February 7, 2024 16:04

john-bodley added the review:draft label Apr 8, 2024

rusackas closed this Jun 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ConditionalFormatting): allow inverse color scaling and add color picker #20353

feat(ConditionalFormatting): allow inverse color scaling and add color picker #20353

stevetracvc commented Jun 11, 2022 •

edited

stevetracvc commented Jun 11, 2022

codecov bot commented Jun 11, 2022 •

edited

michael-s-molina left a comment

kgabryje commented Jun 14, 2022

stevetracvc commented Jun 14, 2022

stevetracvc Jun 14, 2022

stevetracvc commented Jun 18, 2022

rusackas commented Feb 2, 2024

stevetracvc commented Feb 7, 2024

rusackas commented Feb 7, 2024

codecov-commenter commented Apr 8, 2024

rusackas commented Jun 6, 2024

feat(ConditionalFormatting): allow inverse color scaling and add color picker #20353

feat(ConditionalFormatting): allow inverse color scaling and add color picker #20353

Conversation

stevetracvc commented Jun 11, 2022 • edited

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

stevetracvc commented Jun 11, 2022

codecov bot commented Jun 11, 2022 • edited

Codecov Report

michael-s-molina left a comment

Choose a reason for hiding this comment

kgabryje commented Jun 14, 2022

stevetracvc commented Jun 14, 2022

stevetracvc Jun 14, 2022

Choose a reason for hiding this comment

stevetracvc commented Jun 18, 2022

rusackas commented Feb 2, 2024

stevetracvc commented Feb 7, 2024

rusackas commented Feb 7, 2024

codecov-commenter commented Apr 8, 2024

Codecov Report

rusackas commented Jun 6, 2024

stevetracvc commented Jun 11, 2022 •

edited

codecov bot commented Jun 11, 2022 •

edited