Skip to content

[WIP] Add "Save as..." command and toolbar button #596

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mfisher87
Copy link
Member

@mfisher87 mfisher87 commented Apr 2, 2025

Description

We want to enable users working on nameless documents (doc = GISDocument()) to save that document from the UI (#594). The current "Save JGIS As..." menu entry from JupyterLab works great in a full view, but not in a widget view or sidecar view.

Checklist

  • PR has a descriptive title and content.
  • PR description contains references to any issues the PR resolves, e.g. Resolves #XXX.
  • PR has one of the labels: documentation, bug, enhancement, feature, maintenance
  • Checks are passing.
    Failing lint checks can be resolved with:
    • pre-commit run --all-files
    • jlpm run lint

Copy link
Contributor

github-actions bot commented Apr 2, 2025

Binder 👈 Launch a Binder on branch mfisher87/jupytergis/save-as-in-ui

Copy link
Contributor

github-actions bot commented Apr 2, 2025

Integration tests report: appsharing.space

@mfisher87 mfisher87 changed the title [WIP] Init "Save as..." command and toolbar button [WIP] Add "Save as..." command and toolbar button Apr 2, 2025
@mfisher87
Copy link
Member Author

mfisher87 commented Apr 3, 2025

TODO: Add co-authors @tawoodard @YaoTingYao @arjxn-py @martinRenou @rjpolackwich

@mfisher87
Copy link
Member Author

@martinRenou @brichet @gjmooney @simonprovost

I would love to hear your thoughts on this: Right now we can save the file as a new filename and update the model to reflect that. However, changes applied to the widget in the GUI after, for example, adding a new layer, do not get saved to the new filename. "Saving as" a 2nd time, the filename placeholder is properly updated as we expect, but those underlying saves are not happening.

@mfisher87 mfisher87 added enhancement New feature or request help wanted Extra attention is needed labels Apr 7, 2025
mfisher87 and others added 3 commits April 7, 2025 12:46
Co-authored-by: Arjun Verma <[email protected]>
Co-authored-by: Martin Renou <[email protected]>
Co-authored-by: Yao-Ting Yao <[email protected]>
Co-authored-by: Jamison Polackwich <[email protected]>
Co-authored-by: Tammy Woodard <[email protected]>
Fix the file extension automatically if it doesn't end in jGIS

FIXME: Does not save changes to the shared document after changing the
filePath.

Co-authored-by: Arjun Verma <[email protected]>
@brichet
Copy link
Collaborator

brichet commented Apr 8, 2025

@mfisher87 I don't think there is a simple way to achieve it.

The file is updated on disk with the shared model of the widget model. I think that, at least, the shared model should be recreated to be able to update the file on disk (probably using this).
Currently the shared model is a read-only property of the widget model. I'm not sure if it would work to change it on the fly (if we make it writable first).

A other solution (maybe easier) could be to replace the whole widget by a new one.

Maybe @davidbrochart or @trungleduc have a better idea about it.

@brichet
Copy link
Collaborator

brichet commented Apr 8, 2025

Thinking again about it, I wonder if it won't be confusing for user to change the shared model on the fly.
When you run the Notebook, you use an unsaved widget. After saving it, you use a widget saved to a file. If you run again the Notebook, you fall back on an unsaved widget, while you could think that your changes will be saved to the disk.

It could be only an export as button for now, to avoid confusion.

@martinRenou
Copy link
Member

martinRenou commented Apr 8, 2025

A other solution (maybe easier) could be to replace the whole widget by a new one.

It looks to me that this is what JupyterLab does already with notebooks when you run save-as, since I see the widget blink (as if we closed the widget and reopened it). To be verified in the code itself.

@brichet
Copy link
Collaborator

brichet commented Apr 8, 2025

Another way to avoid confusion could be to have 2 classes:

  • GISDocument requiring a filename at instantiation and backed by a file
  • GISSandbox ( or GISExplore, to reuse the suggestion of @fperez about the "bounce" function), that is never backed by a file. It could still export the project to a file, without synchronisation after export.

@mfisher87
Copy link
Member Author

@brichet Interesting idea! Fernando compared this dynamic to similar core concepts in JupyterLab: a Notebook, which is file-backed at all times, and a console, which is ephemeral, not file-backed. When a user opens a console vs a Notebook, they look significantly different and visual cues enable the user to reason about the difference between the two environments.

Do you have thoughts on what visual cues might help the user differentiate between those different classes and how they must use them differently? For example, if calling an explore function from a Notebook opens an interface using GISSandbox that looks similar to an interface using GISDocument, how does the user know that they must behave differently to save their work?

@brichet
Copy link
Collaborator

brichet commented Apr 11, 2025

Do you have thoughts on what visual cues might help the user differentiate between those different classes and how they must use them differently?

I don't know, but I agree that it has to be obvious that it's not saved.

Maybe an additional item in the toolbar and/or a border to the widget.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants