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

Generalize HideSoftInputOnTapped on Android and iOS to support 3rd party input controls #19626

Merged

Conversation

albyrock87
Copy link
Contributor

Description of Change

Generalize recognition of text inputs in HideSoftInputOnTappedChangedManager so that 3rd party libraries with native handlers can leverage this new feature of ContentPage by simply adding a mapper the same way InputView does.

internal static void MapIsFocused(IViewHandler handler, IView view)
{
	handler
		?.GetService<HideSoftInputOnTappedChangedManager>()
		?.UpdateFocusForView(iv);
}

Unfortunately HideSoftInputOnTappedChangedManager is internal, but they can still use it via reflection.

I know it's suboptimal but I didn't want to change public API.

See more at: https://supportcenter.devexpress.com/ticket/details/t1201718/the-keyboard-remains-visible-on-a-click-outside-editors-even-if-the-contentpage

@albyrock87 albyrock87 requested a review from a team as a code owner December 29, 2023 19:59
@ghost ghost added the community ✨ Community Contribution label Dec 29, 2023
@ghost
Copy link

ghost commented Dec 29, 2023

Hey there @albyrock87! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@PureWeen
Copy link
Member

PureWeen commented Jan 4, 2024

Can you add tests for this?

We have some existing tests for this here.

public class HideSoftInputOnTappedPage : TestContentPage

@albyrock87
Copy link
Contributor Author

@PureWeen I think existing unit tests already cover this for the following reasons:

  • iOS => UISearchBar is generally a UIView which contains an UITextField first responder
  • Android => SearchView is a ViewGroup which contains an EditText control
  • existing unit tests cover the search bar control

Therefore, the fact that I removed specific reference to UISearchBar and SearchView and everything still works proofs implicitly that:

  • Search bar still works
  • Generalization works

I don't see what other kind of unit test I should add given that search bar is already an example of a native control which contains a text editor.

@jurganson
Copy link

Bump!

@Alexgoon
Copy link

Alexgoon commented Feb 1, 2024

Hello,
We are working on DevExpress .NET MAUI components, and functionality in our editors depend on this pull request. Could you kindly provide information on whether there are plans to incorporate it into .NET 8? This clarification is crucial for our ongoing work, as it will help us determine whether we can afford to wait for an update or if we should proceed with implementing a temporary workaround.

@albyrock87
Copy link
Contributor Author

@Alexgoon have you seen the workaround I proposed here?
This is fully working and probably covers even more uses cases than what could be achieved with what I proposed here.
I would like to help the "DevExpress Community" but your support center makes it impossible to comment on other tickets.

I will try to propose a different PR here with that strategy (which is completely different but more effective).
But considering the type of change, I guess it'll eventually end up in .NET 9.

@Alexgoon
Copy link

Alexgoon commented Feb 1, 2024

@albyrock87, thank you very much for your contribution! Yes, we've seen the workaround, and it can definitely be used as a temporary solution by our customers. We've mentioned this technique in the following thread: The Keyboard remains visible on a click outside editors even if the ContentPage.HideSoftInputOnTapped property is enabled

However, the workaround requires overriding a MauiAppCompatActivity method, but we would like to fix the issue on our side so that our customers can get a working solution in the default configuration. That's why we are hoping that this PR will be introduced in .NET. However, .NET 9 is too far away, and I'm afraid too many customers may face the issue until a new version is available.

@albyrock87
Copy link
Contributor Author

@Alexgoon if you want to avoid that, you can probably use reflection to add that part of my workaround here:

internal event EventHandler<MotionEvent?>? DispatchTouchEvent;

@PureWeen
Copy link
Member

/rebase

@github-actions github-actions bot force-pushed the generalize-hide-soft-input-on-tapped branch from 40ec49b to d9a9f02 Compare February 28, 2024 21:36
@PureWeen
Copy link
Member

@PureWeen
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

PureWeen commented Mar 1, 2024

Failing tests are unrelated

@PureWeen PureWeen merged commit 11a387b into dotnet:main Mar 5, 2024
37 of 47 checks passed
@albyrock87 albyrock87 deleted the generalize-hide-soft-input-on-tapped branch March 10, 2024 20:04
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants