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

[Android] Fix Keyboard.Numeric issue #16163

Merged
merged 6 commits into from
Aug 25, 2023
Merged

[Android] Fix Keyboard.Numeric issue #16163

merged 6 commits into from
Aug 25, 2023

Conversation

jsuarezruiz
Copy link
Contributor

@jsuarezruiz jsuarezruiz commented Jul 14, 2023

Description of Change

Fix Keyboard.Numeric issue on Android.

fix-keyboard-alert

Change the Android device or simulator language to French and launch the .NET MAUI Gallery. Navigate to Controls > Entry. Try to introduce decimals using the comma as separator in the entry with Keyboard.Numeric

Issues Fixed

Fixes #11865
Fixes #15884
Fixes #16429

@jsuarezruiz jsuarezruiz added t/bug Something isn't working platform/android 🤖 area-controls-dialogalert DisplayAlert, dialog area-keyboard Keyboard, soft keyboard labels Jul 14, 2023
@ghost ghost added the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label Jul 14, 2023
@samhouts samhouts added this to the .NET 8 GA milestone Jul 26, 2023
Copy link
Member

@rachelkang rachelkang left a comment

Choose a reason for hiding this comment

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

This issue and fix are not tied to display prompt at all, so I suggest removing the sample from the Alerts Page and adding a simple <Entry Keyboard="Numeric" Text="Numeric text" /> to the EntryPage.xaml instead

(The original issue used a numeric keyboard entry both within and without a display prompt)

other than that, LGTM!

@@ -215,7 +215,7 @@ bool IsDecimalPointChar(char c)
}
}

return stripped ?? filterFormatted ?? new String("");
return stripped ?? filterFormatted;
Copy link
Member

Choose a reason for hiding this comment

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

This appears to fix the issue, but can you help me understand why? Why wasn't it working before with other system display languages and why does it work now with this change?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, Why??? 😭

@jsuarezruiz jsuarezruiz marked this pull request as ready for review August 11, 2023 10:17
@jsuarezruiz jsuarezruiz requested a review from a team as a code owner August 11, 2023 10:17
@jsuarezruiz
Copy link
Contributor Author

This issue and fix are not tied to display prompt at all, so I suggest removing the sample from the Alerts Page and adding a simple <Entry Keyboard="Numeric" Text="Numeric text" /> to the EntryPage.xaml instead

(The original issue used a numeric keyboard entry both within and without a display prompt)

other than that, LGTM!

Right, we already have that sample in the Entry page https://github.com/dotnet/maui/blob/main/src/Controls/samples/Controls.Sample/Pages/Controls/EntryPage.xaml#L207

@rachelkang
Copy link
Member

ok, great! @jsuarezruiz I still recommend removing the AlertsPage sample added in this PR as well. While one of the repros did have a DisplayPrompt, it wasn't related to the issue, so I don't think it's needed.

Copy link
Member

@rachelkang rachelkang left a comment

Choose a reason for hiding this comment

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

LGTM! I just recommend removing the sample as per my last comment #16163 (comment)

@rachelkang rachelkang changed the title [Android] Fix Keyboard.Numeric issue on DisplayPromptAsync [Android] Fix Keyboard.Numeric issue Aug 23, 2023
@rmarinho rmarinho merged commit 5ac1e80 into main Aug 25, 2023
34 checks passed
@rmarinho rmarinho deleted the fix-11865 branch August 25, 2023 18:23
rmarinho added a commit that referenced this pull request Aug 29, 2023
* [Android] Fix Keyboard.Numeric issue (#16163)

* Fix Keyboard.Numeric issue on DisplayPromptAsync on Android

* Remove AlertsPage sample

---------

Co-authored-by: Rachel Kang <rachel.j.kang@gmail.com>

* Invalidate shell tab title on Windows (#16593)

Co-authored-by: Rui Marinho <me@ruimarinho.net>

* Fix Flyout Crash (Windows) (#16800)

* Fix flyout crash due to invalid casting of child

* Add tests

* Add additional tests, PR feedback

* Fix missing call to `RemoveLogicalChild`

* Update Clear

---------

Co-authored-by: Mike Corsaro <mikecorsaro@microsoft.com>

* [core] factory methods for registering services, part 2 (#17004)

b0bba51 was incomplete, in that it missed registrations with 1 type
argument:

    builder.Services.AddScoped<HideSoftInputOnTappedChangedManager>();

So we need to add entries with "`1" for all of the banned methods:

    ++M:Microsoft.Extensions.DependencyInjection.ServiceCollectionServiceExtensions.AddScoped`1(Microsoft.Extensions.DependencyInjection.IServiceCollection);Use a Factory method to register the service instead
    M:Microsoft.Extensions.DependencyInjection.ServiceCollectionServiceExtensions.AddScoped`2(Microsoft.Extensions.DependencyInjection.IServiceCollection);Use a Factory method to register the service instead

We can improve startup time by using a factory method instead:

    builder.Services.AddScoped(_ => new HideSoftInputOnTappedChangedManager());

This also found a couple more calls to fix throughout .NET MAUI.

* [create-pull-request] automated change (#17017)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Ignore failures from newly added UITests temporarily (#17003)

* Ignore failures from WhenQueryingCarouselItemsInViewThenSingleItemIsRetrieved

* Update Ignore to include all platformss

* Center window on WinUI, so it's always fully on screen in CI

* Set agent screen resolution bigger for UI tests

* Update baseline snapshots for bigger screen

* Just set screen resolution on Windows

* Ignore Issue16320 on Android

* Fix Android text alignment in migrated projects (#16986)

* Fix Android text alignment in migrated projects

* Add (manual) device tests

* Add clarifying comment

* [tentative] Move input tests to generic code

* Wrap up tests

* Make masks constants

* Fix title to be consistent with other test names

* Fix the order of logical modifications (#17020)

* Fix the order of logical modifications

* - keep in sync while removing

* - fix clear

---------

Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com>
Co-authored-by: Rachel Kang <rachel.j.kang@gmail.com>
Co-authored-by: Mike Corsaro <corsarom@gmail.com>
Co-authored-by: Mike Corsaro <mikecorsaro@microsoft.com>
Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Bret Johnson <bret.johnson@microsoft.com>
Co-authored-by: Juan Diego Herrera <juherrera@microsoft.com>
Co-authored-by: Shane Neuville <shneuvil@microsoft.com>
@github-actions github-actions bot locked and limited conversation to collaborators Dec 8, 2023
@samhouts samhouts removed the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label Jan 31, 2024
@samhouts samhouts added the fixed-in-8.0.0-rc.1.9171 Look for this fix in 8.0.0-rc.1.9171 label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-dialogalert DisplayAlert, dialog area-keyboard Keyboard, soft keyboard fixed-in-8.0.0-rc.1.9171 Look for this fix in 8.0.0-rc.1.9171 platform/android 🤖 t/bug Something isn't working
Projects
None yet
5 participants