Explore clarifying that remote repository has uncommitted changes on entry and exit

[joyceerhl]

Problem

1. User makes edits in github.dev via RemoteHub without committing.
2. Time passes. Repository is modified by current user or other collaborators
3. User goes to github.com and hits . to open in github.dev.
4. User is confused that repository doesn't reflect latest state of repository that they were just looking at in the github.com UI.

User may or may not notice that there are uncommitted changes and may or may not know that uncommitted changes prevent syncing with the remote.

Ideas

• When a user reenters a workspace with uncommitted changes, tell them they have uncommitted changes from a prior edit session (can surface "last edited" timestamps that we already show in the Remote Explorer) and that they must revert or commit their changes in order to sync with the remote
• When a user tries to close the tab without committing changes, display a dialog (consider using onWillDisposeWorkbench.runCommands) that displays a custom modal (Ben or Deepak may know more). We already do a blocking native dialog when the user does Ctrl+W (which normally closes the active editor but in web closes the tab). At minimum the dialog we display here must have a Don't Show Again option
  

[joyceerhl]

Current behavior:

• If there are no uncommitted changes, RemoteHub always shows you the latest version of a branch
• We fetch, not sync, if there are uncommitted changes

Papercuts/hypotheses:

• Users don't notice that there are uncommitted changes
• Users are surprised that uncommitted changes are persisted
• Users are surprised that uncommitted changes prevent automatic sync

Related prior art:

• git.autoStash setting in desktop which automatically stashes working changes when pulling
  

• GitHub.com PR UI stale indicator
  

---

Ideas so far:

1. Modal dialog to explain the connection between uncommitted changes and syncing
   
   

2. Setting to configure auto-syncing if there won't be conflicts with incoming changes + notification <-- issue with this approach is that the notification will likely get lost
   
   

3. A consolidated setting which prompts if not configured to auto-sync
   
   

---

Outstanding issues/questions:

• Is it accurate to assume that users on github.dev/vscode.dev always want to see the latest version of their branch ('Always Autosync' option)?
• What should happen if the sync will result in merge conflicts? --> abort or attempt merge? Should this also be configurable or is that too complicated?
• Is this dialog useful enough to warrant its obtrusiveness?
• Are there other ways that we can either explain the connection between uncommitted changes and auto syncing through UI indicators (similar to what GitHub.com does with the 'Refresh' button on stale PRs), or improve the default behavior?
• Do we consider something drastic like making github.dev a truly ephemeral editor where uncommitted changes aren't persisted after navigating away, similar to how github.com behaves?

[joyceerhl]

@bpasero do you have any recommendations for how RemoteHub can reliably display a dialog when the user is about to navigate away from a repo with uncommitted changes?

Based on @sandy081's suggestion at UX sync yesterday, I looked into whether we can display a warning before the user leaves a page containing uncommitted changes. Something like this in vscode.dev when the user navigates away, so that uncommitted changes are not abandoned:

Implementation above is done by registering a command in RemoteHub to display a prompt when there are uncommitted changes, and then including it as a command to run before page unload using the embedder's onWillDisposeWorkbench API

		const onWillDisposeWorkbench: IPreWorkbenchDisposalActions = {
			runCommands: [{ command: 'remoteHub.uncommittedChangesPrompt', args: [] }],
		};

Sadly it seems like we are limited by the browser's APIs here: https://developer.mozilla.org/en-US/docs/Web/API/Window/beforeunload_event This approach would result in us displaying two dialogs--one from RemoteHub with a detailed message about uncommitted changes, and one generic one from the browser with a non-configurable message that contradicts what the RemoteHub dialog says. :(

In addition it seems that

1. beforeunload handler is not invoked when the user closes the browser tab by clicking x, or when the user navigates away by changing the url in the url bar. It's only invoked on ctrl+w for example, so this really doesn't help much
2. It is sometimes invoked when the user refreshes the page but not always. The docs do mention that the beforeunload event is not reliably fired
3. Not sure if the beforeunload handler can be made async--right now I can only get the dialog to appear by hitting ctrl+w > Leave Site? dialog appears > select Cancel

[bpasero]

@joyceerhl in short: there is absolutely nothing we can do:

• you cannot alter the UI that shows when leaving a page
• you cannot run long running operations
• you cannot force prevent the user from leaving a page
• you cannot even know how much time you get to perform something when the user leaves

In long: MicrosoftEdge/MSEdgeExplainers#147

[bpasero]

To give a bit more context: VSCode allows participants to participate in the shutdown phase (via ILifecycleService) and on desktop we can support all of it:

• long running operations
• custom dialogs
• blocking the shutdown until everyone participated
• even block the shutdown in case of error

When we ported VSCode to the web we had to drop all of that and limit it to a single veto from the beforeUnload handler which has the issues outlined in my first comment.

A few examples:

• a pending not-finished save will try to veto the unload
• a pending not-finished backup will try to veto the unload

To mitigate some of it, UI state is written to IndexedDB every 10 seconds (I know, ugly) to make sure that any accumulated UI state is not lost when leaving the app.

The answer from browser vendors so far has been to either:

• use a ServiceWorker that can continue to run in the background even after page unload
• "rescue" any data over to a public endpoint via sendBeacon API which afaik is guaranteed to finish on unload

[joyceerhl]

Yeah, this is unfortunate but it makes sense, and thank you for the context. Until this behavior changes, it seems that it's not feasible for RemoteHub to clarify what happens to uncommitted changes on exit.

In that case I think it's viable to pursue clarifying that there are uncommitted changes which prevent us from syncing with the remote. The proposal here is a dialog shown on entering a repo which is outdated, controlled by a setting "remoteHub.experimental.uncommittedChangesOnEntry":

I also briefly explored nudging the user to commit often, and clarifying that uncommitted changes are stored locally until the user commits since we've gotten feedback that it's unclear where edits go. The prototype I showed of this is a setting "remoteHub.commitOnSave" which will commit the current file changes when the user hits Ctrl+S, with a default commit message like "Update {filename}". In order for this proposal to be viable we will need to ensure that this interacts well with the "files.autoSave" setting and that there aren't any performance issues. Maybe we can also explore unifying this auto-commit behavior with the "editor.codeActionsOnSave" setting.

[joyceerhl]

After selfhosting the uncommitted changes prompt without issues since last Friday, I've pushed a change to enable it in tonight's pre-release RemoteHub (therefore also insiders.vscode.dev). I added a Don't Show Again action to the dialog so that users can easily disable it if they don't find it helpful. Feedback is very welcome.

[joyceerhl]

Closing this out as the modal dialog is now enabled in pre-releases by default and I've tweaked it based on feedback from the team.