fix(heatmap): add detail descriptions for heatmap 'normalize across' #20566

stephenLYZ · 2022-06-30T12:33:54Z

SUMMARY

Add detail desciptions for heatmap 'normalize across'

y -> percentage of the highest value in that row
x -> percentage of the highest value in that column
heatmap -> percentage of the highest value in the whole heatmap

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

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

codecov · 2022-06-30T13:20:46Z

Codecov Report

Merging #20566 (5970dd5) into master (9c7bcfc) will not change coverage.
The diff coverage is n/a.

❗ Current head 5970dd5 differs from pull request most recent head 3e77dff. Consider uploading reports for the commit 3e77dff to get more accurate results

@@           Coverage Diff           @@
##           master   #20566   +/-   ##
=======================================
  Coverage   66.30%   66.30%           
=======================================
  Files        1756     1756           
  Lines       66735    66735           
  Branches     7049     7049           
=======================================
  Hits        44251    44251           
  Misses      20688    20688           
  Partials     1796     1796

Flag	Coverage Δ
javascript	`51.93% <ø> (ø)`

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

Impacted Files	Coverage Δ
...s/legacy-plugin-chart-heatmap/src/controlPanel.tsx	`57.14% <ø> (ø)`

Antonio-RiveroMartnez · 2022-06-30T16:56:41Z

superset-frontend/plugins/legacy-plugin-chart-heatmap/src/controlPanel.tsx

@@ -163,17 +164,34 @@ const config: ControlPanelConfig = {
            name: 'normalize_across',
            config: {
              type: 'SelectControl',
-              label: t('Normalize Across'),
+              label: t('COLOR BASED ON'),


Any reason why having this with all upper case?

yousoph · 2022-07-01T18:57:18Z

/testenv up

superset-frontend/plugins/legacy-plugin-chart-heatmap/src/controlPanel.tsx

github-actions · 2022-07-01T18:59:11Z

@yousoph Ephemeral environment spinning up at http://54.187.156.48:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

yousoph · 2022-07-01T19:22:43Z

the dropdown options don't look renamed in the ephemeral env

stephenLYZ · 2022-07-04T13:00:20Z

@yousoph Yeah, the point I'm currently struggling with is that using like "percentage of the highest value in that row" as an option label is too long, can we simplify it? Or just re-label to "row" and add some tooltip.

superset-frontend/plugins/legacy-plugin-chart-heatmap/src/controlPanel.tsx

rusackas · 2022-07-06T04:37:10Z

superset-frontend/plugins/legacy-plugin-chart-heatmap/src/controlPanel.tsx

@@ -163,17 +164,34 @@ const config: ControlPanelConfig = {
            name: 'normalize_across',
            config: {
              type: 'SelectControl',
-              label: t('Normalize Across'),
+              label: t('Color based on'),


I think I might be OK with "Normalize Across" now that we know it's actually doing normalization, mathematically

rusackas

LGTM. This just needs a rebase to make codecov happy. @yousoph let me know if you agree on the assessment - I think this is clearer.

…ome label

…rolPanel.tsx Co-authored-by: Evan Rusackas <[email protected]>

github-actions · 2022-07-20T15:58:19Z

Ephemeral environment shutdown and build artifacts deleted.

pull-request-size bot added the size/S label Jun 30, 2022

Antonio-RiveroMartnez reviewed Jun 30, 2022

View reviewed changes

rusackas reviewed Jul 1, 2022

View reviewed changes