superset icon indicating copy to clipboard operation
superset copied to clipboard

feat(legacy-plugin-chart-map-box): Add fixed initial viewport settings option

Open jasoncodes42 opened this issue 1 year ago • 4 comments

SUMMARY

It's currently not possible to fix the initial map box view. The current settings are not used when setting up the initial view. The initial is auto calculated from bounds, and in some cases doesn't seem to place all the points on the screen. This PR adds a drop down to enable a "fixed" initial view option which will use the user entered settings as the initial view instead of the calculation from bounds.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

image

TESTING INSTRUCTIONS

By default, the MapBox chart will still auto calculate the initial view from the bounds. To test the fixed option, set the Initial Viewport settings dropdown to fixed and update the default lat/long/zoom

ADDITIONAL INFORMATION

  • [x ] Has associated issue: Fixes https://github.com/apache/superset/issues/17269

jasoncodes42 avatar Jan 22 '24 10:01 jasoncodes42

The last commit fixes an issue where the plots do not load until the map is dragged. The first time the component loads after login works fine, but after that, the plots do not load until dragged. Putting the clusters in state fixes the issue. Also converted it to a functional component.

jasoncodes42 avatar Jan 22 '24 13:01 jasoncodes42

Codecov Report

Attention: Patch coverage is 0% with 28 lines in your changes are missing coverage. Please review.

Project coverage is 69.62%. Comparing base (6443001) to head (9433499). Report is 417 commits behind head on master.

Files Patch % Lines
...plugins/legacy-plugin-chart-map-box/src/MapBox.jsx 0.00% 28 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #26736      +/-   ##
==========================================
+ Coverage   69.07%   69.62%   +0.54%     
==========================================
  Files        1930     1909      -21     
  Lines       75279    74770     -509     
  Branches     8429     8350      -79     
==========================================
+ Hits        51999    52058      +59     
+ Misses      21133    20661     -472     
+ Partials     2147     2051      -96     
Flag Coverage Δ
javascript 57.37% <0.00%> (+0.89%) :arrow_up:

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jan 24 '24 18:01 codecov[bot]

@justinpark Thanks for the review! I updated the PR to use useMemo as per your suggestion, but unfortunately it broke the fix for when the plots do not load until the map is dragged. The latest commit I pushed shows the working code where i added // ++ comments for the added lines and // -- for the removed lines so you can see what the code looked like with useMemo. Any ideas on how to fix this bug with useMemo? Essentially the issue is that when the map loaded for the first time after logging in, it works ok, but then if you go back to that dashboard, or refresh the page, the plots do not load until you drag the map. The useEffect+useState fixes this, but the useMemo how I implemented it does not. Did I miss something?

jasoncodes42 avatar Feb 04 '24 05:02 jasoncodes42

@jseparovic I think this just needs the [precommit hooks] run - one of the linters (prettier, specifically) is not happy with the changelog file.

rusackas avatar May 13 '24 20:05 rusackas