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

Hide notifications (info, warning) even when showing buttons #51013

Closed
eamodio opened this issue Jun 2, 2018 · 25 comments
Closed

Hide notifications (info, warning) even when showing buttons #51013

eamodio opened this issue Jun 2, 2018 · 25 comments
Assignees
Labels
api feature-request Request for new features or functionality on-testplan plan-item VS Code - planned item for upcoming ux User experience issues workbench-notifications Notification widget issues
Milestone

Comments

@eamodio
Copy link
Contributor

eamodio commented Jun 2, 2018

Currently there is no way to have a notification offer actions but also have it go away after a timeout. I would love the ability to specify that the actions provided to the notification api (showInformationMessage, etc) should only be shown (and cause the notification to "stick") if the user interacts (i.e. clicks) on the notification itself.

That way the notification would appear & behave the same as a notification without actions, unless the user clicks on the notification, and then it would expand to show the action buttons (in which case it would also turn off the hide timeout). And maybe these type of notifications are shown a bit longer than normal ones to give the user more time to interact if desired

/cc @bpasero

@vscodebot
Copy link

vscodebot bot commented Jun 2, 2018

(Experimental duplicate detection)
Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

@bpasero bpasero added feature-request Request for new features or functionality api workbench-notifications Notification widget issues labels Jun 2, 2018
@bpasero bpasero changed the title Add support for a notification with actions (buttons) that is behaves like one without until first interaction Allow to show notifications with buttons that hide after a timeout Jun 2, 2018
@eamodio
Copy link
Contributor Author

eamodio commented Sep 18, 2018

@bpasero is this something you'd accept a PR on? If so, what approach should be used? Ideally imomodal would be used to signal that notifications should stay open even if they have buttons, but that would change backward compat. Maybe if modal is explicitly set to false (vs undefined) it could be honored and allow the notification to go away even if there are buttons?

Thanks!

@bpasero
Copy link
Member

bpasero commented Sep 18, 2018

@eamodio we did an explicit design decision that notifications with buttons should be treated like prompts that require user interaction and thus never hide. What is the use case for changing the semantic here? For a user it is then very hard to know if a notification stays open or suddenly closes if there is no visual difference.

@bpasero bpasero added info-needed Issue requires more information from poster ux User experience issues labels Sep 18, 2018
@eamodio
Copy link
Contributor Author

eamodio commented Sep 18, 2018

In CodeStream we use notification for @mentions and DM messages (by default). And we don't want these notifications to stick around, but we do want a user to be able to interact with it (e.g. open the message). Right now because notification don't have a click action, nor the ability that have non-http[s] links (either command: custom scheme from registered UriProviders), we have no ability to support this. So we either show notifications with buttons that will never hide (bad ux) or have no ability to interact with the notification (currently we do that latter - but many users are complaining about it).

An alternative is maybe to just have a click action on the notification to allow a callback for when a notification is clicked on?

@eamodio
Copy link
Contributor Author

eamodio commented Sep 18, 2018

Although I was thinking that from the user point of the the ux for notifications with (that aren't model) and without buttons would be the same, until the user clicks on the notification. At which time the notification would expand, revealing the buttons, and also stopping the auto hide. So its behavior wouldn't be much of a user ux change.

@bpasero
Copy link
Member

bpasero commented Sep 19, 2018

Lets close as duplicate of #46571. The right UX is to use links in this case.

@bpasero bpasero closed this as completed Sep 19, 2018
@bpasero bpasero added the *duplicate Issue identified as a duplicate of another issue(s) label Sep 19, 2018
@eamodio
Copy link
Contributor Author

eamodio commented Sep 19, 2018

While links will work, I still feel like being able to have buttons provides a better experience.

So this would be the notification the user would see if they do nothing:
image

And this is what they would see if they click on the notification:
image

Versus links:
image

or everything is a link:
image

While the ux of the last works better of the 2 link options (because you can just click anywhere to perform the action), imo it is definitely strange for a user because I can see them asking why that notification a different color than the others.

@bpasero bpasero reopened this Oct 10, 2018
@bpasero bpasero removed *duplicate Issue identified as a duplicate of another issue(s) info-needed Issue requires more information from poster labels Oct 10, 2018
@bpasero bpasero self-assigned this Oct 10, 2018
@bpasero bpasero added this to the October 2018 milestone Oct 10, 2018
@bpasero
Copy link
Member

bpasero commented Oct 10, 2018

We talked about this in the UX call and think that we can change our model to always hide notifications after a certain timeout even if buttons are part of it. For very few notifications that are triggered from the core application we might add an internal option to not hide them after a timeout (e.g. our question to opt out of telemetry which is very important to be interacted with).

We currently do not anticipate to expose this internal option as API though.

@eamodio
Copy link
Contributor Author

eamodio commented Oct 10, 2018

While I think that would be a great default -- I think there are some messages that may require an interaction. For example, in the CodeStream extension, if we lose connection (after retrying a few times), we should a notification with a "Reconnect" button. If that notification was to auto-hide, the user could easily miss the fact that they are disconnected.

@bpasero
Copy link
Member

bpasero commented Oct 11, 2018

We do not want to introduce a setting for extensions because this will very likely cause all extension authors marking their notifications as "important" so that they persist. If you want the user to do something on connection lost, you will have to show the notification again after some time or turn the notification into a modal dialog.

@bpasero
Copy link
Member

bpasero commented Oct 11, 2018

@eamodio on a second thought, I wonder if we should just hide messages with severity INFO to begin with. For warning and error it seems OK to me to keep them around because of the potential higher impact.

@eamodio
Copy link
Contributor Author

eamodio commented Oct 11, 2018

I like that given the alternatives -- and by default. I would suggest that warnings should maybe also be included.

@bpasero
Copy link
Member

bpasero commented Oct 11, 2018

We think about only leaving ERROR open. Extensions can then decide to change to that severity if the notification should stay open and action is needed.

@bpasero bpasero added the plan-item VS Code - planned item for upcoming label Oct 11, 2018
@bpasero bpasero changed the title Allow to show notifications with buttons that hide after a timeout Hide notifications (info, warning) even when showing buttons Oct 12, 2018
@bpasero
Copy link
Member

bpasero commented Oct 12, 2018

There is now a new option sticky for notifications that can be used to mark a notification so that it does not close after a timeout. Notifications with buttons that have severity ERROR are sticky by default. I went over our usage of INotificationService.prompt() and added some sticky as I saw fit.

@Microsoft/vscode please check

  • if you want to set INotificationService.prompt() sticky in your use case if it is not sticky already
  • if you want to change notifications from your extension to severity ERROR to cause a sticky notification

There is currently no plans to expose the sticky flag to extensions.

@legomushroom
Copy link
Member

legomushroom commented Oct 17, 2018

@bpasero we hit some issues with the new behaviour, please see the end of the gif: https://monosnap.com/file/mrTsd1L6WoS9536So9euiwx1VVtZz7 Notice that the error notification is hidden automatically and immediately after a reload so the user is confused.

@bpasero
Copy link
Member

bpasero commented Oct 18, 2018

@legomushroom to clarify, what happens is that you reload the window and the error notification does not show up, it immediately shows in the status bar? If that is true please create a new issue with exact steps how to reproduce (be as verbose as possible).

@eamodio
Copy link
Contributor Author

eamodio commented Oct 23, 2018

@bpasero in today's insiders, it seems that all notifications (info or otherwise, with buttons or not) never hide

@bpasero
Copy link
Member

bpasero commented Oct 24, 2018

@eamodio I cannot reproduce, here is what I did

  • install "notification tester" extension
  • bring up a warning or info notification
  • wait some time

=> notification hides

What are your steps?

Note: when the window is not focused, we will not hide the notification assuming that you did not have a chance to get back to it (e.g. the window is not looked at)

@eamodio
Copy link
Contributor Author

eamodio commented Oct 24, 2018

Ah that explains it. The window wasn't focused. Is that new behavior or did I just never notice it before?

@bpasero
Copy link
Member

bpasero commented Oct 24, 2018

@eamodio yeah its new behaviour which I now just tweaked, it will no longer keep notifications around without buttons because I thought that its a bit too aggressive. However, for notifications with buttons, they will stay around if the window is not focused. Maybe you could test again from latest insiders tomorrow.

@eamodio
Copy link
Contributor Author

eamodio commented Oct 24, 2018

Sounds good -- will do!

@eamodio
Copy link
Contributor Author

eamodio commented Oct 25, 2018

@bpasero I tested this morning, but it seems that all notifications now go away if the window isn't focused.

@bpasero
Copy link
Member

bpasero commented Oct 25, 2018

@eamodio oh wow, good catch, now that was embarrassing (4c131e5)

@legomushroom
Copy link
Member

@bpasero that's right the error notification was never shown and immediately was hidden under the bell icon. I cannot reproduce it anymore though, so it might be fixed.

@bpasero
Copy link
Member

bpasero commented Oct 27, 2018

@legomushroom ok let me know when you see this again, ideally with some steps.

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api feature-request Request for new features or functionality on-testplan plan-item VS Code - planned item for upcoming ux User experience issues workbench-notifications Notification widget issues
Projects
None yet
Development

No branches or pull requests

3 participants