Improve UX for the Force Upload [one time force upload]

[borekb]

I'm probably hitting an issue created by "Safe Upload" feature #923 (cc @arnohovhannisyan) as I just updated keybindings.json and got this:

I currently use a single computer, the Gist is 14 days old and I have just updated keybindings.json, so I think it's a bug.

Even if it isn't a bug, I'd like to comment on the UX in general because I find it relatively confusing:

• The dialog doesn't tell me which of the two cases I'm in. Same versions mean no risk in forcing an upload; newer Gist mean high risk of overwriting my data.
• Even if I wanted to forcibly upload the settings now, I don't like that the way to do it is to permanently enable forceUpload in my settings. I'd much rather just invoke a one-time "Force Upload" command and be warned next time again (because, again, one of the scenarios is high-risk).

🌴 Visual Studio Code Version : 1.38.0
🌴 Code Settings Sync Version : 3.4.2

[borekb]

Oh, I clicked "Yes", it added "sync.forceUpload": true to my settings.json but the updated Gist does not contain this. So my local settings.json are immediately out of sync with the Gist 😕.

[borekb]

Hmm, not sure if this is about keybindings.json after all, see Extension always asks to enable Force Upload #1016.

I'm not sure if this is an additional issue beside #1016 or a duplicate of it...

[karl-lunarg]

Yes, version 3.4.2 definitely had a regression. PR #1026 was just approved for 3.4.3 which fixes this particular refusal to upload, but the PR does not change the UI.

BTW, I've been working around the refusal to upload problem by going ahead and letting the dialog set Force Upload and then immediately resetting Force Upload to off in my local settings. I'm sort of glad that it didn't update the Force Upload setting in the Gist. In any case, this should be better in the next release.

As far as the UX goes, I agree with @borekb. I like how you get a status message when you try to Download but already have the same settings as the Gist. Perhaps the same should be done for Upload.

The Upload path in the extension compares the local settings with the Gist settings and therefore knows if they are different or not.

The Upload logic could be:

• If the local and Gist are the same, display a 3-second status message saying so and don't do anything.

• If the local and Gist are different and the local LastDownload is earlier than the Gist LastUpload, then display a popup dialog warning that there is a risk in overwriting changes in the Gist. Don't say in the dialog that they could be identical because they are not. Don't do anything, but perhaps offer the same option to force the upload without changing the local ForceUpload setting.

• If the local and Gist are different and the local LastDownload is later than or same as the Gist LastUpload, then upload the settings with no popup dialog. Perhaps add a 3-second status message saying that the settings were uploaded. I think that the upload details are also shown in the console.

It would be nice if this sort of UX improvement could be made for the 3.4.3 release if @shanalikhan and @borekb are in agreement.

[shanalikhan]

I'd much rather just invoke a one-time "Force Upload" command and be warned next time again

Yes, i agree with @borekb we can add one more button in this popup.

1. Yes
2. Always Force Upload ( text can be improved )
3. Don't Show This again.

@karl-lunarg

The Upload logic could be...

Yes, the first 2 of 3 you have mentioned were kept in mind when this feature was originally implemented.
These are 3 scenarios we can cover. I hope the PR that is merged recently should be covering these scenarios.

As v3.4.3 is not released yet, if you can send PR that contain the updated button for the updated UX and verifying the above 3 scenarios you mentioned, v3.4.3 would be concrete release :)

[borekb]

If the local and Gist are different and the local LastDownload is earlier than the Gist LastUpload, then display a popup dialog warning that there is a risk in overwriting changes in the Gist. Don't say in the dialog that they could be identical because they are not. Don't do anything, but perhaps offer the same option to force the upload without changing the local ForceUpload setting.

This is the key 👍 . Well-articulated, @karl-lunarg! I fully agree with your description and suggestions.

[karl-lunarg]

Yes, i agree with @borekb we can add one more button in this popup.

1. Yes
2. Always Force Upload ( text can be improved )
3. Don't Show This again.

Since we're talking about changing the UX anyway, how about making it simpler instead of more complex and potentially confusing?

I'm thinking that the popup should just say: "The settings in the Gist were changed after you last downloaded them. Upload anyway?". And there would be two buttons: Yes and No. Closing the popup would be the same as No. No side effects.

The popup would only display when there is potential for data loss, as is the case here. As such, it should ALWAYS be displayed when there is this potential unless the user explicitly requests ForceUpload.

So:

• There is no "Don't show this again" setting or UX button to set this setting. The dialog won't show if the user explicitly set ForceUpload in the settings because the prompt isn't needed to get the "Yes" response implied by ForceUpload being on.

• There is also no button in the popup UX to set the ForceUpload setting. There is already a way to do this via the normal settings. If a person repeatedly hits the Yes button on the proposed UX, they can set ForceUpload to avoid the popup if they intend to force all the time.

I think that this approach has less ambiguity and fewer side-effects.

Does this sound good to everyone?

If we settle on something, I can try working on the implementation. But I am a Typescript and vs code internals noob. I think that there will be a lot of localization and documentation work, so it could take a while.

[shanalikhan]

@karl-lunarg

Does this sound good to everyone?

That's sound perfect. No problem, take your time. It would be great if we can release the improved UX in the upcoming release. :)

[karl-lunarg]

I've started working on this.

Note that I think that there is a problem with the latest commit for #993. The gist file name separator is supposed to be "|". If you have snippets, the data sent during an upload to patch the gist is malformed. I've included a commit that will be in this PR to correct this problem.