Skip to content
This repository has been archived by the owner on Dec 10, 2021. It is now read-only.

feat: improve world map colors #711

Merged
merged 4 commits into from
Aug 4, 2020
Merged

feat: improve world map colors #711

merged 4 commits into from
Aug 4, 2020

Conversation

mistercrunch
Copy link
Contributor

Screen Shot 2020-07-27 at 10 22 33 PM

@mistercrunch mistercrunch requested a review from a team as a code owner July 28, 2020 05:45
@vercel
Copy link

vercel bot commented Jul 28, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/superset/superset-ui/h0syolcfc
✅ Preview: https://superset-ui-git-improveworldmap.superset.vercel.app

@vercel vercel bot temporarily deployed to Preview July 29, 2020 05:35 Inactive
@@ -28,7 +28,9 @@
"access": "public"
},
"dependencies": {
"@superset-ui/color": "^0.14.9",
Copy link
Contributor

Choose a reason for hiding this comment

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

@superset-ui/* packages are normally peerDependencies

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ooops

@ktmud
Copy link
Contributor

ktmud commented Aug 4, 2020

Close and reopen to trigger CI

@ktmud ktmud closed this Aug 4, 2020
@ktmud ktmud reopened this Aug 4, 2020
@codecov
Copy link

codecov bot commented Aug 4, 2020

Codecov Report

Merging #711 into master will increase coverage by 0.11%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #711      +/-   ##
==========================================
+ Coverage   24.19%   24.30%   +0.11%     
==========================================
  Files         339      339              
  Lines        7617     7759     +142     
  Branches      925      961      +36     
==========================================
+ Hits         1843     1886      +43     
- Misses       5701     5789      +88     
- Partials       73       84      +11     
Impacted Files Coverage Δ
...gins/legacy-plugin-chart-world-map/src/WorldMap.js 0.00% <0.00%> (ø)
...egacy-plugin-chart-world-map/src/transformProps.js 0.00% <0.00%> (ø)
packages/superset-ui-style/src/index.ts 100.00% <0.00%> (ø)
...ugins/legacy-plugin-chart-sunburst/src/Sunburst.js 0.00% <0.00%> (ø)
...legacy-plugin-chart-sunburst/src/transformProps.js 0.00% <0.00%> (ø)
plugins/plugin-chart-table/src/TableChart.tsx 59.13% <0.00%> (+3.29%) ⬆️
...ins/plugin-chart-table/src/DataTable/DataTable.tsx 56.75% <0.00%> (+9.53%) ⬆️

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 9c28e93...3445ada. Read the comment docs.

@mistercrunch
Copy link
Contributor Author

@ktmud unclear why merging is blocked despite Will's approval. Looking to merge this.

Side-thought: wondering if we want to proxy some sort of "utils" to make sure core visualizations are aligned on the version of d3 and lodash that they bring in, or using peerdependecies for this as well?

I'm just afraid that many visualization would depend on misaligned pinned versions of commonly used libs and bloat the bundles sizes.

@mistercrunch mistercrunch merged commit 9455f89 into master Aug 4, 2020
@mistercrunch mistercrunch deleted the improve_world_map branch August 4, 2020 23:10
@ktmud
Copy link
Contributor

ktmud commented Aug 4, 2020

@mistercrunch Agreed we should probably have a proxy for the common libraries. d3 could be included, not sure about lodash though (since many lodash functions have easy alternatives in esnext).

@mistercrunch
Copy link
Contributor Author

mistercrunch commented Aug 5, 2020

I hate to introduce yet another package superset-ui-utils or the like, I wonder if there's a way to generate a common package-lock.json from many package.json.

Probably premature optimization... I'd be curious to see how many versions of d3-* we end up with currently.

@ktmud
Copy link
Contributor

ktmud commented Aug 5, 2020

generate a common package-lock.json from many package.json.

That's what monorepo is used for. We do have only one package-lock.json (or rather yarn.lock) in superset-ui, but it's still possible to have many version of d3 packages if not all plugins are updated at the same time. Currently it's relatively easy to update them all since all core plugins live in the same repo---you just search "d3.*": in package.json, sync the versions, and fix any potential backward compatibility issues.

In the async plugin world, we won't be able to control which libraries users bundle into their plugin, but we can provide this proxy to common libraries as part of the plugin API so plugins can reuse the libraries shipped with Superset (ECharts, D3, AntD, etc) and load faster.

@mistercrunch
Copy link
Contributor Author

Nice! I didn't know the monorepo had all this magic happening.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants