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

feat(legacy-plugin-chart-map-box): show all points in mapbox chart#960

Merged
villebro merged 2 commits intoapache-superset:masterfrom
maloun96:hotfix/mapbox-bbox
Feb 20, 2021
Merged

feat(legacy-plugin-chart-map-box): show all points in mapbox chart#960
villebro merged 2 commits intoapache-superset:masterfrom
maloun96:hotfix/mapbox-bbox

Conversation

@maloun96
Copy link
Contributor

Closes: apache/superset#10843

Increase area with 0.05 to include all points.

BEFORE

mapbox-before

AFTER

mapbox-after

@maloun96 maloun96 requested a review from a team as a code owner February 16, 2021 11:32
@vercel
Copy link

vercel bot commented Feb 16, 2021

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/23qytp6sm
✅ Preview: https://superset-ui-git-fork-maloun96-hotfix-mapbox-bbox.superset.now.sh

@codecov
Copy link

codecov bot commented Feb 16, 2021

Codecov Report

Merging #960 (4d32f1e) into master (62c5e07) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #960      +/-   ##
==========================================
- Coverage   27.70%   27.69%   -0.01%     
==========================================
  Files         401      401              
  Lines        8255     8257       +2     
  Branches     1143     1143              
==========================================
  Hits         2287     2287              
- Misses       5831     5833       +2     
  Partials      137      137              
Impacted Files Coverage Δ
plugins/legacy-plugin-chart-map-box/src/MapBox.jsx 0.00% <0.00%> (ø)

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 62c5e07...4d32f1e. Read the comment docs.

Comment on lines -111 to +118
const bbox = [bounds[0][0], bounds[0][1], bounds[1][0], bounds[1][1]];
// add this variable to widen the visible area
const offset = 0.05;
const bbox = [
bounds[0][0] - offset,
bounds[0][1] - offset,
bounds[1][0] + offset,
bounds[1][1] + offset,
];
Copy link
Contributor

Choose a reason for hiding this comment

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

Applying a constant offset of 0.05 could be much or little depending on where on the zoom level. Would it make sense to make this a percentage of the width for the horizontal and height for the vertical bounds?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, 0.5 percentage of width and height. Can you check, please?

Copy link

@nikolagigic nikolagigic left a comment

Choose a reason for hiding this comment

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

LGTM

@villebro villebro merged commit 6ca76f2 into apache-superset:master Feb 20, 2021
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.

Mapbox display is incorrect for one region not missing always one datapoint

3 participants