Skip to content
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

Desktop user experience overhaul #42

Open
wants to merge 62 commits into
base: main
Choose a base branch
from
Open

Conversation

SleepyLeslie
Copy link
Collaborator

@SleepyLeslie SleepyLeslie commented Jul 10, 2024

This PR refactored a major portion of Grist Desktop:

Filesystem changes

Grist Desktop used to rely on symlinks to comply with Grist Core's requirement that all documents be stored centrally in GRIST_DATA_DIR. This causes issues on Windows, and impacts the user experience since users cannot choose where to store their Grist documents.

Moreover, this contributed to a bad user experience as documents use docID as name, and GRIST_DATA_DIR defaults to the "Documents" folder. This means users will end up having a lot of <random string>.grist files in their "Documents" folder.

It has also led to complaints from macOS users, since they basically cannot use Grist Desktop without granting permissions to the "Documents" folder.

This PR enables Grist Desktop to open Grist documents anywhere on the local filesystem, without making symlinks. All modifications will directly write to the document.

Window manager

Grist Desktop now knows all documents it has open, and would bring the open document to you if you open it twice. It can keep track of window content changes (i.e. when a user navigates away from / into a document).

It must work together with this PR for grist-core gristlabs/grist-core#1099

@SleepyLeslie
Copy link
Collaborator Author

SleepyLeslie commented Jul 11, 2024

Tested to work on Linux, Windows 10 and Intel Macs; can open documents and symlinks to documents anywhere on the filesystem.

  • macOS open-file events are handled correctly.
  • Can use CLI argument to open file (Windows and Linux) correctly.
  • Single instance lock is handled correctly.

Changes unrelated to the filesystem don't seem to break anything (good!)

Some comments:

  • Can no longer create new document. This is because new documents don't have externalId by default. More changes to grist-core is required. This must be fixed. We can probably also support the "file -> new" menu properly.
  • To prevent opening the same document twice, grist-desktop tracks open documents with a map from document IDs to Electron windows. If a user attempts to open an already open document, the corresponding window is raised. However, this mapping doesn't change when the user navigates to the home page or to another document, leading to weird behavior.
  • After a document is deleted, it is still shown in the "All Documents" list. Attempting to open it will result in an unrecoverable error. It can be dismissed by clicking outside and going back to the home page, but this is bad UX. We should report an informative error message (rather than the dummy "error accessing document"), take the user back, then delete the entry from the database automatically.

@SleepyLeslie
Copy link
Collaborator Author

Now it is possible to create new documents using the in-app "Create Empty Document" button. However, this button will only work for the electron user. Should any user choose to use Grist Desktop as a self-hosted server, external visitors will now be unable to create new documents - they will get an error when clicking on the button.

@SleepyLeslie
Copy link
Collaborator Author

Implemented WindowManager to track open documents properly.

@SleepyLeslie SleepyLeslie changed the title Enable opening Grist documents anywhere directly Desktop user experience overhaul Jul 19, 2024
@mpsijm mpsijm mentioned this pull request Sep 6, 2024
paulfitz pushed a commit to gristlabs/grist-core that referenced this pull request Sep 17, 2024
Make a set of changes required for Desktop FS improvements, see
gristlabs/grist-desktop#42

---------

Co-authored-by: Spoffy <[email protected]>
Co-authored-by: Spoffy <[email protected]>

export const create: ICreate = makeSimpleCreator({
deploymentType: "electron",
sessionSecret: "something",
Copy link
Member

@paulfitz paulfitz Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe no-longer-needed instead of something just to emphasize it isn't important anymore, we haven't just forgotten to put something secure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants