Add a information popup about default terminals #11397
Conversation
lhecker
force-pushed
the
dev/lhecker/default-terminal-popup
branch
from
85f5639
to
752ab6c
Compare
| } | ||
|
|
||
| dismissedMessages.Append(message); | ||
| ApplicationState::SharedInstance().DismissedMessages(dismissedMessages); | ||
| applicationState.DismissedMessages(std::move(messages)); |
There was a problem hiding this comment.
FYI This fixes a pretty ugly programming error: ApplicationState is supposed to be a thread-safe class.
Calling Append on the returned vector however isn't thread-safe, so I rewrote this.
| <data name="SetAsDefaultInfoBar.Message" xml:space="preserve"> | ||
| <value>Windows Terminal can be set as your default terminal application in your settings.</value> | ||
| </data> |
There was a problem hiding this comment.
(Offline discussion w/ @cinnamon-msft ) "Looks good"
There was a problem hiding this comment.
I don't love the repeated "your". "the settings" is better, no?
There was a problem hiding this comment.
I changed it to
Windows Terminal can be set as the default terminal application in your settings.
| @@ -287,6 +287,11 @@ namespace winrt::TerminalApp::implementation | |||
| _defaultPointerCursor = CoreWindow::GetForCurrentThread().PointerCursor(); | |||
| } | |||
| CATCH_LOG(); | |||
|
|
|||
| if (CascadiaSettings::IsDefaultTerminalAvailable()) | |||
There was a problem hiding this comment.
I wanted to add a check here that only makes it show up if conhost is the current default:
if (CascadiaSettings::IsDefaultTerminalAvailable() && !_settings.CurrentDefaultTerminal())But unfortunately the Name() returned by CurrentDefaultTerminal() is always a localized name of the application including conhost. So I'd have to add some new API functions to make this work.
But before I do that I wanted to ask if anyone else would like to see that...
There was a problem hiding this comment.
we will probably want to hide this infobar if we're not running packaged, also. which is silly.
There was a problem hiding this comment.
Shouldn't we just change IsDefaultTerminalAvailable to detect unpackaged activation then?
zadjii-msft
added
zPreview-Service-Queued-1.13
|
Mind sharing a screenshot? 😄 |
|
@DHowett Is it fine as just a regular comment here? |
|
Love it! In fact, I prefer screenshots to be outside of the commit body. Github does not render commit bodies in markdown, so... they look messy and the user has to dig out a URL and open it themselves just to see it. |
|
As a big trick, you could expose the |
|
@DHowett I've updated the PR with your suggestions. Can you check the latest commit again if you get a chance? |
DHowett
added
the
AutoMerge
|
Hello @DHowett! Because this pull request has the 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 (
|
ghost
merged commit
DHowett
removed
the
zPreview-Service-Queued-1.13
PankajBhojwani
removed
the
zStable-Service-Queued-1.12
This commit adds a simple information popup about default terminals, guiding first-time Windows 11 users into changing the default terminal. * Info bar pops up on Windows 11 ✔️ * Info bar can be dismissed persistently ✔️ (cherry picked from commit 2b1468e)
|
🎉 Handy links: |
|
🎉 Handy links: |
This commit adds a simple information popup about default terminals,
guiding first-time Windows 11 users into changing the default terminal.
Validation Steps Performed