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

feat: make all deckgl charts handle their own tooltips #13

Merged
merged 2 commits into from
Mar 19, 2020
Merged

feat: make all deckgl charts handle their own tooltips #13

merged 2 commits into from
Mar 19, 2020

Conversation

kristw
Copy link
Collaborator

@kristw kristw commented Mar 19, 2020

馃弳 Enhancements

Following the same approach in #12 for all charts.

image
image
image
image
image

@kristw kristw requested a review from a team as a code owner March 19, 2020 22:05
@kristw kristw mentioned this pull request Mar 19, 2020
Merged
1 task
espressoroaster
espressoroaster reviewed Mar 19, 2020
View reviewed changes
packages/superset-ui-legacy-preset-chart-deckgl/src/layers/Geojson/Geojson.jsx
render() {
const { formData, payload, setControlValue, onAddFilter, viewport } = this.props;

// TODO get this to work
Copy link

Choose a reason for hiding this comment

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

What is autozoom being used for? Has it ever been supported?

Copy link

Choose a reason for hiding this comment

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

Seems like a legacy comment from before you touched it, just asking in case you have context.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, legacy comment. I also have no idea.

espressoroaster
espressoroaster approved these changes Mar 19, 2020
View reviewed changes
Copy link

@espressoroaster espressoroaster left a comment

Choose a reason for hiding this comment

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

LGTM. We could trade the extra ref access for a const.

setTooltip = tooltip => {
    const { current } = this.containerRef;
    if (current) {
        current.setTooltip(tooltip);
    }
}

@kristw kristw merged commit e3f1861 into master Mar 19, 2020
@delete-merged-branch delete-merged-branch bot deleted the kristw--tooltip2 branch March 19, 2020 23:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@espressoroaster espressoroaster espressoroaster approved these changes

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@kristw @espressoroaster