Add support for renaming windows #9662

zadjii-msft · 2021-03-30T17:59:40Z

Summary of the Pull Request

This PR adds support for renaming windows.

It does so through two new actions:

renameWindow takes a name parameter, and attempts to set the window's name
to the provided name. This is useful if you always want to hit F3
and rename a window to "foo" (READ: probably not that useful)
openWindowRenamer is more interesting: it opens a TeachingTip with a
TextBox. When the user hits Ok, it'll request a rename for the provided
value. This lets the user pick a new name for the window at runtime.

In both cases, if there's already a window with that name, then the monarch will
reject the rename, and pop a Toast in the window informing the user that the
rename failed. Nifty!

References

Builds on the toasts from Add an action for identifying windows #9523
Scenario: Windows Terminal 2.0 Process Model Improvements #5000 - process model megathread

PR Checklist

Closes https://github.com/microsoft/terminal/projects/5#card-50771747
I work here
Tests addded (and pass with the help of Another fix for the localtests, March 2021 edition #9660)
Requires documentation to be updated

Detailed Description of the Pull Request / Additional comments

I'm sending this PR while finishing up the tests. I figured I'll have time to sneak them in before I get the necessary reviews.

PAIN: We can't immediately focus the textbox in the TeachingTip. It's
not technically focusable until it is opened. However, it doesn't
provide an even tto tell us when it is opened. That's tracked in
microsoft/microsoft-ui-xaml#1607. So for now, the user needs to
click on the text box manually.
We're also not using a ContentDialog for this, because in Xaml
Islands a text box in a ContentDialog won't recieve any keypresses.
Fun!

Validation Steps Performed

I've been playing with

        { "keys": "f1", "command": "identifyWindow" },
        { "keys": "f2", "command": "identifyWindows" },
        { "keys": "f3", "command": "openWindowRenamer" },
        { "keys": "f4", "command": { "action": "renameWindow", "name": "foo" } },
        { "keys": "f5", "command": { "action": "renameWindow", "name": "bar" } },

and they seem to work as expected

The history of this had gotten way, way too long. It included everything since I started working on this

WE LIVE IN A MAD WORLD. Teaching tips are exactly the UI we want, but they just don't fucking work man. WE want them to light dismiss, but if the window is inactive, and you have ILDE=true, then the tip immediately dismisses itself. So inactive windows need to not enable light dismiss. ALSO we need inactive windows to not focus something when the tip dismisses itself. Like, focus was _nowhere_ when we started. We need to toss the focus back to _nowhere_.

(cherry picked from commit aa51c07)

(cherry picked from commit fa26f7f)

…dows-3 # Conflicts: # src/cascadia/Remoting/FindTargetWindowArgs.h # src/cascadia/Remoting/ProposeCommandlineResult.h

add a test, and add liberal comments.

…Windows # Conflicts: # src/cascadia/Remoting/WindowManager.cpp # src/cascadia/TerminalApp/Resources/en-US/Resources.resw

…Windows

…windows # Conflicts: # src/cascadia/TerminalApp/Resources/en-US/Resources.resw # src/cascadia/TerminalApp/TerminalPage.cpp # src/cascadia/TerminalApp/TerminalPage.h # src/cascadia/WindowsTerminal/AppHost.cpp

…Windows

…indows

doc/cascadia/profiles.schema.json

src/cascadia/LocalTests_TerminalApp/TabTests.cpp

src/cascadia/Remoting/Monarch.cpp

src/cascadia/TerminalApp/Resources/en-US/Resources.resw

src/cascadia/TerminalApp/TerminalPage.xaml

tools/README.md

ghost · 2021-04-02T15:36:45Z

Hello @zadjii-msft!

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.

## Summary of the Pull Request In exactly the same fashion as the tab renamer, handle <kbd>Enter</kbd> for committing the rename, and <kbd>Escape</kbd> for dismissing the rename. ## References * Added in #9662 ## PR Checklist * [x] Closes #9720 * [x] I work here * [n/a] Tests added/passed * [n/a] Requires documentation to be updated ## Validation Steps Performed Played with it - this feels good.

## Summary of the Pull Request Huh, I guess I missed making the window renamer light-dismissable. This is a oneline fix for that. Light dismissing is treated as a _cancel_, not as a commit. ## References * Added in #9662 ## PR Checklist * [x] Closes #9718 * [x] I work here * [n/a] Tests added/passed * [n/a] Requires documentation to be updated ## Validation Steps Performed This feels right.

## Summary of the Pull Request Make sure that the window renamer and other toasts follow the requested app theme. We accomplish this by doing something similar to what we do with ContentDialogs. Since TeachingTips aren't in the same XAML root, we have to traverse the entire tree upwards setting RequestedTheme. If we don't, then we'll update the background color of the TeachingTip, but not the text inside it. ## References * Added in #9662 and #9523 ## PR Checklist * [x] Closes #9717 * [x] I work here * [n/a] Tests added/passed * [n/a] Requires documentation to be updated ## Validation Steps Performed Tested with system theme light & dark, and `theme` set to `light, dark, and unset, and verified that they worked as expected.

ghost · 2021-04-14T17:09:30Z

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

Handy links:

## Summary of the Pull Request Clearly, I didn't run these tests on my last commit where I made the toasts lazy-load. ## References * broken in in #9662 * ## PR Checklist * [x] Closes #9769 * [x] I work here * [x] Tests added/passed * [n/a] Requires documentation to be updated ## Detailed Description of the Pull Request / Additional comments For whatever reason, these tests are unhappy running back to back, but are just fine running isolated.

## Summary of the Pull Request Clearly, I didn't run these tests on my last commit where I made the toasts lazy-load. ## References * broken in in microsoft#9662 * ## PR Checklist * [x] Closes microsoft#9769 * [x] I work here * [x] Tests added/passed * [n/a] Requires documentation to be updated ## Detailed Description of the Pull Request / Additional comments For whatever reason, these tests are unhappy running back to back, but are just fine running isolated.

I added a `RenameSucceededText` property to the `TerminalPage` which returns the formatted message `Successfully renamed window to "{WindowNameForDisplay()}"` This _doesn't_ pop the dialog when you `wt -w foo` for the first time. Only _subsequent_ renames. ## References * Added in #9662 * Closes #9804

I added a `RenameSucceededText` property to the `TerminalPage` which returns the formatted message `Successfully renamed window to "{WindowNameForDisplay()}"` This _doesn't_ pop the dialog when you `wt -w foo` for the first time. Only _subsequent_ renames. ## References * Added in #9662 * Closes #9804 (cherry picked from commit aa54de1)

Does what it says on the tin. This is maximal BODGE. `TeachingTip` doesn't provide an `Opened` event. (microsoft/microsoft-ui-xaml#1607). But we want to focus the renamer text box when it's opened. We can't do that immediately, the TextBox technically isn't in the visual tree yet. We have to wait for it to get added some time after we call IsOpen. How do we do that reliably? Usually, for this kind of thing, we'd just use a one-off LayoutUpdated event, as a notification that the TextBox was added to the tree. HOWEVER: * The _first_ time this is fired, when the box is _first_ opened, yeeting focus doesn't work on the first LayoutUpdated. It does work on the second LayoutUpdated. Okay, so we'll wait for two LayoutUpdated events, and focus on the second. * On subsequent opens: We only ever get a single LayoutUpdated. Period. But, you can successfully focus it on that LayoutUpdated. So, we'll keep track of how many LayoutUpdated's we've _ever_ gotten. If we've had at least 2, then we can focus the text box. We're also not using a ContentDialog for this, because in Xaml Islands a text box in a ContentDialog won't receive _any_ keypresses. Fun! ## References * microsoft/microsoft-ui-xaml#1607 * microsoft/microsoft-ui-xaml#6910 * microsoft/microsoft-ui-xaml#3257 * #9662 ## PR Checklist * [x] Will close out #12021, but that's an a11y bug that needs secondary validation * [x] Closes #11322 * [x] I work here * [x] Tests added/passed * [n/a] Requires documentation to be updated ## Validation Steps Performed Tested manually

Does what it says on the tin. This is maximal BODGE. `TeachingTip` doesn't provide an `Opened` event. (microsoft/microsoft-ui-xaml#1607). But we want to focus the renamer text box when it's opened. We can't do that immediately, the TextBox technically isn't in the visual tree yet. We have to wait for it to get added some time after we call IsOpen. How do we do that reliably? Usually, for this kind of thing, we'd just use a one-off LayoutUpdated event, as a notification that the TextBox was added to the tree. HOWEVER: * The _first_ time this is fired, when the box is _first_ opened, yeeting focus doesn't work on the first LayoutUpdated. It does work on the second LayoutUpdated. Okay, so we'll wait for two LayoutUpdated events, and focus on the second. * On subsequent opens: We only ever get a single LayoutUpdated. Period. But, you can successfully focus it on that LayoutUpdated. So, we'll keep track of how many LayoutUpdated's we've _ever_ gotten. If we've had at least 2, then we can focus the text box. We're also not using a ContentDialog for this, because in Xaml Islands a text box in a ContentDialog won't receive _any_ keypresses. Fun! ## References * microsoft/microsoft-ui-xaml#1607 * microsoft/microsoft-ui-xaml#6910 * microsoft/microsoft-ui-xaml#3257 * #9662 ## PR Checklist * [x] Will close out #12021, but that's an a11y bug that needs secondary validation * [x] Closes #11322 * [x] I work here * [x] Tests added/passed * [n/a] Requires documentation to be updated ## Validation Steps Performed Tested manually (cherry picked from commit b57fe85) Service-Card-Id: 79978834 Service-Version: 1.13

Does what it says on the tin. This is maximal BODGE. `TeachingTip` doesn't provide an `Opened` event. (microsoft/microsoft-ui-xaml#1607). But we want to focus the renamer text box when it's opened. We can't do that immediately, the TextBox technically isn't in the visual tree yet. We have to wait for it to get added some time after we call IsOpen. How do we do that reliably? Usually, for this kind of thing, we'd just use a one-off LayoutUpdated event, as a notification that the TextBox was added to the tree. HOWEVER: * The _first_ time this is fired, when the box is _first_ opened, yeeting focus doesn't work on the first LayoutUpdated. It does work on the second LayoutUpdated. Okay, so we'll wait for two LayoutUpdated events, and focus on the second. * On subsequent opens: We only ever get a single LayoutUpdated. Period. But, you can successfully focus it on that LayoutUpdated. So, we'll keep track of how many LayoutUpdated's we've _ever_ gotten. If we've had at least 2, then we can focus the text box. We're also not using a ContentDialog for this, because in Xaml Islands a text box in a ContentDialog won't receive _any_ keypresses. Fun! ## References * microsoft/microsoft-ui-xaml#1607 * microsoft/microsoft-ui-xaml#6910 * microsoft/microsoft-ui-xaml#3257 * #9662 ## PR Checklist * [x] Will close out #12021, but that's an a11y bug that needs secondary validation * [x] Closes #11322 * [x] I work here * [x] Tests added/passed * [n/a] Requires documentation to be updated ## Validation Steps Performed Tested manually (cherry picked from commit b57fe85) Service-Card-Id: 79978833 Service-Version: 1.12

zadjii-msft added 30 commits February 25, 2021 10:39

Rebase all the changes on main

1c7da00

The history of this had gotten way, way too long. It included everything since I started working on this

fix a bug and fix the tests

97818c6

fix tests

2ec9415

Good ole Java

e13e1e7

finish that test

136ce6d

macros are life

ec97c43

THIS NEEDS TO GO TO THE PARENT

fa26f7f

Bind the labels to the actual TerminalPage object

001f545

bind the name, id down to the actual page

9270e0f

add an action for just identifying one single window

1f0dceb

Action boilerplate for both these actions

99c2976

bubble these up and down too

c09b5e9

THIS SHOULD GO TO THE PARENT BRANCH

aa51c07

Okay the window renamer is pretty slick

e818f8d

THIS SHOULD GO TO THE PARENT BRANCH

e238daa

(cherry picked from commit aa51c07)

THIS NEEDS TO GO TO THE PARENT

27d12a2

(cherry picked from commit fa26f7f)

yea you better believe the Toast just worked

9d6f47f

feedback from review

c202257

initial review feedback

eaaa204

Merge remote-tracking branch 'origin/main' into dev/migrie/f/name-win…

9f54229

…dows-3 # Conflicts: # src/cascadia/Remoting/FindTargetWindowArgs.h # src/cascadia/Remoting/ProposeCommandlineResult.h

Address PR concerns

bba09e3

add a test, and add liberal comments.

Merge branch 'dev/migrie/f/name-windows-3' into dev/migrie/f/identify…

0799a19

…Windows # Conflicts: # src/cascadia/Remoting/WindowManager.cpp # src/cascadia/TerminalApp/Resources/en-US/Resources.resw

That's better

1105328

Cleanup for review

97b4935

Merge branch 'dev/migrie/f/name-windows-3' into dev/migrie/f/identify…

765c969

…Windows

Merge branch 'dev/migrie/f/identifyWindows' into dev/migrie/f/rename-…

e8edc71

…windows # Conflicts: # src/cascadia/TerminalApp/Resources/en-US/Resources.resw # src/cascadia/TerminalApp/TerminalPage.cpp # src/cascadia/TerminalApp/TerminalPage.h # src/cascadia/WindowsTerminal/AppHost.cpp

Merge remote-tracking branch 'origin/main' into dev/migrie/f/identify…

b66503c

…Windows

Merge remote-tracking branch 'origin/main' into dev/migrie/f/identify…

b7703d2

…Windows

Merge remote-tracking branch 'origin/main' into dev/migrie/f/identify…

d035e50

…Windows

zadjii-msft added 2 commits April 1, 2021 06:34

Merge remote-tracking branch 'origin/main' into dev/migrie/f/rename-w…

e207f4e

…indows

nits for carlos

edff3bb

zadjii-msft assigned DHowett Apr 2, 2021

DHowett approved these changes Apr 2, 2021

View reviewed changes

zadjii-msft unassigned DHowett Apr 2, 2021

Nits for dustin

631d3ed

zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Apr 2, 2021

that's what I get for having two different git clones

312db84

ghost merged commit fb597ed into main Apr 2, 2021

ghost deleted the dev/migrie/f/rename-windows branch April 2, 2021 16:00

This was referenced Apr 6, 2021

Stylize the renamer Action button as a Accent button #9728

Merged

Manually handle Enter and Escape in the Window Renamer #9730

Merged

This was referenced Apr 6, 2021

Match the RequestedTheme of our TeachingTips to the set theme #9732

Merged

Add support for LightDismissing the renamer #9733

Merged

This was referenced Apr 9, 2021

Scenario: Windows Terminal 2.0 Process Model Improvements #5000

Closed

Add a success dialog to window renaming #9808

Merged

Fix the renamer tests #9815

Merged

zadjii-msft mentioned this pull request Mar 31, 2022

Auto-focus window renamer textbox on open #12798

Merged

4 tasks

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for renaming windows #9662

Add support for renaming windows #9662

zadjii-msft commented Mar 30, 2021 •

edited

Loading

ghost commented Apr 2, 2021

ghost commented Apr 14, 2021

Add support for renaming windows #9662

Add support for renaming windows #9662

Conversation

zadjii-msft commented Mar 30, 2021 • edited Loading

Summary of the Pull Request

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

ghost commented Apr 2, 2021

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 commented Apr 14, 2021

zadjii-msft commented Mar 30, 2021 •

edited

Loading

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.