You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/04/28 12:45:56 UTC

[GitHub] [superset] stephenLYZ opened a new pull request, #19881: feat(world-map): support color by metric or country column

stephenLYZ opened a new pull request, #19881:
URL: https://github.com/apache/superset/pull/19881

   <!---
   Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
   Example:
   fix(dashboard): load charts correctly
   -->
   
   ### SUMMARY
   <!--- Describe the change below, including rationale and design decisions -->
   This PR allows world map to support color presentation in two ways:
   - color by metric (same as before):  use gradient color palettes
   - color by country column (new): use categorical color palettes
   
   Here we add a radio control to achieve it.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   ### before
   <img width="1827" alt="image" src="https://user-images.githubusercontent.com/11830681/165754963-c65f4558-f63f-4d5e-9c69-84d7978aad4e.png">
   
   ### after
   https://user-images.githubusercontent.com/11830681/165754993-67ae98d4-f1b8-420e-b0f1-20380f9e08ca.mov
   
   ### TESTING INSTRUCTIONS
   
   <!--- Required! What steps can be taken to manually verify the changes? -->
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] 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
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] rusackas commented on a diff in pull request #19881: feat(world-map): support color by metric or country column

Posted by GitBox <gi...@apache.org>.
rusackas commented on code in PR #19881:
URL: https://github.com/apache/superset/pull/19881#discussion_r861365614


##########
superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/components/RadioButtonControl.tsx:
##########
@@ -53,8 +53,12 @@ export default function RadioButtonControl({
         '.btn:focus': {
           outline: 'none',
         },
+        '.control-label': {
+          color: theme.colors.grayscale.base,
+          marginBottom: theme.gridUnit,
+        },
         '.control-label + .btn-group': {
-          marginTop: 1,
+          marginTop: theme.gridUnit * 0.25,

Review Comment:
   I think it's fine to avoid fractional gridUnits, and just use 1px in this case.... if it's even needed at all. 
   ```suggestion
             marginTop:  1px,
   ```
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] stephenLYZ merged pull request #19881: feat(world-map): support color by metric or country column

Posted by GitBox <gi...@apache.org>.
stephenLYZ merged PR #19881:
URL: https://github.com/apache/superset/pull/19881


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] rusackas commented on a diff in pull request #19881: feat(world-map): support color by metric or country column

Posted by GitBox <gi...@apache.org>.
rusackas commented on code in PR #19881:
URL: https://github.com/apache/superset/pull/19881#discussion_r861368317


##########
superset-frontend/plugins/legacy-plugin-chart-world-map/src/controlPanel.ts:
##########
@@ -106,7 +107,25 @@ const config: ControlPanelConfig = {
           },
         ],
         ['color_picker'],
+        [
+          {
+            name: 'color_by',
+            config: {
+              type: 'RadioButtonControl',
+              label: t('Color by'),
+              default: ColorBy.metric,
+              options: [
+                [ColorBy.metric, t('metric')],
+                [ColorBy.country, t('country')],
+              ],
+              description: t(
+                'Whether to define a color by metric or country column',

Review Comment:
   ```suggestion
                   'Choose whether a country should be shaded by the metric, or assigned a color based on a categorical color palette',
   ```
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] rusackas commented on pull request #19881: feat(world-map): support color by metric or country column

Posted by GitBox <gi...@apache.org>.
rusackas commented on PR #19881:
URL: https://github.com/apache/superset/pull/19881#issuecomment-1112726130

   Two random thoughts... and these can be separate tickets/PRs if we want to do either of them:
   
   1) This is the weird one: If you select to color the map by country, and deselect select "show bubbles," then you don't really need the Metric input (which is currently a required input). I thought for a second about making that input optional in that case... but in this configuration, the map is essentially useless, and doesn't tell you anything... so... whatever - fine as is. ¯\\\_(ツ)_/¯ 
   
   2a) You only actually need four colors to display a map. As a default, it might be nice to take advantage of that, if your mapping library happens to support the [four color theorem](https://en.wikipedia.org/wiki/Four_color_theorem) as a feature.
   
   2b) If we do take advantage of the four color theorem, it may get ugly with color consistency... so... I don't quite know what to do there.
   
   3) Also, with the color consistency thing... since a map might gobble up all the colors of a palette instantly, is there a good way to de-prioritize this chart (or others like it) it in the color assignment process?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] rusackas commented on a diff in pull request #19881: feat(world-map): support color by metric or country column

Posted by GitBox <gi...@apache.org>.
rusackas commented on code in PR #19881:
URL: https://github.com/apache/superset/pull/19881#discussion_r861367868


##########
superset-frontend/plugins/legacy-plugin-chart-world-map/src/controlPanel.ts:
##########
@@ -106,7 +107,25 @@ const config: ControlPanelConfig = {
           },
         ],
         ['color_picker'],
+        [
+          {
+            name: 'color_by',
+            config: {
+              type: 'RadioButtonControl',
+              label: t('Color by'),
+              default: ColorBy.metric,
+              options: [
+                [ColorBy.metric, t('metric')],
+                [ColorBy.country, t('country')],

Review Comment:
   ```suggestion
                   [ColorBy.metric, t('Metric')],
                   [ColorBy.country, t('Country')],
   ```
   Nitpicking. This really doesn't matter since the CSS capitalizes it. But if we ever move away from all-caps in the design, then this will be nicely capitalized that day ;)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] codecov[bot] commented on pull request #19881: feat(world-map): support color by metric or country column

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #19881:
URL: https://github.com/apache/superset/pull/19881#issuecomment-1112320083

   # [Codecov](https://codecov.io/gh/apache/superset/pull/19881?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#19881](https://codecov.io/gh/apache/superset/pull/19881?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (497d035) into [master](https://codecov.io/gh/apache/superset/commit/481ccfe0a6e32caa58b760ca2d0bfb247a558f1f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (481ccfe) will **decrease** coverage by `0.04%`.
   > The diff coverage is `55.31%`.
   
   > :exclamation: Current head 497d035 differs from pull request most recent head b608026. Consider uploading reports for the commit b608026 to get more accurate results
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #19881      +/-   ##
   ==========================================
   - Coverage   66.51%   66.46%   -0.05%     
   ==========================================
     Files        1714     1714              
     Lines       65027    65036       +9     
     Branches     6711     6716       +5     
   ==========================================
   - Hits        43250    43224      -26     
   - Misses      20070    20105      +35     
     Partials     1707     1707              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `51.24% <55.31%> (+0.01%)` | :arrow_up: |
   | presto | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/19881?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [.../shared-controls/components/RadioButtonControl.tsx](https://codecov.io/gh/apache/superset/pull/19881/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9jb21wb25lbnRzL1JhZGlvQnV0dG9uQ29udHJvbC50c3g=) | `0.00% <ø> (ø)` | |
   | [...gins/legacy-plugin-chart-world-map/src/WorldMap.js](https://codecov.io/gh/apache/superset/pull/19881/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcGx1Z2luLWNoYXJ0LXdvcmxkLW1hcC9zcmMvV29ybGRNYXAuanM=) | `0.00% <0.00%> (ø)` | |
   | [.../legacy-plugin-chart-world-map/src/controlPanel.ts](https://codecov.io/gh/apache/superset/pull/19881/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcGx1Z2luLWNoYXJ0LXdvcmxkLW1hcC9zcmMvY29udHJvbFBhbmVsLnRz) | `33.33% <0.00%> (-66.67%)` | :arrow_down: |
   | [...egacy-plugin-chart-world-map/src/transformProps.js](https://codecov.io/gh/apache/superset/pull/19881/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcGx1Z2luLWNoYXJ0LXdvcmxkLW1hcC9zcmMvdHJhbnNmb3JtUHJvcHMuanM=) | `0.00% <0.00%> (ø)` | |
   | [...plugins/plugin-chart-handlebars/src/Handlebars.tsx](https://codecov.io/gh/apache/superset/pull/19881/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtaGFuZGxlYmFycy9zcmMvSGFuZGxlYmFycy50c3g=) | `0.00% <0.00%> (ø)` | |
   | [...ars/src/components/Handlebars/HandlebarsViewer.tsx](https://codecov.io/gh/apache/superset/pull/19881/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtaGFuZGxlYmFycy9zcmMvY29tcG9uZW50cy9IYW5kbGViYXJzL0hhbmRsZWJhcnNWaWV3ZXIudHN4) | `0.00% <0.00%> (ø)` | |
   | [...lugin-chart-handlebars/src/plugin/controlPanel.tsx](https://codecov.io/gh/apache/superset/pull/19881/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtaGFuZGxlYmFycy9zcmMvcGx1Z2luL2NvbnRyb2xQYW5lbC50c3g=) | `100.00% <ø> (ø)` | |
   | [...n-chart-handlebars/src/plugin/controls/groupBy.tsx](https://codecov.io/gh/apache/superset/pull/19881/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtaGFuZGxlYmFycy9zcmMvcGx1Z2luL2NvbnRyb2xzL2dyb3VwQnkudHN4) | `16.66% <ø> (ø)` | |
   | [...ndlebars/src/plugin/controls/handlebarTemplate.tsx](https://codecov.io/gh/apache/superset/pull/19881/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtaGFuZGxlYmFycy9zcmMvcGx1Z2luL2NvbnRyb2xzL2hhbmRsZWJhclRlbXBsYXRlLnRzeA==) | `28.57% <0.00%> (+6.34%)` | :arrow_up: |
   | [...hart-handlebars/src/plugin/controls/includeTime.ts](https://codecov.io/gh/apache/superset/pull/19881/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtaGFuZGxlYmFycy9zcmMvcGx1Z2luL2NvbnRyb2xzL2luY2x1ZGVUaW1lLnRz) | `100.00% <ø> (ø)` | |
   | ... and [23 more](https://codecov.io/gh/apache/superset/pull/19881/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/19881?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/19881?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [481ccfe...b608026](https://codecov.io/gh/apache/superset/pull/19881?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] rusackas commented on a diff in pull request #19881: feat(world-map): support color by metric or country column

Posted by GitBox <gi...@apache.org>.
rusackas commented on code in PR #19881:
URL: https://github.com/apache/superset/pull/19881#discussion_r861369225


##########
superset-frontend/plugins/legacy-plugin-chart-world-map/src/WorldMap.js:
##########
@@ -69,15 +74,27 @@ function WorldMap(element, props) {
     .domain([extRadius[0], extRadius[1]])
     .range([1, maxBubbleSize]);
 
-  const colorScale = getSequentialSchemeRegistry()
-    .get(linearColorScheme)
-    .createLinearScale(d3Extent(filteredData, d => d.m1));
+  let processedData;
+  let colorScale;
+  if (colorBy === ColorBy.country) {
+    colorScale = CategoricalColorNamespace.getScale(colorScheme);
 
-  const processedData = filteredData.map(d => ({
-    ...d,
-    radius: radiusScale(Math.sqrt(d.m2)),
-    fillColor: colorScale(d.m1),
-  }));
+    processedData = filteredData.map(d => ({
+      ...d,
+      radius: radiusScale(Math.sqrt(d.m2)),
+      fillColor: colorScale(d.name, sliceId),

Review Comment:
   We could also base this on the metric, rather than the name, so that the country with the highest value gets the first color in the palette.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] rusackas commented on a diff in pull request #19881: feat(world-map): support color by metric or country column

Posted by GitBox <gi...@apache.org>.
rusackas commented on code in PR #19881:
URL: https://github.com/apache/superset/pull/19881#discussion_r861368317


##########
superset-frontend/plugins/legacy-plugin-chart-world-map/src/controlPanel.ts:
##########
@@ -106,7 +107,25 @@ const config: ControlPanelConfig = {
           },
         ],
         ['color_picker'],
+        [
+          {
+            name: 'color_by',
+            config: {
+              type: 'RadioButtonControl',
+              label: t('Color by'),
+              default: ColorBy.metric,
+              options: [
+                [ColorBy.metric, t('metric')],
+                [ColorBy.country, t('country')],
+              ],
+              description: t(
+                'Whether to define a color by metric or country column',

Review Comment:
   ```suggestion
                   'Choose whether a country should be shaded by the metric, or assigned a color based on a categorical color palette',
   ```
   I'm not even sure if this is any clearer... but I thought I'd try.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] stephenLYZ commented on a diff in pull request #19881: feat(world-map): support color by metric or country column

Posted by GitBox <gi...@apache.org>.
stephenLYZ commented on code in PR #19881:
URL: https://github.com/apache/superset/pull/19881#discussion_r861665515


##########
superset-frontend/plugins/legacy-plugin-chart-world-map/src/WorldMap.js:
##########
@@ -69,15 +74,27 @@ function WorldMap(element, props) {
     .domain([extRadius[0], extRadius[1]])
     .range([1, maxBubbleSize]);
 
-  const colorScale = getSequentialSchemeRegistry()
-    .get(linearColorScheme)
-    .createLinearScale(d3Extent(filteredData, d => d.m1));
+  let processedData;
+  let colorScale;
+  if (colorBy === ColorBy.country) {
+    colorScale = CategoricalColorNamespace.getScale(colorScheme);
 
-  const processedData = filteredData.map(d => ({
-    ...d,
-    radius: radiusScale(Math.sqrt(d.m2)),
-    fillColor: colorScale(d.m1),
-  }));
+    processedData = filteredData.map(d => ({
+      ...d,
+      radius: radiusScale(Math.sqrt(d.m2)),
+      fillColor: colorScale(d.name, sliceId),

Review Comment:
   Yeah. But actually we may need to use country name as the color label instead of metric value when we use categorical color, and the metric values here are not in order. If we want to make the country with the highest value get the first color in the palette, we can sort the `filteredData` first.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


[GitHub] [superset] stephenLYZ commented on pull request #19881: feat(world-map): support color by metric or country column

Posted by GitBox <gi...@apache.org>.
stephenLYZ commented on PR #19881:
URL: https://github.com/apache/superset/pull/19881#issuecomment-1113241645

   > Two random thoughts... and these can be separate tickets/PRs if we want to do either of them:
   > 
   > 1. This is the weird one: If you select to color the map by country, and deselect select "show bubbles," then you don't really need the Metric input (which is currently a required input). I thought for a second about making that input optional in that case... but in this configuration, the map is essentially useless, and doesn't tell you anything... so... whatever - fine as is. ¯\_(ツ)_/¯
   > 
   > 2a) You only actually need four colors to display a map. As a default, it might be nice to take advantage of that, if your mapping library happens to support the [four color theorem](https://en.wikipedia.org/wiki/Four_color_theorem) as a feature.
   > 
   > 2b) If we do take advantage of the four color theorem, it may get ugly with color consistency... so... I don't quite know what to do there.
   > 
   > 3. Also, with the color consistency thing... since a map might gobble up all the colors of a palette instantly, is there a good way to de-prioritize this chart (or others like it) it in the color assignment process?
   
   Thanks for these inputs! 
   1.  Yeah. The only scenario I can think of for categorical color is that the user wants to have a world map in a dashboard, and the colors of different countries correspond to the series in different charts, and that's it, so let's just focus on the metric 😂.
   2. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org