-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add support for Map output #102
base: main
Are you sure you want to change the base?
Conversation
c-milocanovich
commented
Jun 7, 2024
- Add support to map
- Allow types Bubble and Choropleth
- Click item action
- Select and unselect point actions
…ols-library into feat/query-viewer-map
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for implementing the output! I leave some comments.
src/components/query-viewer-map/controller/query-viewer-map-controller.tsx
Outdated
Show resolved
Hide resolved
src/components/query-viewer-map/controller/query-viewer-map-controller.tsx
Outdated
Show resolved
Hide resolved
src/components/query-viewer-map/controller/query-viewer-map-controller.tsx
Outdated
Show resolved
Hide resolved
src/components/query-viewer-map/controller/query-viewer-map-controller.tsx
Outdated
Show resolved
Hide resolved
src/components/query-viewer-map/controller/query-viewer-map-controller.tsx
Outdated
Show resolved
Hide resolved
componentDidRender() { | ||
this.renderMap(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to have a more reactive approach to re-rendering the map?
For example, currently, if the queryTitle property is updated at runtime, the map will be destroyed and recreated just to update the title. But, if we use a Watch to execute a method that updates the title and don't execute the renderMap method, we would save a lot of proccessing and time.
The same goes for all properties when we can skip the entire map recreation by running a method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: The current behavior of the map matches what the chart output does, so in the future we'll have to do the same with the chart.
src/components/query-viewer/controller/query-viewer-controller.tsx
Outdated
Show resolved
Hide resolved
src/components/query-viewer-map/controller/query-viewer-map-controller.tsx
Outdated
Show resolved
Hide resolved
remove use of bind