Notification usage guidelines

[isidorn]

I think we should:

• Revisit all internal usages of notifications and consider if they should be converted to modal dialogs if they require immediate user actions
• Once we figure out the best thing to do in VS Code core we should update our extension guidelines so extension can follow the patterns that we do in core

Ideally this exercise would move "important" to be modal dialogs, and thus we can start exercising a more aggressive hide strategy towards regular notifications.

fyi @devinvalenciano @meganrogge

[Tyriar]

The terminal workspace permissions dialogs should be modals imo, but that might get fixed when we adopt trusted workspaces #119344

[isidorn]

@misolori is working on a nice flow chart to make it easy to distinguish if a usage should use a modal dialog or a notification.
Until he has that, here are some rules of thumb which I think can be helpful:

• User interaction immediately needed -> modal
• Feature can not continue without input -> modal
• Not directly user initiated -> notification

Also I am not aware that we have any modal dialogs that should instead be notifications.
So for this exercise I plan to simply go through the usages of prompt api. I will list any questionable usages here.

[isidorn]

Notifications which might no longer be needed:

• FindWidget @rebornix
• Close Workspace action @sandy081 I think this action should be disabled when there is no workspace, so ideally this notification is not needed
• Settings are automatically saved @sandy081 I think we have educated our users enough and I think the future is that setting do not have to be Applied. So I would think about removing this notification alltogether
• Tests shuold this be an .error or at least a warn @connor4312
• License @joaomoreno I think we can just remove this
• Welcome @chrmarti this feels like it should use the progress api? And not notifications
• Sync Sync turned on. @sandy081 do we really need to notify the user that sync is on, doesn't the activity bar badge update accordingly? Actually in this file there are a lot of notifications usages. If you think @sandy081 all of them make good sense feel free to check this off.
• Extensions Install @sandy081 should this action be disabled if there are no workspace extension recommendations?

I have removed some notifications in debug and file land already and changed some from info to warn in undoRedoService.

[isidorn]

Here's a list of questionable Notifications which are used for user prompts:

• MenuBar @sbatten we might be able to remove this since now the custom menu is on by default, correct?
• LargeFileOptimisation @alexdima the flow here is -> we show notification with actions -> user chooses one -> we show another notification asking the user to reload the file. Could we improve this flow to just have one prompt notification which would just update the setting and automatically reload the file
• Workspace recommendation prompt captured here "Do you want to install the recommend extensions for this repository" should not show up on startup #121802
• Malicious Extension Uninstalled @sandy081 why would a user want to reload? If it is required for security we should use a modal dialog, if not then we should probably just use a status message to notify the user
• All extensions disabled @sandy081 I think all extensions are disabled due to user action and then we still show this notification on startup. Does it makes sense to instead show a badge on the Extensions viewlet?
• Formatter @jrieken The first .prompt should actually be a warn and the second one might deserve to be a modal dialog since it is a user triggered action which can not proceed.
• Installation corrupt @alexdima I believe this should be changed to be a modal dialog, and to not allow to not show again. Since we do not really want the user to continue using VS Code in this state
• Tasks @alexr00 I know you are removing old version of tasks, since you are revisiting that can you maybe review the usage of .prompt in tasks. If it is user blocking it should be a modal dialog, if we can get rid of it even better. This one looks like we can remove it once we have workspace trust
• Localization @dbaeumer after a user has installed an extension we show a notification asking if he would like to apply and reload, since this is user initiated it might make sense to use a modal dialog. What do you think?
• Trusted Domains @JacksonKearl so here we open the trusted domains editor and show a notification. Can we just have the explanation in the trusted domains editor and not show the notification at all?
• Update @joaomoreno can we just use a progress service here for the prompt that it is in progress? That way might be a bit less obtrusive

[alexr00]

I removed the tasks prompt related to tasks v1 ✔.
However, I also added a new one to notify users that I upgraded their tasks from v1 to v2. This happens on startup, and few people will actually see it because of the low use of tasks v1:

For the automatic tasks prompt, I expect we can move that to a setting and not show a prompt once workspace trust is turned on.

[isidorn]

@alexr00 great, thanks a lot! As for the the new notification it makes sense, however if the automatic upgrade just works I do not see a point in showing it. Though I guess it might not just work so that is the reason to show it. But in the long run (after 3 milestones) I would just remove it as well.
And yeah for the automatic task prompt - removing it next milestone would be great.

[alexr00]

I think we need to show something after the tasks upgrade. It will not be perfect, and the user will see that they have changes in their workspace when they go to commit. I want to make sure they know why that file changed.

[isidorn]

I have manually reviewed all internal usages of the notificationService. In the comment above I have listed all usages which I think should be discussed and maybe improved. For larger items I have created issues and linked them to this one.

I think we are actually not so bad in VS Code core, but I think we can still improve to be a great role model for our extension ecosystem.
I have assigned the owners to issues and checkbox items - thanks for your help!

[dbaeumer]

@isidorn regarding localization: I am not a fan of a modal dialog here. Best would be to have something during install that lets user choose to automatically switch to that locale.

[alexdima]

I suggest that we also offer some kind of API which says "show notification if no other notifications are currently shown".

For example, here's what I got today. The survey notification should really not show at the same time when 2 other notifications are shown.

[isidorn]

@alexdima good point, I outline the same idea in this comment #121802 (comment)
Though this holds particularly true for survey notifications

[JacksonKearl]

Re using status bar progress for low priority notifications, I've been using that to show the download of the search index in remote hub and I've gotten feedback from a few different people (@kieferrm , @lszomoru , maybe others) that they did not see it at all. I think there's a general concern that people aren't going to look down to the status bar for progress notifications while they are focused on elements elsewhere in the editor.

[isidorn]

@JacksonKearl yeah I think that's the point, if they are low priority it is fine if some users do not notice it.
Things like download progress of the search index sounds to me like a perfect candidate for the status bar. Unless it was triggered by the explicit user action in the search view, than on top of this showing a progress spin in the search view makes sense. Though you understand this flow better than me so feel free to ping me and we can connect on this in a short meeting.

[miguelsolorio]

I've pushed changes to the vscode website and added the decision tree from above. If anyone has any additional changes/feedback, please add them here. Thanks!

[isidorn]

@misolori thanks for updating the guidelines 👏
@devinvalenciano can we now share these guidelines with some of our partner extensions?

[yunusefendi52]

I just updated vscode to latest version, the formatter dialog now prevents me from typing because now it's blocking me, from the diagram above which desicion is this taken from? In my opinion the formatter dialog should not be blocking, I know it's user triggered but at the same time it's triggered while typing which should not be blocked and there is no need for immediate action

Note: Previous version if there is no formatter, it's showing notification prompt instead of dialog (which IMO is the correct one)

[isidorn]

We are not blindly following the diagram, it is just a recommendation on the best flow.
As for the formatter if you think this interrupts the flow please file a new issue with repro steps and ping me on it @isidorn and we can discuss it further. Thanks

[isidorn]

No further action items planned here, thus closing for now.