-
Notifications
You must be signed in to change notification settings - Fork 1
Layer Comparison #239
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
Layer Comparison #239
Conversation
Deploying geoinsight with
|
| Latest commit: |
890a2d1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a472979f.geoinsight.pages.dev |
| Branch Preview URL: | https://layer-comparison.geoinsight.pages.dev |
… use a single list
…e/add/remove layers/sources, opacity support
annehaley
left a comment
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.
When I opened the modified files in my own vscode environment, I noticed highlighting for whitespace inconsistencies and unused imports. Could you comb through modified files for those? We may need to update our client linting to catch those.
|
As for the reviewer questions in the description:
I think we should consolidate wherever possible, so let's try just using ToggleCompare.
Without the structure of a PR review, where would be the best place to put comments and track conversations? Should I start a google doc for it? |
Feel free to either do a google doc or if you feel they are actual changes/features that are needed place them as issues in the repo. |
|
Before doing a more thorough review of https://github.com/BryonLewis/vue-maplibre-compare, I have a high-level question to confirm my understanding. The library's README explains that three components are available: Is it possible to write all of this functionality into a single component? It seems that our use case on GeoInsight will need all of that functionality. After #250 (sorry for the complexity that PR adds to this one), we will want the ability to compare maps with different layer sets (with different styles) and different basemaps, and we still need the ability to toggle compare mode. Unless there is a technical reason that they need to be separate, I'm imagining that one component could offer all of those features. |
|
With the updated basemap user layers basemaplayers are layers that don't container Also remember that the layer ordering for sub-layers inside a vector pmtiles needs to be preserved as well. |
…tting and retrieving the styles
2d9b0a5 to
3742e94
Compare
annehaley
left a comment
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.
I tried out this feature locally and it's really smooth. This is pretty close to ready, but I have one more suggestion.
Could you make it so that opening the tooltip does not move the map location? Right now, the map will move to center the tooltip (both when compare mode is enabled and disabled)
I just want to confirm, this wasn't something I added during this PR. Centering the featureId that was clicked was something that happened whenever something was clicked for this PR. I created logic for that to work for each (left/right) map when in comparison mode. I can easily disable this for all items. It's done in this function: https://github.com/OpenGeoscience/geoinsight/blob/layer-comparison/web/src/components/map/MapTooltip.vue#L88. I can remove the code or just remove the refernce when clicking here: https://github.com/OpenGeoscience/geoinsight/blob/layer-comparison/web/src/components/map/MapTooltip.vue#L165 I.E you want me to remove all of the code, or perhaps save it in a utils? |
|
Oh you're right, we did add that as an intentional feature so that large tooltips are less likely to get cut off by map bounds. I think the centering can make sense when not using the compare mode, but it is disruptive while comparing two maps. The centering moves the map underneath without moving the comparison slider, so if my slider is positioned to split the map at a particular location, opening the tooltip will change where on the map the split occurs. Can you just disable the centering when comparison mode is enabled? |
annehaley
left a comment
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 changing the centering behavior when compare mode is enabled. This looks good to merge!
resolves #111
vue-maplibre-comparedependency for comparing mapsuseMapCompareStoreto handle theisComparingas well as properties for comparisonMapWrapper.vueto switch between default map and comparison mapmdi-comparebutton to selected Layers to toggle into comparison modeCompareLayersPanel.vueto panels that displays the left/right or top/bottom list of layers that can be toggled on/off for comparison. This doesn't include the ability to reorder or change styles or remove layers. To do that you need to go back to the main viewTODO:
vue-maplibre-comparefor that to be updated20251205 Updates/TODO:
ToggleCompareReviewer Questions:
TODO 20251216:
vue-maplibre-compareto know how to center the map