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

Dismiss Terminal-by-default banner on handoff #13344

Merged
1 commit merged into from
Jun 21, 2022

Conversation

lhecker
Copy link
Member

@lhecker lhecker commented Jun 21, 2022

It's not useful to notify users that WT can be made the default if it's already
clearly being used for handoff. This commit will suppresses the banner then.

PR Checklist

Validation Steps Performed

  • Modify TerminalPage::ShowSetAsDefaultInfoBar to not check for
    CascadiaSettings::IsDefaultTerminalSet()
  • Set Terminal Dev as the default
  • Set incoming connections to open in the latest Terminal window
  • Delete state.json after every test below
  • Launching Terminal Dev shows the banner ✅
    Launching cmd.exe dismisses the banner in the current Terminal ✅
  • Launching cmd.exe launches Terminal Dev without banner ✅

@ghost ghost added Area-DefApp Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-0 Bugs that we consider release-blocking/recall-class (P0) Product-Terminal The new Windows Terminal. Severity-Blocking We won't ship a release like this! No-siree. labels Jun 21, 2022
@DHowett
Copy link
Member

DHowett commented Jun 21, 2022

Does this work if the first tab in a session is a handoff?

@lhecker
Copy link
Member Author

lhecker commented Jun 21, 2022

Does this work if the first tab in a session is a handoff?

Yep! That's what I meant to say with the last bullet point in my validation steps.

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. It's so clever.

@lhecker lhecker added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jun 21, 2022
@ghost
Copy link

ghost commented Jun 21, 2022

Hello @lhecker!

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.

@ghost ghost merged commit 24a53d4 into main Jun 21, 2022
@ghost ghost deleted the dev/lhecker/13314-wt-default-banner branch June 21, 2022 23:02
DHowett pushed a commit that referenced this pull request Jun 30, 2022
It's not useful to notify users that WT can be made the default if it's already
clearly being used for handoff. This commit will suppresses the banner then.

## PR Checklist
* [x] Closes #13314
* [x] I work here

## Validation Steps Performed
* Modify `TerminalPage::ShowSetAsDefaultInfoBar` to not check for
  `CascadiaSettings::IsDefaultTerminalSet()`
* Set Terminal Dev as the default
* Set incoming connections to open in the latest Terminal window
* Delete `state.json` after every test below
* Launching Terminal Dev shows the banner ✅
  Launching `cmd.exe` dismisses the banner in the current Terminal ✅
* Launching `cmd.exe` launches Terminal Dev without banner ✅

(cherry picked from commit 24a53d4)
Service-Card-Id: 83434412
Service-Version: 1.14
@ghost
Copy link

ghost commented Jul 6, 2022

🎉Windows Terminal v1.14.186 has been released which incorporates this pull request.:tada:

Handy links:

@ghost
Copy link

ghost commented Jul 6, 2022

🎉Windows Terminal Preview v1.15.186 has been released which incorporates this pull request.:tada:

Handy links:

Copy link

@manxontanx777 manxontanx777 left a comment

Choose a reason for hiding this comment

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

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-DefApp AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-0 Bugs that we consider release-blocking/recall-class (P0) Product-Terminal The new Windows Terminal. Severity-Blocking We won't ship a release like this! No-siree.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DxD: Terminal tells you that it can be set as default even when it was DxD'd
4 participants