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

[geo] Add JS controls to remaining layers #4272

Merged
merged 7 commits into from
Jan 25, 2018
Merged

[geo] Add JS controls to remaining layers #4272

merged 7 commits into from
Jan 25, 2018

Conversation

hughhhh
Copy link
Member

@hughhhh hughhhh commented Jan 23, 2018

Added JS controls to remaining DeckGL layers
merged small bugfix with arc layer payload #4238

@mistercrunch

@hughhhh hughhhh changed the title Js controls [Geo] Add JS controls to remaining layers Jan 23, 2018
@hughhhh hughhhh changed the title [Geo] Add JS controls to remaining layers [geo] Add JS controls to remaining layers Jan 23, 2018
Copy link
Member

@mistercrunch mistercrunch left a comment

Choose a reason for hiding this comment

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

Running into a bit of a conflict with #4262 here. Let's make sure both PRs align.

if (fd.js_datapoint_mutator) {
// Applying user defined data mutator if defined
const jsFnMutator = sandboxedEval(fd.js_datapoint_mutator);
data = data.map(jsFnMutator);
Copy link
Member

Choose a reason for hiding this comment

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

With my recent PR we moved to js_data_mutator instead of js_datapoint_mutator and the call goes data = jsFnMutator(data);

...d,
color: [c.r, c.g, c.b, 255 * c.a],
}));

if (fd.js_datapoint_mutator) {
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as above. Related PR: #4262

...d,
color: [c.r, c.g, c.b, 255 * c.a],
}));

if (fd.js_datapoint_mutator) {
Copy link
Member

Choose a reason for hiding this comment

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

Same here...

@hughhhh
Copy link
Member Author

hughhhh commented Jan 23, 2018

@mistercrunch lets make sure #4262 goes in first

@mistercrunch
Copy link
Member

Now that my changes are in master let's rebase to make sure things look ok. We may want to switch to avoid the "Squash and merge" flow in Github an follow the "Create a merge commit" option instead eventually.

t('Javascript data point mutator'),
t('Define a javascript function that receives each data point and can alter it ' +
'before getting sent to the deck.gl layer'),
js_data_mutator: jsFunctionControl(
Copy link
Member

Choose a reason for hiding this comment

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

Sorry I had not yet merge this change when I asked you to rebase earlier, now it's merge and you should re-re-rebase

@mistercrunch mistercrunch merged commit 2384ad4 into apache:master Jan 25, 2018
mistercrunch added a commit that referenced this pull request Jan 27, 2018
michellethomas pushed a commit to michellethomas/panoramix that referenced this pull request May 24, 2018
* Update viz.py

* added JS controls

* add JS to grid layout

* add JS to hexagon layer

* added JS controls to screengrid

* update to js_data_mutator controls

* remove .map()
wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018
* Update viz.py

* added JS controls

* add JS to grid layout

* add JS to hexagon layer

* added JS controls to screengrid

* update to js_data_mutator controls

* remove .map()
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.23.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.23.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants