Auto zoom when adding a layer from the UI#1286
Conversation
|
That's really cool thanks! This PR is fixing #667 Screencast.From.2026-04-09.14-11-26.mp4 |
martinRenou
left a comment
There was a problem hiding this comment.
I badly want to merge this PR, but I don't think we can as it is... The issue is that when collaboratively annotating a map with markers we'll have the view keep on jumping whenever someone else is adding a new annotation.
We need to think of a way to track the origin of the change, and only zoom to the layer if it was created by the current user. I guess we have no way of knowing that though... Thoughts @gjmooney @davidbrochart @trungleduc ?
|
We could introduce in our document model an "origin" attribute for Layers, that's a UUID generated by the front-end. If the origin of the layer is the same as the current page, we do zoom. I wonder if the added "noise" in the document would make it annoying or if it would be just fine. |
|
Doesn't the awareness stuff have a unique identifier? Could we use that as the origin?Alternatively, we could disable the zoom if there's other collaborators connected? |
Probably that's a good degraded solution for now indeed 👍🏽 then we don't mess around with the document model (yet?). If we really want that in the multi-collaborators case we can rethink this later. |
|
Couldn't this be an event in the awareness, but not the document, like cursor moves? I feel like I'm missing something important here! |
Indeed I guess we could explore having a new entry in the awareness, If we take this approach, we need to hack around the awareness when it comes from Python. Can you maybe test this approach @nakul-py ? |
Not sure what you mean by "hack" here, but to me the least hacky way to do this would be to specify the source of the event -- was it triggered by an LLM, by programmatic API call, or by user interaction? e.g. I know very little about this part of JupyterGIS and I'd like to learn more about it and see and touch it. Maybe we could have a session to pair/group program on it some time? |
| syncSelected(value: { [key: string]: ISelection }, emitter?: string): void { | ||
| this.sharedModel.awareness.setLocalStateField('selected', { | ||
| value, | ||
| emitter, | ||
| }); | ||
| } | ||
|
|
||
| syncLastAddedLayer(layerId: string): void { | ||
| this.sharedModel.awareness.setLocalStateField('lastAddedLayer', { | ||
| layerId, | ||
| clientId: this.getClientId(), | ||
| }); | ||
| } |
There was a problem hiding this comment.
Can you double check why we don't need to inject clientId for syncSelected but we do for "lastAddedLayer"? Since the selection is bound to the current user I assume it's bound to the clientId somehow too. So you probably don't need to add clientId here. To be confirmed.
There was a problem hiding this comment.
OK let me check this one 👍
There was a problem hiding this comment.
Yes we can get rid of manually adding clientId and getting it from awareness.
|
This is looking nice! Although for the Python API it does not work how we want: Screencast.From.2026-04-14.10-55-08.mp4
Now, that is what I meant. The Python API does not write into the awareness so we'll need to have a special case for this one. |
martinRenou
left a comment
There was a problem hiding this comment.
Thanks! Let's get this one in and figure out the Python case separately. #1246 will probably help us, or at least be a good step in the direction we want.
Description
Auto zoom to layer's extent when adding a layer from the UI.
Also now in collaboration only zooms for whom, who created the layer, Not for all collaborators.
Respects the user view 👍
zoom-collab.mp4
explore(data)#667Checklist
Resolves #XXX.Failing lint checks can be resolved with:
pre-commit run --all-filesjlpm run lintCITATION.cffcontains an author entry for yourself📚 Documentation preview: https://jupytergis--1286.org.readthedocs.build/en/1286/
💡 JupyterLite preview: https://jupytergis--1286.org.readthedocs.build/en/1286/lite
💡 Specta preview: https://jupytergis--1286.org.readthedocs.build/en/1286/lite/specta