-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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] Fixed the issue with SearchBar focus and unfocus events. #28529
base: main
Are you sure you want to change the base?
Conversation
Hey there @Ahamed-Ali! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know you are not done yet, but we do lose the logic in the UpdateIsFocused
method in the manager. Maybe that method should be private protected
and we use that?
Yes, I have now utilized |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
// However, for AutoSuggestBox, when handling the GotFocus or LostFocus methods, tapping the AutoSuggestBox causes e.NewFocusedElement and e.OldFocusedElement to be a TextBox (which receives the focus). | ||
// As a result, when comparing the PlatformView with the appropriate handler in FocusManagerMapping, the condition is not satisfied, causing the focus and unfocus methods to not work correctly. | ||
// To address this, I have specifically handled the focus and unfocus events for AutoSuggestBox here. | ||
platformView.GotFocus += OnGotFocus; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good comment!
I was wondering if we could improve the logic in ViewHandler.Windows
to manage the focus, and if comparing also based on derived types would make sense, even though AutoSuggestBox
doesn't derive from Textbox
(it does from ItemsControl). So, I think changes are fine.
Root Cause of the issue
ViewHandler
.Windows,FocusManager
.GotFocus and LostFocus are handled for other controls. However, forAutoSuggestBox
, when handling the GotFocus or LostFocus methods, tapping the AutoSuggestBox causes e.NewFocusedElement and e.OldFocusedElement to be aTextBox
(which is the ControlTemplate of AutoSuggestBox and receives the focus). As a result, when comparing the PlatformView with the appropriate handler inFocusManagerMapping
, the condition is not satisfied, causing the focus and unfocus events to not work correctly.Description of Change
SearchBarHandler
.Windows.Regressed PR
FocusManager
(take 2) #24695Issues Fixed
Fixes #28419
Tested the behaviour in the following platforms
Screenshot
SearchBarIssueVideo.mp4
SearchBarFixVideo.mp4