-
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
base: master
Are you sure you want to change the base?
Layer Comparison #239
Conversation
… 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.
| const currentLayerStyles = computed(() => { | ||
| return { | ||
| A: compareStore.compareLayerStyles.A[styleKey.value], | ||
| B: compareStore.compareLayerStyles.B[styleKey.value], | ||
| } | ||
| }) |
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.
If I have a saved style selected for each layer when I enable compare mode, this is not populated with the current selected styles.
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 made it so it should grab the default style if it is specified in the compareStore when loaded.
| <v-card-text class="pa-2"> | ||
| <v-row> | ||
| <v-card width="400" flat color="transparent"> | ||
| <v-card-title>{{ compareStore.orientation === 'vertical' ? 'Left Map' : 'Top Map' }}</v-card-title> |
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.
Not sure if this would look too crowded, but when the comparison orientation is vertical (and we have left vs. right maps instead of top vs. bottom), can we organize the two style selections in this menu to be side-by-side instead of stacked? The visibility checkboxes are side-by-side, so let's try to maintain that spatial relationship when we can.
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.
web/src/store/compare.ts
Outdated
| const getBaseLayerSourceIds = () => { | ||
| const map = mapStore.getMap(); | ||
| if (!map) return []; | ||
| return map.getStyle().layers.filter((layer: any) => { | ||
| const layerWithSource = layer as { source?: string }; | ||
| return layerWithSource.source?.includes('openstreetmap'); | ||
| }).map((layer: any) => layer.id); | ||
| } |
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.
There is a similar function with the same name in the map store. They have different logic, though. Can you reconcile this so only one getBaseLayerSourceIds exists?
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 realize I actually need a getBaseLayerIds for usage in the ToggleCompare.vue component because the visibility is dependent on the layerIds and not the sourceIds. I created that new function when integrating/merging the user basemaps. Still need to fix some issues with loading the basemap in the secondary (MapB) 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.
I've made a different function in map.ts getBaseLayerIds in f4f0a2a
|
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


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