#7956 [Shell] Unable to focus and unfocus search handler for Android #14375

germancoines · 2021-06-29T01:07:25Z

Description of Change

This pull request modifies the ShellSearchView class in order to actually trigger the SearchHandler OnFocused() and OnUnfocused() overridable methods as well as the Focused and Unfocused EventHandlers (disregarding if they were declared on XAML or in the CodeBehind) whenever the TextBox within the SearchHandler changes its focus state.
It also adds the Github Issues tests 7956 and 7956_1, in order to properly test such triggering of events.

The original commit that introduced the issue was e231882 . After having analised the code, my guess is that such commit intended to implement most of the features already existing within the VisualElement class by replicating parts of its code within the SearchHandler class (rather than by extending it, as many other controls do either directly or indirectly).

The mentioned commit ammended existing test cases based on the SearchHandlerPage and StorePages in order to test properties like BackgroundColor, CancelButtonColor, TextColor, PlaceholderColor, HorizontalTextAlignment, Keyboard, FontFamily, etc. but in none of them it tests that the Focused or Unfocused event handlers trigger.

Issues Resolved

fixes [Shell] Unable to focus and unfocus search handler for Android #7956

API Changes

Added:

protected virtual void ShellSearchView.OnSearchHandlerFocusChange(object sender, FocusChangeEventArgs e);

Changed:

protected virtual void ShellSearchView.LoadView(SearchHandler searchHandler);

Removed:
None.

Platforms Affected

Android

Behavioral/Visual Changes

In case a SearchHandler (or an inheriting class) is declared within XAML providing Focused and/or Unfocused EventHandler functions, upgrading to this version will result in such events to actually trigger.

Before/After Screenshots

Before: The SearchHandler.cs Focused and/or Unfocused overridable functions nor the Focused and/or Unfocused EventHandlers (no matter if declared in XAML or CodeBehind) wouldn't trigger. Actions within the event handlers wouldn't take place (in this test, the labels contents wouldn't be shown).

After:

The SearchHandler.cs Focused and/or Unfocused overridable functions and the Focused and/or Unfocused EventHandlers (no matter if declared in XAML or CodeBehind) trigger. Actions within the event handlers take place (in this test, the labels contents are shown)

Issue7956.xaml - Focus

Issue7956.xaml - Unfocus

Issue7956_1.xaml - Focus

Issue7956_1.xaml - Unfocus

Testing Procedure

Launch any of the Xamarin.Forms.ControlGallery.[Platform]
On the main App screen, hit the 'GO TO TEST CASES'
Search for the Test case 7956 and open it.
A Shell view appears having a SearchHandler control on top, also a descriptive text explaining the test itself.
Click (or poke) on the SearchHandler in order to cause the Focus event to trigger. A text should appear saying: Focus EventHandler triggered!
Click (or poke) on any other area of the screen but the SearchHandler in order to cause the Unfocus event to trigger. A text should appear saying: Unfocus EventHandler triggered!

If steps 5 or 6 produce the mentioned output, the test would have passed. Otherwise, it would have failed.

PR Checklist

Targets the correct branch
Tests are passing (or failures are unrelated)

…Handlers triggering test. FAILS (does nothing)

net-foundation-cla · 2021-06-29T01:07:37Z

All CLA requirements met.

…sed EventHandlers triggering test. FAILS (does nothing)

…textBlock. This is done in order to propagate any Focus state change from the textBlock (graphically, the search bar for the SearchHandler) to the SearchHandler IsFocused state. This causes the SearchHandler.OnIsFocusedPropertyChanged method to finally execute, as it was originally expected to do so.

germancoines

After testing the original cases as well as the new ones I've implemented, and reviewing the changes: As far as I'm concerned the impact of this PR is minimal. No breaking changes are introduced.

Xamarin.Forms.Platform.Android/Renderers/ShellSearchView.cs

Co-authored-by: Pedro Jesus <pedrojesus.cefet@gmail.com>

germancoines

Thanks @pictos !

Xamarin.Forms.Platform.Android/Renderers/ShellSearchView.cs

pictos · 2021-07-01T02:29:04Z

@germancoines why do you implemented 7956 and 7956_1 tests? They look like the same thing. Am I missing something here? Other than that your code fixed the issue.

@PureWeen can you take a look at this one?

germancoines · 2021-07-01T16:17:37Z

@germancoines why do you implemented 7956 and 7956_1 tests? They look like the same thing. Am I missing something here? Other than that your code fixed the issue.

@PureWeen can you take a look at this one?

Hi @pictos, @PureWeen . I just did it to check that the events (declared on XAML) were firing on both SearchHandler as well as inheriting classes . May be this wasn't needed at all, but I just wanted to double check that, as it seems to be a common use case scenario for developers.

See this comment I left on the Issue: #7956 (comment)

germancoines · 2021-07-28T22:25:33Z

Hi @pictos @PureWeen. is there any chance to have a second reviewer for this PR and related issue?

germancoines · 2021-10-20T11:03:58Z

Thanks @jsuarezruiz
@pictos , do you have a chance to finally approve the changes?

pictos · 2021-10-24T15:39:35Z

@germancoines I don't have permissions on this repo. So if Javier approved it's good to be merged (:

Thanks for your contribution ❣️

jfversluis · 2022-01-14T13:22:30Z

Hey @germancoines thank you so much for your patience here. And @pictos thanks for helping out here as well!

If this is still of interest to you, could you confirm this is still an issue with the current stable version of Forms? I know there have been some changes in this area, and just want to confirm that this hasn't been fixed by another PR. Thanks!

germancoines · 2022-01-20T14:45:19Z

Hey @germancoines thank you so much for your patience here. And @pictos thanks for helping out here as well!

If this is still of interest to you, could you confirm this is still an issue with the current stable version of Forms? I know there have been some changes in this area, and just want to confirm that this hasn't been fixed by another PR. Thanks!

Hi @jfversluis , I'll be checking this as soon as I can to be back with the results for your request :)

germancoines · 2022-03-09T20:52:14Z

Hi @jfversluis, is there any update on this after my previous confirmation?

jfversluis · 2022-03-10T12:27:17Z

@germancoines unfortunately not. So what you are basically saying is this change still needs to happen, right?

jfversluis · 2022-03-10T12:27:22Z

/azp run

azure-pipelines · 2022-03-10T12:27:36Z

Azure Pipelines successfully started running 1 pipeline(s).

germancoines · 2022-03-29T19:17:00Z

@germancoines unfortunately not. So what you are basically saying is this change still needs to happen, right?

Yes @jfversluis.

Not sure about the failing test results for OSX and Windows on the pipeline. Is that usual?

germancoines · 2022-07-28T15:09:25Z

/azp run

azure-pipelines · 2022-07-28T15:09:31Z

Commenter does not have sufficient privileges for PR 14375 in repo xamarin/Xamarin.Forms

germancoines · 2022-07-28T15:13:14Z

Hi @jfversluis @jsuarezruiz, I was willing to run the pipeline again in order to check the Build errors. Would it be possible to relaunch the pipeline, please?

jfversluis · 2024-04-25T07:59:34Z

Now that we're so close to the sunsetting of Xamarin.Forms unfortunately we won't be able to take this in anymore, we're really sorry about that. Nevertheless, thank you so much for your time and effort that you have put into this PR.

Please have a look at the evolution of Xamarin.Forms, .NET MAUI. A lot of development has been going on there. Hopefully this issue was already fixed in that codebase. If not, feel free to port this over to there.

Again, thank you so much for being a contributor and Xamarin.Forms user!

xamarin#7956 SearchHandler XAML declared. Focused and Unfocused Event…

35ac0e0

…Handlers triggering test. FAILS (does nothing)

germancoines mentioned this pull request Jun 29, 2021

[Shell] Unable to focus and unfocus search handler for Android #7956

Open

Germán Coines Laguna added 3 commits June 29, 2021 03:34

xamarin#7956 InheritedSearchHandler XAML declared. Focused and Unfocu…

19a9ada

…sed EventHandlers triggering test. FAILS (does nothing)

override of OnFocused and OnUnfocus methods on InheritedSearchHandler.

828ba2f

germancoines marked this pull request as ready for review June 29, 2021 12:51

germancoines changed the title ~~#7956 SearchHandler XAML declared. Focused and Unfocused EventHandler…~~ #7956 [Shell] Unable to focus and unfocus search handler for Android Jun 29, 2021

germancoines commented Jun 29, 2021

View reviewed changes

germancoines mentioned this pull request Jun 29, 2021

[Shell,iOS,Android] Add Colors, Font, HorizontalAlignment. Keyboard, Focus to SearchHandler #5810

Merged

3 tasks

pictos reviewed Jun 30, 2021

View reviewed changes

Xamarin.Forms.Platform.Android/Renderers/ShellSearchView.cs Outdated Show resolved Hide resolved

ammends derived from @pictos code review.

15ff9ed

pictos reviewed Jul 1, 2021

View reviewed changes

Xamarin.Forms.Platform.Android/Renderers/ShellSearchView.cs Outdated Show resolved Hide resolved

germancoines and others added 2 commits July 1, 2021 03:29

Update Xamarin.Forms.Platform.Android/Renderers/ShellSearchView.cs

a097670

Co-authored-by: Pedro Jesus <pedrojesus.cefet@gmail.com>

indentation

db148b1

germancoines commented Jul 1, 2021

View reviewed changes

Xamarin.Forms.Platform.Android/Renderers/ShellSearchView.cs Outdated Show resolved Hide resolved

Xamarin.Forms.Platform.Android/Renderers/ShellSearchView.cs Outdated Show resolved Hide resolved

Xamarin.Forms.Platform.Android/Renderers/ShellSearchView.cs Outdated Show resolved Hide resolved

germancoines added 2 commits July 6, 2021 14:52

Merge branch '5.0.0' into fix-7956

8c93095

Merge branch '5.0.0' into fix-7956

20c2cb6

jsuarezruiz added a/shell 🐚 p/Android labels Aug 16, 2021

Merge branch '5.0.0' into fix-7956

8c112be

jsuarezruiz approved these changes Sep 22, 2021

View reviewed changes

Merge branch '5.0.0' into fix-7956

ffae0ea

jfversluis closed this Apr 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#7956 [Shell] Unable to focus and unfocus search handler for Android #14375

#7956 [Shell] Unable to focus and unfocus search handler for Android #14375

germancoines commented Jun 29, 2021 •

edited

Loading

net-foundation-cla bot commented Jun 29, 2021 •

edited

Loading

germancoines left a comment

germancoines left a comment

pictos commented Jul 1, 2021

germancoines commented Jul 1, 2021 •

edited

Loading

germancoines commented Jul 28, 2021

germancoines commented Oct 20, 2021

pictos commented Oct 24, 2021

jfversluis commented Jan 14, 2022

germancoines commented Jan 20, 2022

germancoines commented Mar 9, 2022

jfversluis commented Mar 10, 2022

jfversluis commented Mar 10, 2022

azure-pipelines bot commented Mar 10, 2022

germancoines commented Mar 29, 2022

germancoines commented Jul 28, 2022

azure-pipelines bot commented Jul 28, 2022

germancoines commented Jul 28, 2022

jfversluis commented Apr 25, 2024

#7956 [Shell] Unable to focus and unfocus search handler for Android #14375

#7956 [Shell] Unable to focus and unfocus search handler for Android #14375

Conversation

germancoines commented Jun 29, 2021 • edited Loading

Description of Change

Issues Resolved

API Changes

Platforms Affected

Behavioral/Visual Changes

Before/After Screenshots

Testing Procedure

PR Checklist

net-foundation-cla bot commented Jun 29, 2021 • edited Loading

germancoines left a comment

Choose a reason for hiding this comment

germancoines left a comment

Choose a reason for hiding this comment

pictos commented Jul 1, 2021

germancoines commented Jul 1, 2021 • edited Loading

germancoines commented Jul 28, 2021

germancoines commented Oct 20, 2021

pictos commented Oct 24, 2021

jfversluis commented Jan 14, 2022

germancoines commented Jan 20, 2022

germancoines commented Mar 9, 2022

jfversluis commented Mar 10, 2022

jfversluis commented Mar 10, 2022

azure-pipelines bot commented Mar 10, 2022

germancoines commented Mar 29, 2022

germancoines commented Jul 28, 2022

azure-pipelines bot commented Jul 28, 2022

germancoines commented Jul 28, 2022

jfversluis commented Apr 25, 2024

germancoines commented Jun 29, 2021 •

edited

Loading

net-foundation-cla bot commented Jun 29, 2021 •

edited

Loading

germancoines commented Jul 1, 2021 •

edited

Loading