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

[Windows] Register focus events for controls that can receive focus #23885

Conversation

MartyIX
Copy link
Contributor

@MartyIX MartyIX commented Jul 29, 2024

Description of Change

I noticed that MAUI registers GotFocus and LostFocus events for all platform views on Windows.

However, for MAUI <Label>s, I believe it's not really necessary.

It seems that TextBlock (WinUI 3 type that MAUI's Label is mapped to) can be focusable as it contains this method:

// Focus and selection handling related overrides and helpers.
bool IsFocusable() final
{
    return IsActive() &&
        IsVisible() &&
        IsEnabled() && (m_isTextSelectionEnabled || IsTabStop()) &&
        AreAllAncestorsVisible();
}

However, support for TabStops was removed in #1777 (see also #1646 for reasoning) and MAUI does not support text selection for Labels.

Performance impact

image

-> ~ 96% improvement

Issues Fixed

Contributes to #21787

Alternative approaches

There is FocusManager in WinUI and it has similar events ( https://learn.microsoft.com/en-us/windows/windows-app-sdk/api/winrt/microsoft.ui.xaml.input.focusmanager?view=windows-app-sdk-1.5#events), so instead of registering events on all views one by one (and unregistering for that matter). One can probably subscribe FocusManager events once and just check if the platform view corresponds to one in a dictionary and do the action.

@MartyIX MartyIX requested a review from a team as a code owner July 29, 2024 10:13
@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Jul 29, 2024
@MartyIX MartyIX requested review from Foda and jonathanpeppers and removed request for Eilon and StephaneDelcroix July 29, 2024 10:20
@MartyIX MartyIX added platform/windows 🪟 t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.) (sub: perf) labels Jul 29, 2024
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

@PureWeen who would know the most about accessibility here? Is there some way focus is used for screen-readers?

@jonathanpeppers
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@samhouts samhouts added the area-core-hosting Extensions / Hosting / AppBuilder / Startup label Jul 31, 2024
@MartyIX
Copy link
Contributor Author

MartyIX commented Aug 21, 2024

Replaced with #24355

@MartyIX MartyIX closed this Aug 21, 2024
@MartyIX MartyIX deleted the feature/2024-07-29-Windows-labels-no-focus-events branch August 21, 2024 14:39
@github-actions github-actions bot locked and limited conversation to collaborators Sep 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-core-hosting Extensions / Hosting / AppBuilder / Startup community ✨ Community Contribution platform/windows 🪟 t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.) (sub: perf)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants