Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SIP-5] Refactor MapBox #5783

Merged
merged 8 commits into from
Sep 5, 2018
Merged

[SIP-5] Refactor MapBox #5783

merged 8 commits into from
Sep 5, 2018

Conversation

kristw
Copy link
Contributor

@kristw kristw commented Aug 30, 2018

  • Break MapBox into smaller files
  • Enable renderTrigger for viewport changing
  • Pass explicit props rather than pass all that exists in payload.data
  • Detach setControlValue from the vis component
  • Replace React.createElement with regular jsx
  • Remove usage of refs which is deprecated.

@williaster @conglei @hughhhh @betodealmeida @mistercrunch

@codecov-io
Copy link

codecov-io commented Aug 31, 2018

Codecov Report

Merging #5783 into master will decrease coverage by 0.04%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5783      +/-   ##
==========================================
- Coverage    63.8%   63.75%   -0.05%     
==========================================
  Files         364      365       +1     
  Lines       23109    23127      +18     
  Branches     2588     2585       -3     
==========================================
  Hits        14745    14745              
- Misses       8349     8367      +18     
  Partials       15       15
Impacted Files Coverage Δ
superset/assets/src/explore/controls.jsx 46.26% <ø> (ø) ⬆️
...perset/assets/src/visualizations/MapBox/MapBox.jsx 0% <0%> (ø)
...c/visualizations/MapBox/ScatterPlotGlowOverlay.jsx 0% <0%> (ø)

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 2811498...71065f9. Read the comment docs.

Copy link
Contributor

@williaster williaster left a comment

Choose a reason for hiding this comment

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

LGTM overall! left a couple comments.

@@ -0,0 +1,16 @@
.mapbox div.widget .slice_container {
Copy link
Contributor

Choose a reason for hiding this comment

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

could you tell if the .widget styles still apply without dashboard v1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could not find any element on both explore and dashboard so I delete these css.

@@ -1802,6 +1802,7 @@ export const controls = {
viewport_zoom: {
type: 'TextControl',
label: t('Zoom'),
renderTrigger: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

superset/assets/src/visualizations/MapBox/MapBox.jsx Outdated Show resolved Hide resolved
@williaster williaster merged commit bebbdb8 into apache:master Sep 5, 2018
kristw added a commit to kristw/incubator-superset that referenced this pull request Sep 5, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
@kristw kristw deleted the kristw-mapbox branch September 5, 2018 21:06
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Sep 21, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Sep 21, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Sep 21, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Sep 21, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Sep 21, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Sep 21, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Sep 21, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Sep 21, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Oct 11, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Oct 11, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Oct 11, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Oct 12, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Oct 12, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Oct 12, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
youngyjd pushed a commit to lyft/incubator-superset that referenced this pull request Oct 17, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
youngyjd pushed a commit to lyft/incubator-superset that referenced this pull request Oct 17, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
youngyjd pushed a commit to lyft/incubator-superset that referenced this pull request Oct 17, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
youngyjd pushed a commit to lyft/incubator-superset that referenced this pull request Oct 17, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
youngyjd pushed a commit to lyft/incubator-superset that referenced this pull request Oct 17, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Oct 29, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Oct 30, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Oct 30, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Oct 30, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Oct 30, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Oct 30, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
youngyjd pushed a commit to lyft/incubator-superset that referenced this pull request Nov 2, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
youngyjd pushed a commit to lyft/incubator-superset that referenced this pull request Nov 2, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css

(cherry picked from commit bebbdb8)
wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018
* Break MapBox into smaller pieces

* Replace React.createElement with regular jsx

* detach setControlValue

* enable render trigger

* Pass explicit props rather than pass all that exists in payload.data. Also use formData when possible.

* Rename sliceWidth, sliceHeight to width, height. Use deconstructor. Extract function.

* use arrow function

* fix linting and remove css
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.28.0 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.28.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants