Add an Opened event to TeachingTip#6910
Conversation
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
| 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; |
There was a problem hiding this comment.
Same here, you can find an example to mark a single property as preview here.
| }; | ||
|
|
||
|
|
||
| [MUX_PUBLIC] |
There was a problem hiding this comment.
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.
| [MUX_PUBLIC] | ||
| [default_interface] | ||
| [webhosthidden] | ||
| runtimeclass TeachingTipOpenedEventArgs |
There was a problem hiding this comment.
I can't think of any information that could be introduced here but maybe other folks know something?
|
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. |
|
@gabbybilka I think we should reconsider this PR, without Opened/Opening event is totally unacceptable. |
|
Fix merged on April 24. ADO PR commit: Commit dc7386d7 |
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
Openedevent toTeachingTip, like theClosedevent. The payload is aTeachingTipOpenedEventArgs, but I didn't know if there was anything reasonable to put in there.Arguably, there should also be an
Openingevent. 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
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
Openedevent and used that to manually focus aTextBoxin theTeachingTip. This proved that the event was fired after theTextBoxwas successfully added to the visual tree. Subsequent opening of theTeachingTipcontinued to successfully focus theTextBox.