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

Do not hide notifications without prompt when window has no focus #80285

Closed
SteveBenz opened this issue Sep 3, 2019 · 7 comments · Fixed by microsoft/azuredatastudio#7206
Closed
Assignees
Labels
feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded workbench-notifications Notification widget issues

Comments

@SteveBenz
Copy link

Issue Type: Bug

If an extension posts a notification while VSCode is, say, minimized, the user has to actively search for the notification in order to see it.

While the 15-second timer on notifications is generally a good thing, the 15 seconds should only tick while VSCode is in-focus.

Here are some examples of cases where notifications might pop up while VSCode is not focused:

  1. An extension that's monitoring for changes to the branch that the user is coding on.
  2. An extension that monitors for work items or pull requests assigned to the user.
  3. An extension that checks for updates for itself.

VS Code version: Code 1.37.1 (f06011a, 2019-08-15T16:17:55.855Z)
OS version: Windows_NT x64 10.0.18975

System Info
Item Value
CPUs Intel(R) Xeon(R) CPU E5-1650 v3 @ 3.50GHz (12 x 3492)
GPU Status 2d_canvas: enabled
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
native_gpu_memory_buffers: disabled_software
oop_rasterization: disabled_off
protected_video_decode: unavailable_off
rasterization: enabled
skia_deferred_display_list: disabled_off
skia_renderer: disabled_off
surface_synchronization: enabled_on
video_decode: enabled
viz_display_compositor: disabled_off
webgl: enabled
webgl2: enabled
Load (avg) undefined
Memory (System) 15.92GB (10.38GB free)
Process Argv .
Screen Reader no
VM 0%
Extensions (4)
Extension Author (truncated) Version
tslint eg2 1.0.44
vscode-nmake-tools Mic 2.0.190702001
cpptools ms- 0.25.1
quokka-vscode Wal 1.0.240
@vscodebot
Copy link

vscodebot bot commented Sep 3, 2019

(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 workbench-notifications Notification widget issues labels Sep 4, 2019
@bpasero bpasero removed their assignment Sep 4, 2019
@bpasero bpasero changed the title Notification Timeout should only tick if VSCode is in-focus Notifications should not hide when VSCode window is minimized Sep 4, 2019
@bpasero bpasero self-assigned this Sep 4, 2019
@bpasero bpasero added info-needed Issue requires more information from poster and removed feature-request Request for new features or functionality workbench-notifications Notification widget issues labels Sep 4, 2019
@bpasero
Copy link
Member

bpasero commented Sep 4, 2019

Actually that should be the case given this line:

if ((item.sticky || item.hasPrompt()) && !this.windowService.hasFocus) {

Can you demonstrate where this does not work? A notification should not start to hide automatically if the window does not have focus.

@SteveBenz
Copy link
Author

Thanks for having a look @bpasero ! That line might save us if the notification were sticky or prompting. For for stuff like what I'm talking about, you wouldn't want a notification to be that heavyweight - you'd just want it to tell you what it's got to tell you and disappear. For example, you go to a meeting and come back to your desk and re-open VSCode - you want that notification that the branch head moved half an hour ago to appear, get its 15 seconds of screen-time, and disappear.

To be sure I'm clear, here are some repro-steps:

  1. Create a trivial extension with a single command, whose implementation is:

    setTimeout(() => vscode.window.showInformationMessage("Zow!"), 5000);

  2. Run the command - observe that it pops up the notification after 5 seconds, and the notification dies after 15 seconds. All is well.

  3. Run the command, and immediately minimize VSCode, get coffee. When you come back and re-open VSCode - observe that there's no notification to be seen, but if you dig into your notifications, you can see it.

  4. Run the command, alt-tab to a web browser. Again the notification pops up, and pops down after 15 seconds even though VSCode was not in focus.

@bpasero
Copy link
Member

bpasero commented Sep 5, 2019

Got it. It looks like we did an explicit decision (in 29ab698) to only keep notifications around that ask the user for input, even if the window does not have focus. I think we can leave this open as feature request, but I am not sure if we want to change this in the near term. We are sensitive to not showing too many notifications at once. For that purpose we have the notification center to always access notifications as needed.

@bpasero bpasero changed the title Notifications should not hide when VSCode window is minimized Do not hide notifications without prompt when window has no focus Sep 5, 2019
@bpasero bpasero added feature-request Request for new features or functionality workbench-notifications Notification widget issues and removed info-needed Issue requires more information from poster labels Sep 5, 2019
@bpasero bpasero removed their assignment Sep 5, 2019
@SteveBenz
Copy link
Author

I'm not sure I buy that. We used notifications as a way to inform people of available updates to our extension for a while. I believe we made the correct decision to only inform people, and not, say, add a button to require interaction. It's just an update, the user can make their own decision about it. We just wanted 15-seconds of face-time with the end-user to tell them the option was available to them.

Our telemetry showed that we didn't get that 15-seconds. Most of the time when a new update was detected, VSCode wasn't visible and thus the user never saw it. The idea that users can go sniffing in the notifications window to find notifications they missed is a non-starter. Sorry, users just never do it. They open the notification panel when they say "Hey wait, I saw a notification about that a minute ago!" and that's the only time they do it.

Likewise, for the scenarios I mention, a push to the user's branch, a new pull request to review, etc. Are all things that the user just needs to be informed of and shouldn't demand anything more than a glance. Hence, we haven't put any buttons. The behavior it does now when the app is open is perfect - it shows it, the user can read it or not, act on it or not, and in any case it's gone in 15 seconds, is really what we want. But if the app isn't open, I might as well not even bother to pop the notification, because I can know for a certainty it'll not get read.

So, I'm an extension author. What would you have me do?

  1. I could find some way of showing notifications entirely outside of VSCode... Kinda dumb and awkward.
  2. I could game the system - I could, in my extension, detect if VSCode was the active window or not. If it's focused, I would pop my notifications the way I think I should (without any actions) and if it's not in focus, kludge up an action for the user to click on.

I'm leaning towards number 2 there, and I hope you can imagine why. You're encouraging extension developers to do stuff that defeats what you were trying to achieve by the code change you reference. I dare say if anything, the code change made the problem worse because it encourages extension developers to do the wrong thing.

Notifications aren't fundamentally bad - but it's certainly the case that over-eager extension developers can make them bad. I respect that you're trying to put up some rails here to ensure that bad extensions don't make VSCode look bad, but there's a limit to what you can do here. It really needs to be up to the extension to behave well here.

@bpasero bpasero closed this as completed in a269614 Sep 6, 2019
@bpasero bpasero added this to the September 2019 milestone Sep 6, 2019
@bpasero bpasero self-assigned this Sep 6, 2019
@bpasero bpasero added the verification-needed Verification of issue is requested label Sep 6, 2019
@bpasero
Copy link
Member

bpasero commented Sep 6, 2019

Fair enough. I have pushed a change to treat all notifications equally and not hide them if the window does not have focus. I appreciate your concern that extensions should not trick our system by adding buttons and I fully agree on that.

I still think we have a fair amount of work to do to allow users to control notifications more fine grained, especially allow them to disable certain notifications from extensions.

@bpasero
Copy link
Member

bpasero commented Sep 6, 2019

The more I think about it, the more I feel it would be better to keep notifications around only when the window is not visible to the user. I fear Electron does not provide this API, thus filed electron/electron#20145

@alexr00 alexr00 added the verified Verification succeeded label Oct 2, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Oct 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded workbench-notifications Notification widget issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants