Add an Opened event to TeachingTip #6910

zadjii-msft · 2022-03-31T11:55:42Z

Filed to prompt discussion. I understand API changes are temporarily locked down. This is to prompt discussion for inclusion in MUX 2.8+ (and also make sure I don't forget to actually submit the code as a PR)

Description

Adds an Opened event to TeachingTip, like the Closed event. The payload is a TeachingTipOpenedEventArgs, but I didn't know if there was anything reasonable to put in there.

Arguably, there should also be an Opening event. I was trying to scope the changes to the smallest ones needed for the Terminal, but since we'll need to pursue another solution, we can likely increase the scope of this change if need be.

Motivation and Context

Relevant to Proposal: Add a Opened event in TeachingTip similar to Closed event #1607.
Downstream bug: [Terminal> Rename Window]: Keyboard users are not able to access 'Window 1' pop-up when they open it using defined shortcut key. terminal#12021 - P1 a11y bug that prompted this
See also Focus is completely lost with Teaching tip #3257 and TeachingTip: Pressing F6 now sets focus on custom content of light-dismissable tip #3545 - what could have likely been another acceptable workaround for us.

How Has This Been Tested?

Tested only manually, by building a nuget package and ingesting that into the Terminal. Terminal side, I added a handler for the Opened event and used that to manually focus a TextBox in the TeachingTip. This proved that the event was fired after the TextBox was successfully added to the visual tree. Subsequent opening of the TeachingTip continued to successfully focus the TextBox.

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

marcelwgn · 2022-04-01T17:56:26Z

dev/TeachingTip/TeachingTip.idl

@@ -128,6 +136,7 @@ unsealed runtimeclass TeachingTip : Windows.UI.Xaml.Controls.ContentControl
    event Windows.Foundation.TypedEventHandler<TeachingTip, Object> CloseButtonClick;
    event Windows.Foundation.TypedEventHandler<TeachingTip, TeachingTipClosingEventArgs> Closing;
    event Windows.Foundation.TypedEventHandler<TeachingTip, TeachingTipClosedEventArgs> Closed;
+    event Windows.Foundation.TypedEventHandler<TeachingTip, TeachingTipOpenedEventArgs> Opened;


Same here, you can find an example to mark a single property as preview here.

marcelwgn · 2022-04-01T17:56:52Z

dev/TeachingTip/TeachingTip.idl

@@ -64,6 +64,14 @@ runtimeclass TeachingTipClosingEventArgs
    Windows.Foundation.Deferral GetDeferral();
 };

+
+[MUX_PUBLIC]


I think this would need to be marked as preview API first (@MikeHillberg correct me if I'm wrong). You can find an example of this here.

marcelwgn · 2022-04-01T17:57:22Z

dev/TeachingTip/TeachingTip.idl

+[MUX_PUBLIC]
+[default_interface]
+[webhosthidden]
+runtimeclass TeachingTipOpenedEventArgs


I can't think of any information that could be introduced here but maybe other folks know something?

gabbybilka · 2022-11-21T19:43:02Z

Thank you for submitting this PR, the current focus of the team is to assist our users in moving from WinUI 2 to WinUI 3. We are closing this PR as it does not address resolving a critical issue for WinUI 2 or WinUI 3.

zhuxb711 · 2023-07-12T09:55:38Z

@gabbybilka I think we should reconsider this PR, without Opened/Opening event is totally unacceptable.

zadjii-msft added 3 commits March 30, 2022 06:57

It can't be this easy, right?

737c146

okay I knew I needed this

c96e21f

I needed these too

ec93969

ghost added the needs-triage Issue needs to be triaged by the area owners label Mar 31, 2022

zadjii-msft mentioned this pull request Mar 31, 2022

Auto-focus window renamer textbox on open microsoft/terminal#12798

Merged

4 tasks

marcelwgn reviewed Apr 1, 2022

View reviewed changes

ojhad added area-TeachingTip team-Controls Issue for the Controls team and removed needs-triage Issue needs to be triaged by the area owners labels Apr 6, 2022

gabbybilka closed this Nov 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add an Opened event to TeachingTip #6910

Add an Opened event to TeachingTip #6910

zadjii-msft commented Mar 31, 2022

marcelwgn Apr 1, 2022

marcelwgn Apr 1, 2022

marcelwgn Apr 1, 2022

gabbybilka commented Nov 21, 2022

zhuxb711 commented Jul 12, 2023

Add an Opened event to TeachingTip #6910

Add an Opened event to TeachingTip #6910

Conversation

zadjii-msft commented Mar 31, 2022

Description

Motivation and Context

How Has This Been Tested?

marcelwgn Apr 1, 2022

Choose a reason for hiding this comment

marcelwgn Apr 1, 2022

Choose a reason for hiding this comment

marcelwgn Apr 1, 2022

Choose a reason for hiding this comment

gabbybilka commented Nov 21, 2022

zhuxb711 commented Jul 12, 2023