[reviewable] Integrate @superset-ui/{core,color,chart} modules #6234

kristw · 2018-10-30T22:26:08Z

Delete files that are already extracted to @superset-ui
Update references to the files above to point to corresponding @superset-ui code.
Update color module usage to utilize the new SequentialScheme function to create linear color scale.
Remove core-js from vendor-major hard-coded bundle. The core-js comes from babel-polyfill and by enforcing it into vendor-major causes some issues.

@williaster @conglei @graceguo-supercat @michellethomas @xtinec @mistercrunch

codecov-io · 2018-10-30T22:43:57Z

Codecov Report

Merging #6234 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #6234   +/-   ##
=======================================
  Coverage   77.33%   77.33%           
=======================================
  Files          67       67           
  Lines        9578     9578           
=======================================
  Hits         7407     7407           
  Misses       2171     2171

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 3ffb48c...5e5218e. Read the comment docs.

xtinec

Thanks for this big sweeping change! 🎉
Looking awesome. Just a couple small nits.

superset/assets/src/explore/components/controls/ColorSchemeControl.jsx

superset/assets/src/explore/controls.jsx

williaster · 2018-11-09T20:06:35Z

superset/assets/src/modules/colors.js

-    return v => hexToRGB(scaler(v));
-  }
-  return scaler;
+  return d3.scale.linear().domain(points).range(colors).clamp(true);


just noting that if we swapped out scaleLinear from d3 v4 here would could get rid of d3 from another file -- def not blocking tho!

There are slight difference between d3 v3 scale.linear and v4 scaleLinear. The deck.gl util that uses this function fails the test after the swap. I want to take a closer look at it in another PR and remove this colorScalerFactory function completely.

I ended up taking care of it. Goodbye colorScalerFactory!

williaster · 2018-11-09T20:08:04Z

superset/assets/src/visualizations/BigNumber/BigNumber.jsx

@@ -2,7 +2,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import shortid from 'shortid';
 import { XYChart, AreaSeries, CrossHair, LinearGradient } from '@data-ui/xy-chart';
-import { BRAND_COLOR } from '../../modules/colors';
+import { BRAND_COLOR } from '@superset-ui/color';


noting that we should probably make this settable in the future since most people will never be able to modify @superset-ui/color

Umm, good point. maybe ConstantRegistry in @superset-ui/core?

williaster · 2018-11-09T20:10:24Z

superset/assets/src/visualizations/Calendar/Calendar.js

-    const colorScale = colorScalerFactory(linearColorScheme, null, null, extents);
+    const colorScale = getSequentialSchemeRegistry()
+      .get(linearColorScheme)
+      .createLinearScale(extents);

    const legend = d3.range(steps)


also noting that you could swap out d3Range here from d3-array and also get rid of d3 v3 in this file 🎉 (also not blocking you just seem to be swapping some so pointing out the low hanging 🍒🍌 :)

williaster · 2018-11-09T20:13:43Z

superset/assets/src/visualizations/deckgl/utils.js

@@ -1,5 +1,6 @@
 import d3 from 'd3';


d3.scale => scaleThreshold would remove d3 v3 in this file (not blocking)

👍 Done, plus some more clean up

williaster

noted a couple easier d3 swaps but, LGTM! thanks for such a big swap! 💪

also curious what the core-js in vendor issues were :)

williaster · 2018-11-09T23:25:49Z

sweet! will merge when 🍏 (green)

williaster · 2018-11-09T23:56:58Z

@kristw there's a conflict now, looks fixable with the web UI possibly 🤔

… kristw-d3

kristw · 2018-11-11T07:32:00Z

conflict resolved

…e#6234) * Add d3 micro packages * Replace d3 imports with specific modules import * Define d3 colors * import specific d3 submodules instead of entire d3 * update function name * move function location and fix small bug * Move primary color to control * remove colorscalefactory usage * remove unused d3 * fix unit test * fix color picker * use @superset-ui/color * update package version * remove files that are extracted * replace all references * fix two files * Revert some changes to split to another PR * remove adaptor * Address Christine's comment * remove d3 v3 from calendar * remove d3.scale.threshold * Get rid of colorScalerFactory and revise hexToRGB * fix color cleaning * fix lint

kristw changed the title ~~[WIP] Import specific d3 submodules instead of entire d3~~ Import specific d3 submodules instead of entire d3 Oct 31, 2018

kristw force-pushed the kristw-d3 branch 3 times, most recently from 0c3d5ba to 078e286 Compare November 6, 2018 21:52

kristw changed the title ~~Import specific d3 submodules instead of entire d3~~ Integrate @superset-ui/core,color/chart modules Nov 7, 2018

kristw closed this Nov 7, 2018

kristw reopened this Nov 7, 2018

kristw closed this Nov 7, 2018

kristw reopened this Nov 7, 2018

kristw changed the title ~~Integrate @superset-ui/core,color/chart modules~~ Integrate @superset-ui/{core,color,chart} modules Nov 7, 2018

kristw closed this Nov 7, 2018

kristw reopened this Nov 7, 2018

kristw changed the title ~~Integrate @superset-ui/{core,color,chart} modules~~ [reviewable] Integrate @superset-ui/{core,color,chart} modules Nov 7, 2018

xtinec reviewed Nov 7, 2018

View reviewed changes

superset/assets/src/explore/components/controls/ColorSchemeControl.jsx Outdated Show resolved Hide resolved

superset/assets/src/explore/controls.jsx Show resolved Hide resolved

kristw force-pushed the kristw-d3 branch 2 times, most recently from 58b28ef to fcdc50e Compare November 8, 2018 06:43

williaster reviewed Nov 9, 2018

View reviewed changes

williaster approved these changes Nov 9, 2018

View reviewed changes

kristw added 8 commits November 9, 2018 13:40

Add d3 micro packages

de61c0d

Replace d3 imports with specific modules import

6aa1ca5

Define d3 colors

f7c0b08

import specific d3 submodules instead of entire d3

e5b2198

update function name

54a9c6b

move function location and fix small bug

2dbbba3

Move primary color to control

78ab2f8

remove colorscalefactory usage

6a11805

kristw added 13 commits November 9, 2018 13:40

remove unused d3

9c64f8a

fix unit test

90aa050

fix color picker

f41f398

use @superset-ui/color

cbac528

update package version

5ef075f

remove files that are extracted

ff2d005

replace all references

c5e7446

fix two files

41875b0

Revert some changes to split to another PR

3197246

remove adaptor

1660968

Address Christine's comment

5a05d28

remove d3 v3 from calendar

178631c

remove d3.scale.threshold

1c28541

kristw force-pushed the kristw-d3 branch from fcdc50e to 1c28541 Compare November 9, 2018 21:57

Get rid of colorScalerFactory and revise hexToRGB

2cbe35e

fix color cleaning

beb9785

kristw added 3 commits November 9, 2018 17:37

Merge branch 'master' into kristw-d3

0ac4f6f

fix lint

e0db1dd

Merge branch 'kristw-d3' of github.com:kristw/incubator-superset into…

5e5218e

… kristw-d3

williaster merged commit a7b52da into apache:master Nov 11, 2018

kristw deleted the kristw-d3 branch March 5, 2019 23:55

ilyakam mentioned this pull request Aug 22, 2019

Superset how to support custom add Color Scheme? #5420

Closed

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.34.0 labels Feb 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[reviewable] Integrate @superset-ui/{core,color,chart} modules #6234

[reviewable] Integrate @superset-ui/{core,color,chart} modules #6234

kristw commented Oct 30, 2018 •

edited

Loading

codecov-io commented Oct 30, 2018 •

edited

Loading

xtinec left a comment

williaster Nov 9, 2018

kristw Nov 9, 2018

kristw Nov 9, 2018

williaster Nov 9, 2018

kristw Nov 9, 2018

williaster Nov 9, 2018

williaster Nov 9, 2018 •

edited

Loading

kristw Nov 9, 2018

williaster Nov 9, 2018

kristw Nov 9, 2018

williaster left a comment

williaster commented Nov 9, 2018

williaster commented Nov 9, 2018

kristw commented Nov 11, 2018 •

edited

Loading

[reviewable] Integrate @superset-ui/{core,color,chart} modules #6234

[reviewable] Integrate @superset-ui/{core,color,chart} modules #6234

Conversation

kristw commented Oct 30, 2018 • edited Loading

codecov-io commented Oct 30, 2018 • edited Loading

Codecov Report

xtinec left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

williaster Nov 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

williaster left a comment

Choose a reason for hiding this comment

williaster commented Nov 9, 2018

williaster commented Nov 9, 2018

kristw commented Nov 11, 2018 • edited Loading

kristw commented Oct 30, 2018 •

edited

Loading

codecov-io commented Oct 30, 2018 •

edited

Loading

williaster Nov 9, 2018 •

edited

Loading

kristw commented Nov 11, 2018 •

edited

Loading