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.

Sign up for GitHub

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

Jump to bottom

Implement Keyboard on SearchBarHandler #9851

Merged
merged 25 commits into from
Mar 17, 2023
Merged

Implement Keyboard on SearchBarHandler #9851

merged 25 commits into from
Mar 17, 2023

Conversation

jsuarezruiz
Copy link
Contributor

Description of Change

Implement Keyboard on SearchBarHandler.

fix-9594

Issues Fixed

Fixes #9594

@jsuarezruiz jsuarezruiz added t/bug Something isn't working legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor area-controls-entry Entry labels Sep 1, 2022
@jsuarezruiz jsuarezruiz mentioned this pull request Sep 2, 2022
Closed
@jsuarezruiz jsuarezruiz marked this pull request as draft November 3, 2022 08:55
@jsuarezruiz jsuarezruiz marked this pull request as ready for review November 18, 2022 12:56
@jsuarezruiz jsuarezruiz changed the base branch from net6.0 to main November 22, 2022 16:02
@jsuarezruiz
Copy link
Contributor Author

Changed target from net6.0 to main.

rachelkang
rachelkang reviewed Dec 9, 2022
View reviewed changes
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.

I just tested and unfortunately, this doesn't fix the issue referenced :/ The keyboard continues to switch immediately back from numeric to alphabetic

I am curious about your approach here, though, as the issue you're working on here is related to the issue I'm working on in #11981

@jsuarezruiz
Copy link
Contributor Author

jsuarezruiz commented Dec 9, 2022 •
edited
Loading

I just tested and unfortunately, this doesn't fix the issue referenced :/ The keyboard continues to switch immediately back from numeric to alphabetic

I am curious about your approach here, though, as the issue you're working on here is related to the issue I'm working on in #11981

On Android?. Let me review it and take a quick look to the Japanese issue. This one was target to net6.0 and retarget to main now. Maybe something has changed in between.

@jsuarezruiz
Copy link
Contributor Author

Added several Device tests validating all the keyboards on the SearchBar.

image

mandel-macaque
mandel-macaque requested changes Feb 17, 2023
View reviewed changes
src/Core/src/Platform/Android/SearchViewExtensions.cs Show resolved Hide resolved
src/Core/src/Platform/Windows/SearchBarExtensions.cs
Comment on lines +138 to +143
var queryTextBox = platformControl.GetFirstDescendant<TextBox>();

if (queryTextBox == null)
return;

queryTextBox.UpdateInputScope(searchBar);
Copy link
Member

Choose a reason for hiding this comment

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

I have added #13421 to make it cleaner.

src/Core/tests/DeviceTests/Handlers/SearchBar/SearchBarHandlerTests.Android.cs Outdated Show resolved Hide resolved
src/Core/tests/DeviceTests/Handlers/SearchBar/SearchBarHandlerTests.Android.cs Outdated Show resolved Hide resolved
src/Core/tests/DeviceTests/Handlers/SearchBar/SearchBarHandlerTests.Android.cs Outdated Show resolved Hide resolved
src/Core/tests/DeviceTests/Handlers/SearchBar/SearchBarHandlerTests.iOS.cs Outdated Show resolved Hide resolved
src/Core/tests/DeviceTests/Handlers/SearchBar/SearchBarHandlerTests.iOS.cs Outdated Show resolved Hide resolved
src/Core/tests/DeviceTests/Handlers/SearchBar/SearchBarHandlerTests.iOS.cs Outdated Show resolved Hide resolved
src/Core/tests/DeviceTests/Handlers/SearchBar/SearchBarHandlerTests.iOS.cs Outdated Show resolved Hide resolved
src/Core/tests/DeviceTests/Handlers/SearchBar/SearchBarHandlerTests.iOS.cs Outdated Show resolved Hide resolved
@samhouts
Copy link
Member

samhouts commented Mar 3, 2023

Was this already resolved by #11981?

@mandel-macaque
Copy link
Member

@jsuarezruiz lets close it or land it. We have too many PRs.

@jsuarezruiz
Copy link
Contributor Author

Was this already resolved by #11981?

This PR maps the Keyboard property to allow setting the numeric keypad for example. The other PR is a nice fix that prevents flickering on keyboard hints etc. Related but not the same.

@jsuarezruiz
Copy link
Contributor Author

@jsuarezruiz lets close it or land it. We have too many PRs.

I agree, let me add more reviewers and push forward.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2023

Thank you for your pull request. We are auto-formatting your source code to follow our code guidelines.

rachelkang
rachelkang requested changes Mar 16, 2023
View reviewed changes
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.

Looks awesome! Left a few comments. I haven't been able to test on mac/ios yet, as my setup is currently in flux. I'll test on those platforms ASAP unless someone else is able to get to it first. Looking forward to seeing this through!

src/Core/tests/DeviceTests/Handlers/SearchBar/SearchBarHandlerTests.iOS.cs Show resolved Hide resolved
src/Core/tests/DeviceTests/Handlers/SearchBar/SearchBarHandlerTests.iOS.cs Show resolved Hide resolved
src/Core/src/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt Show resolved Hide resolved
src/Core/tests/DeviceTests/Handlers/SearchBar/SearchBarHandlerTests.cs
Comment on lines +265 to +272
#if WINDOWS
// The Text keyboard is the default one on Windows
[InlineData(nameof(Keyboard.Default), true)]
// Plain is the same as the Default keyboard on Windows
[InlineData(nameof(Keyboard.Plain), true)]
#else
[InlineData(nameof(Keyboard.Default), false)]
[InlineData(nameof(Keyboard.Plain), false)]
Copy link
Member

Choose a reason for hiding this comment

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

What are "Default" and "Plain" on the other platforms?

Copy link
Member

Choose a reason for hiding this comment

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

on iOS, Default and Text appear the same, and Plain appears differently - maybe we can capture that somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On Android:

Default = InputTypes.ClassText | InputTypes.TextVariationNormal
Text = InputTypes.ClassText | InputTypes.TextFlagCapSentences

On iOS:

Default
textInput.SetAutocapitalizationType(UITextAutocapitalizationType.Sentences);
textInput.SetAutocorrectionType(UITextAutocorrectionType.Default);
textInput.SetSpellCheckingType(UITextSpellCheckingType.Default);

Text
textInput.SetAutocapitalizationType(UITextAutocapitalizationType.Sentences);
textInput.SetAutocorrectionType(UITextAutocorrectionType.Yes);
textInput.SetSpellCheckingType(UITextSpellCheckingType.Yes);

We have this in the docs https://learn.microsoft.com/en-us/dotnet/maui/user-interface/controls/entry?view=net-maui-7.0#customize-the-keyboard

We could extend more details by platform. cc @davidbritch

src/Controls/samples/Controls.Sample/Pages/Controls/BoxViewPage.xaml Outdated Show resolved Hide resolved
src/Core/src/Handlers/SearchBar/SearchBarHandler.Android.cs Outdated Show resolved Hide resolved
@rachelkang
Copy link
Member

looks great on iOS and iPadOS too

@rmarinho rmarinho enabled auto-merge (squash) March 17, 2023 12:59
@rmarinho rmarinho merged commit 8940dce into main Mar 17, 2023
@rmarinho rmarinho deleted the fix-9594 branch March 17, 2023 18:55
@github-actions github-actions bot locked and limited conversation to collaborators Dec 17, 2023
@Eilon Eilon removed the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label May 10, 2024
@samhouts samhouts added the fixed-in-8.0.0-preview.3.8149 Look for this fix in 8.0.0-preview.3.8149! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rmarinho rmarinho rmarinho approved these changes

@mandel-macaque mandel-macaque mandel-macaque approved these changes

@rachelkang rachelkang rachelkang approved these changes

@PureWeen PureWeen Awaiting requested review from PureWeen

Assignees
No one assigned
Labels
area-controls-entry Entry fixed-in-8.0.0-preview.3.8149 Look for this fix in 8.0.0-preview.3.8149! t/bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Searchbar keyboard is switching back to alphabet characters immediately after entering one numeric character
6 participants
@jsuarezruiz @samhouts @mandel-macaque @rachelkang @rmarinho @Eilon