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

Import Keyboard Extension APIs #16572

Merged
merged 28 commits into from
Aug 17, 2023

Conversation

drasticactions
Copy link
Contributor

Fixes #11565

  • Import CommunityToolkit.MAUI Keyboard Extensions into MAUI Proper
  • Retrofit Android KeyboardManager into Keyboard Extensions.

@Eilon Eilon added the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label Aug 7, 2023
@drasticactions drasticactions marked this pull request as ready for review August 9, 2023 08:49
@drasticactions
Copy link
Contributor Author

@PureWeen I wrote a simple Appium test for verifying the change. I was wondering how in-depth to go with it since your keyboard PR covers similar ground, and it may make sense to combine the tests onto a single page.

@samhouts samhouts added this to the .NET 8 GA milestone Aug 9, 2023
@samhouts samhouts requested review from a team and StephaneDelcroix and removed request for a team August 10, 2023 18:37
@PureWeen PureWeen requested review from rachelkang and removed request for StephaneDelcroix August 10, 2023 23:08
@Eilon
Copy link
Member

Eilon commented Aug 10, 2023

@rachelkang - are you able to review this PR?

src/Core/src/Platform/KeyboardExtensions.cs Outdated Show resolved Hide resolved
@@ -60,5 +65,67 @@ public static InputTypes ToInputType(this Keyboard self)

return result;
}

internal static bool HideKeyboard(this AView inputView)
Copy link
Member

Choose a reason for hiding this comment

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

Rename these to match HideSoftInput

using IPlatformViewHandler = Microsoft.Maui.IViewHandler;
#endif

namespace Microsoft.Maui.Platform;
Copy link
Member

Choose a reason for hiding this comment

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

Hmmm I'm wondering if we want the extensions against ITextInput to be inside the Microsoft.Maui namespace so they are just by default exposed opposed to needing to pull in Microsoft.Maui.Platform. I realize that create of a naming conflict.

What if for the public facing one we just name it SoftInputExtentions

Co-authored-by: Shane Neuville <shane94@hotmail.com>
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.

Thanks for bringing these in!

Seconding Shane on renaming all the "Keyboard" stuff to "SoftInput" and a few other comments, but otherwise LGTM!

src/Core/src/Platform/Android/KeyboardExtensions.cs Outdated Show resolved Hide resolved
src/Core/src/Platform/Android/KeyboardExtensions.cs Outdated Show resolved Hide resolved
src/Core/src/Platform/Android/KeyboardExtensions.cs Outdated Show resolved Hide resolved
src/Core/src/Platform/iOS/KeyboardExtensions.cs Outdated Show resolved Hide resolved
src/Core/src/Platform/iOS/KeyboardExtensions.cs Outdated Show resolved Hide resolved
src/Core/src/Platform/iOS/KeyboardExtensions.cs Outdated Show resolved Hide resolved
src/Core/src/Platform/iOS/KeyboardExtensions.cs Outdated Show resolved Hide resolved
src/Core/src/Platform/Android/KeyboardExtensions.cs Outdated Show resolved Hide resolved
drasticactions and others added 3 commits August 14, 2023 10:47
Co-authored-by: Rachel Kang <rachel.j.kang@gmail.com>
Co-authored-by: Rachel Kang <rachel.j.kang@gmail.com>
@drasticactions drasticactions requested a review from a team as a code owner August 14, 2023 03:29
@PureWeen
Copy link
Member

@drasticactions once this PR is merged that should fix up UITests from failing in CI

@PureWeen
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@drasticactions drasticactions enabled auto-merge (squash) August 16, 2023 02:30
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! Thanks for incorporating all the feedback!

One thing I've noticed is that your indentations generally look extra space-y - could you review your tabs/spaces, indentations, new lines to make sure they're all as expected?

I have a few other nitpick suggestions and questions I've left comments on, but overall, looks great!

src/Core/src/Platform/Android/SoftInputExtensions.cs Outdated Show resolved Hide resolved
src/Core/src/Platform/Android/SoftInputExtensions.cs Outdated Show resolved Hide resolved
src/Core/src/Platform/SoftInputExtensions.cs Outdated Show resolved Hide resolved
src/Core/src/Platform/SoftInputExtensions.cs Outdated Show resolved Hide resolved
src/Core/src/Platform/SoftInputExtensions.cs Outdated Show resolved Hide resolved
src/Core/src/Platform/SoftInputExtensions.cs Outdated Show resolved Hide resolved
src/Core/src/Platform/SoftInputExtensions.cs Outdated Show resolved Hide resolved
src/Core/src/Platform/SoftInputExtensions.cs Outdated Show resolved Hide resolved
src/Core/src/Platform/SoftInputExtensions.cs Outdated Show resolved Hide resolved
src/Core/src/Platform/SoftInputExtensions.cs Outdated Show resolved Hide resolved
drasticactions and others added 2 commits August 16, 2023 14:12
Co-authored-by: Rachel Kang <rachel.j.kang@gmail.com>
Co-authored-by: Rachel Kang <rachel.j.kang@gmail.com>
rachelkang
rachelkang previously approved these changes Aug 16, 2023
using IPlatformViewHandler = Microsoft.Maui.IViewHandler;
#endif

namespace Microsoft.Maui.Platform;
Copy link
Member

@PureWeen PureWeen Aug 16, 2023

Choose a reason for hiding this comment

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

Thoughts on

  • Move this extension to Microsoft.Maui and up one folder
    • My concern is requiring folks to import Microsoft.Maui.Platform to use these APIs
  • move the platform level implementations out to other extension methods for those view types
    • FrameworkElementExtentions (windows)
    • ViewExtentions (iOS)
    • ViewExtentions (Android)

@drasticactions drasticactions merged commit 22fab2d into dotnet:main Aug 17, 2023
39 checks passed
rmarinho pushed a commit that referenced this pull request Aug 19, 2023
* Import Keyboard Extension APIs

* Tizen...

* Test

* Fix API

* API

* Fix original tests

* Add Soft Input buttons to entry page

* Start adding tests

* Update test

* Update src/Core/src/Platform/KeyboardExtensions.cs

Co-authored-by: Shane Neuville <shane94@hotmail.com>

* Apply suggestions from code review

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

* Apply suggestions from code review

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

* Move namespaces/files

* API

* Internal Compatibility update

* More fixes

---------

Co-authored-by: Shane Neuville <shane94@hotmail.com>
Co-authored-by: Rachel Kang <rachel.j.kang@gmail.com>
@github-actions github-actions bot locked and limited conversation to collaborators Dec 7, 2023
@Eilon Eilon added the area-keyboard Keyboard, soft keyboard label May 13, 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-keyboard Keyboard, soft keyboard fixed-in-8.0.0-rc.1.9171 Look for this fix in 8.0.0-rc.1.9171 legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ITextInput extensions for interacting with platform soft input (keyboard)
5 participants