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

Focus is completely lost with Teaching tip #3257

Closed
nilaysanghvi opened this issue Sep 8, 2020 · 12 comments
Closed

Focus is completely lost with Teaching tip #3257

nilaysanghvi opened this issue Sep 8, 2020 · 12 comments
Labels
area-TeachingTip bug Something isn't working closed-Fixed Described behavior has been fixed. help wanted Issue ideal for external contributors team-Controls Issue for the Controls team
Milestone

Comments

@nilaysanghvi
Copy link

I am using the Teaching Tip control in a UWP app. However, I am not able to get focus on the teaching tip. As suggested in the documentation, pressing F6 does not help. The focus is completely lost and I am not able to navigate using the tab key even in the rest of the app. The only option is to press the Esc key which dismisses the teaching tip.

The reason I suspect this happens is because the "IsLightDismissEnabled" property is set to True, hence there is no cross button on the top right corner. Moreover, I do not use the "ActionButton" or the "CloseButton". If I have any of those, then the focus gets set on pressing F6. I have my own custom content in the Teaching tip -

<muxc:TeachingTip
            x:Name="TestTip"
            Title="Sample Title"
            Target="{x:Bind TaskEntryBar.PlusIcon}"
            PreferredPlacement="TopRight"
            IsLightDismissEnabled="True">
            <Grid>
                <Grid.ColumnDefinitions>
                    <ColumnDefinition Width="*"/>
                    <ColumnDefinition Width="*"/>
                </Grid.ColumnDefinitions>
                <TextBlock Grid.Column="0" Text="2 of 2" FontSize="14"/>
                <Button
                    x:Name="TestButton"
                    Grid.Column="1"
                    FontSize="14"
                    Margin="6,0"
                    Content="Got it"
                    Click="AddTaskTip_Click" />
            </Grid>
</muxc:TeachingTip>

image

As suggested in one of the other issues, I tried to set the focus ( in the code behind file ) on my custom button but that also did not work -

TestTip.IsOpen = true;
TestButton.Focus(FocusState.Programmatic);

And using the visual tree helper, I got the popup, but Focus() method is not available on a popup.

var popups = VisualTreeHelper.GetOpenPopups(Window.Current);
foreach (var popup in popups)
{
    popup.Focus(FocusState.Programmatic);
}

Please suggest what can I do to get focus on the teaching tip in case of only custom content and with IsLightDismissEnabled set to true.

Windows 10 Version: Build 19041.450
Device Form Factor: Desktop

@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label Sep 8, 2020
@StephenLPeters
Copy link
Contributor

Seems like a few issues. First is an oversight in the F6 logic, that at tip could have no close button, we should fix this so it works with your content. Second, the isOpen property on teaching tip is async, which means you cannot set it to true and immediately call focus on your content. You should be able to do this if you attach to the TestButton.Loaded event and call focus in there. But perhaps we should also add an Opened event to teaching tip, given that IsOpen is async.

@StephenLPeters StephenLPeters added team-Controls Issue for the Controls team help wanted Issue ideal for external contributors and removed needs-triage Issue needs to be triaged by the area owners labels Sep 8, 2020
@StephenLPeters
Copy link
Contributor

Help wanted for the F6 portion. @SavoySchuler fyi about the opened event.

@nilaysanghvi
Copy link
Author

@StephenLPeters Thanks, the TestButton.Loaded solution worked. However, the problem with that solution is that the narrator announcement is not made due to the Focus shifting to the button immediately.

@Felix-Dev
Copy link
Contributor

I would like to take a look at the Focus keyboard shortcut problem (F6) if that's fine with everybody.

@nilaysanghvi

However, I am not able to get focus on the teaching tip. As suggested in the documentation, pressing F6 does not help

Where exactly did you find that in the documentation I checked the TeachingTip main documentation page but found no mention of F6 setting focus into the TeachingTip. I only found such a note here. If this info is not present in the docs, then it should be added.

@nilaysanghvi
Copy link
Author

@Felix-Dev I'm sorry it is not in the documentation but the spec.
Also if you switch on the narrator service then it announces that you need to press F6.

@Felix-Dev
Copy link
Contributor

Definitely should be added to the TeachingTip's main documentation page then. Can open a PR for that later today.

@nilaysanghvi
Copy link
Author

@Felix-Dev there is another issue regarding the narrator not announcing the subtitle? Should I open a new issue for that?

@Felix-Dev
Copy link
Contributor

@nilaysanghvi Yep :)

@Felix-Dev
Copy link
Contributor

Since my PR for this issue adds an interaction test I am currently more or less blocked on issue #3349.

@joshkoon
Copy link
Member

@StephenLPeters has there been any progress on this? Is this blocked?

@joshkoon
Copy link
Member

@StephenLPeters is there any traction we can get on this to get it approved/merged?

ghost pushed a commit to microsoft/terminal that referenced this issue Mar 31, 2022
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
DHowett pushed a commit to microsoft/terminal that referenced this issue Mar 31, 2022
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
DHowett pushed a commit to microsoft/terminal that referenced this issue Mar 31, 2022
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
@ghost ghost removed the working on it label May 19, 2022
@GuildOfCalamity
Copy link

GuildOfCalamity commented May 4, 2023

This is especially problematic in a WinUI app for the TextBox controls since is causes the user to have to click the TextBox twice.

The only way I know to get around this is to setup a PropertyChangedCallback for the teaching tip...

teachingTip.RegisterPropertyChangedCallback(TeachingTip.IsOpenProperty, IsTipOpenChanged);

Then inside the callback shift the focus back to the TextBox.

@duncanmacmichael duncanmacmichael added the bug Something isn't working label Nov 3, 2023
@llongley llongley closed this as completed Feb 6, 2024
@llongley llongley added the closed-Fixed Described behavior has been fixed. label Feb 6, 2024
@bpulliam bpulliam added this to the WinAppSDK 1.5 milestone Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-TeachingTip bug Something isn't working closed-Fixed Described behavior has been fixed. help wanted Issue ideal for external contributors team-Controls Issue for the Controls team
Projects
None yet
9 participants