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

Add a setting to flash the taskbar when the terminal emits BEL #8215

Merged
8 commits merged into from
Nov 18, 2020

Conversation

PankajBhojwani
Copy link
Contributor

@PankajBhojwani PankajBhojwani commented Nov 10, 2020

The terminal taskbar icon can now flash when the BEL sequence is
emitted, to let the user know something needs their attention.

The BellStyle setting can now be set to audible, visual or both or
none. When the pane receives a BEL event and the bellStyle includes
visual, we bubble the event up all the way to AppHost to handle
flashing the taskbar.

Closes #1608

@ghost ghost added Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Area-VT Virtual Terminal sequence support Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Conpty For console issues specifically related to conpty Product-Terminal The new Windows Terminal. labels Nov 10, 2020
@WSLUser
Copy link
Contributor

WSLUser commented Nov 10, 2020

Can you rename the setting of all to both instead please? I think that will be more user friendly.

@PankajBhojwani
Copy link
Contributor Author

Can you rename the setting of all to both instead please? I think that will be more user friendly.

The copy format setting also only has two flags (html/rtf) but we use all to set them both - I want to keep things consistent. Also, the idea is that there may potentially be more flags added in the future so all covers all bases nicely.

@zadjii-msft
Copy link
Member

Can you rename the setting of all to both instead please? I think that will be more user friendly.

What if we inevitably add a third value to that enum? Suddenly, both is confusing 😉

Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts:

  • Should this only flash the taskbar when the window isn't active? From the docs on FlashWindow:

    Typically, a window is flashed to inform the user that the window requires attention but that it does not currently have the keyboard focus.

  • Should we let unfocused panes/tabs flash the taskbar?
  • I'm pretty against having visual be the default, considering typing in bash and having it BEL at you when it fails to autocomplete, which now causes the window to flash too? IDK that just seems annoying, but that's my opinion

src/cascadia/TerminalSettingsModel/Profile.h Outdated Show resolved Hide resolved
@@ -800,7 +819,7 @@
},
"bellStyle": {
"default": "audible",
"description": "Controls what happens when the application emits a BEL character. When set to \"audible\", the Terminal will play a sound. When set to \"none\", nothing will happen.",
"description": "Controls what happens when the application emits a BEL character. When set to \"all\", the Terminal will play a sound and flash the taskbar icon. An array of specific behaviors can also be used. Supported array values include `audible` and `visual`. When set to \"none\", nothing will happen.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For whatever reason, my head is having a hard time parsing this description. I don't know how to do it better, but this feels weird to me. Summoning @cinnamon-msft

@miniksa
Copy link
Member

miniksa commented Nov 10, 2020

The SND_SENTRY flag passed to PlaySound in the original PR should be blinking the window/taskbar icon when a sound is played already, presuming the user has reported that they want audio alerts to be visual in the OS Settings:

image

So a few things:

  • I'm not sure this feature is strictly necessary as this provision to turn audio into visual queues is just a part of the Windows OS.
  • I'm not opposed to adding an app-specific overriding setting to make us blink ourselves should someone be focused on just visual cues in Terminal and not across their whole OS.
  • I don't think it should be the default either way. People with an accessibility issue are likely already familiar with the broad range of Ease Of Access options in Windows and can turn it on that way. People who want it to be specific to our app could turn it on.

@PankajBhojwani
Copy link
Contributor Author

Noted the comments, the default is only audible now and we no longer flash the taskbar icon if the window is focused.

I think we should still flash the taskbar even if it wasn't the last active control that sent the bell though - its the same with the audible bell currently, we will play the sound even if it wasn't the last active control that sent it.

@PankajBhojwani
Copy link
Contributor Author

The SND_SENTRY flag passed to PlaySound in the original PR should be blinking the window/taskbar icon when a sound is played already, presuming the user has reported that they want audio alerts to be visual in the OS Settings:

This does not seem to work for me (tried setting this and playing BEL in terminal - no flash in taskbar). Can someone else try as well just in case?

@j4james
Copy link
Collaborator

j4james commented Nov 11, 2020

This does not seem to work for me (tried setting this and playing BEL in terminal - no flash in taskbar).

It doesn't flash the taskbar for me, but I didn't think it was meant to. It just flashes whatever part of the screen you've chosen in the settings. It's either the title bar (or in the case of a UWP app, the window border), the whole of the active window, or the whole screen. You can't miss it, even if the app is in the background, because it effects the focused window, not the window that produced the sound.

@miniksa
Copy link
Member

miniksa commented Nov 17, 2020

OK well as not default, flashing our own HWND/taskbar icon, and selectively depending on how focused it is sounds better to me.

@miniksa
Copy link
Member

miniksa commented Nov 17, 2020

Also to be clear, this only flashes the taskbar icon, not the window frame? And only when it's not otherwise in focus? Can you show a gif of it working?

@PankajBhojwani
Copy link
Contributor Author

Also to be clear, this only flashes the taskbar icon, not the window frame? And only when it's not otherwise in focus? Can you show a gif of it working?

It only flashes the taskbar icon when the terminal is not focused, yes. Gif added!

Copy link
Member

@miniksa miniksa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I'm good with this.

src/cascadia/TerminalApp/Pane.cpp Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/Pane.cpp Outdated Show resolved Hide resolved
@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Nov 17, 2020
@ghost ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Nov 18, 2020
Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love it!

@DHowett
Copy link
Member

DHowett commented Nov 18, 2020

moved out of body

flash

@DHowett
Copy link
Member

DHowett commented Nov 18, 2020

Just gotta give it a code format!

@DHowett DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Nov 18, 2020
@ghost
Copy link

ghost commented Nov 18, 2020

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@DHowett
Copy link
Member

DHowett commented Nov 18, 2020

Needs conflict resolve

@UweKeim
Copy link

UweKeim commented May 22, 2022

Any plan to make this change ever available, at least in the Preview of Windows Terminal?

@DHowett
Copy link
Member

DHowett commented May 22, 2022

This feature has been available for two years.

@UweKeim
Copy link

UweKeim commented May 22, 2022

Thanks, @DHowett

I've just tried the sequence of the above GIF in a PowerShell:

sleep 3;"^G"

Then quickly minimized the window and nothing happens. I.e. nothing flashing.

I'm using:

  • German Windows 11, 64-bit
  • PowerShell-Tab, run as Administrator
  • Terminal Preview version 1.13.10984.0

Probably I'm just doing something stupid and/or missing some plain obvious things.


Trying it from a .NET console application with:

Console.Write("\u001b[G");

also does not flash the task bar icon.


OK, I think I get it now:

The "BellStyle" setting was not set explicitly, thus resulting in only a sound (which was not hearable since I turned of all system sounds).

After configuring the CMD type to this:

image

My C# console application was able to make the task bar flash/be lightened when using this:

Console.Write("\a");

Resulting in this effect:

image

I'm still unable to do the same for PowerShell and sleep3;"^G" but since I do need it for CMD only, not PowerShell, this is sufficient for me.


Update:

After reading Dustin's reply, this one worked correctly in PowerShell:

sleep 3;[char]0x7

@DHowett
Copy link
Member

DHowett commented May 22, 2022

Glad you got it working!

So, couple things. \x1b[G is ESC [ G (or CSI G) which definitely isn't the same as ^G. In addition, PowerShell does not treat ^ as introducing a literal control character. If that's what you're entering, you are asking it to print the literal characters Caret and G. You might have better luck with `a or even [char]0x7.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Area-VT Virtual Terminal sequence support AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. Product-Conpty For console issues specifically related to conpty Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make terminal beeps highlight Windows Terminal in the taskbar, to get user attention when backgrounded
7 participants