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

Improve keyboard scrolling with editors, contentInsets, and different keyboards #24589

Merged
merged 13 commits into from
Sep 19, 2024

Conversation

tj-devel709
Copy link
Member

@tj-devel709 tj-devel709 commented Sep 3, 2024

Description of Change

This PR addresses a few different but related issues regarding the user experience with the keyboard on iOS devices.

  1. The first issue is that currently, keyboard scrolling logic only applies if a focused entry/editor’s cursor is obstructed by the keyboard. Not only does the keyboard scroll, but the contentInsets are adjusted on the scrolling page/control allowing us to scroll to the top and bottom even with the keyboard showing. However, if we interact with something above the keyboard, we still need to adjust the contentInsets to adjust so that we can scroll and reach the bottom of the content now covered by the showing keyboard. This PR addresses this issue.

  2. The second issue is when pickers are focused, the cursor has a size of zero so when we click on a picker, the screen scrolls to where only the top of the picker is showing. This PR makes it so the whole picker is now visible.

  3. The third issue this PR addresses is when the keyboard is already showing and we focus a different control that invokes a different type of keyboard, we were not calculating if we need to scroll again. This issue shows up when things like a picker are focused and then an entry with a larger numeric keyboard is focused after.

  4. The final issue this PR addresses is keeping the cursor above the keyboard when we add new lines to an editor. This is done by changing the contentInset on editors (whether the editor is scrolling or not) essentially moving the bottom of the editor above the keyboard. As a result, we can have a much better experience and see the cursor as we are typing.

This PR is a continuation of #21807 and #21932 and a special thanks to @albyrock87 for seeing that keeping the editor’s cursor above the keyboard was possible and for pointing out the correct way to find the rects respective to the windows which were both crucial to this PR!

KeyboardScrollingDemo.mp4

Small inconsistency where the cursor may stay a little higher above the keyboard than usual in an edge case: #24635.

Issues Fixed

Fixes #19214
Fixes #24496
Fixes #20631
Fixes #22715

@albyrock87
Copy link
Contributor

I know this was painful to fix, so thanks a lot for all the effort spent here!

@ramonB1996
Copy link

Thank you for your work on this! Please let me know if you need somebody to test it for you :)

I am patiently waiting for the next SR that contains this PR

@ajay-mundi
Copy link

thanks @tj-devel709 this is super helpful

@mattleibow
Copy link
Member

@ramonB1996 you can test this PR using the artifacts from the MAUI-public build. Just download the nuget artifact and use that in your project.

PureWeen
PureWeen previously approved these changes Sep 11, 2024
PureWeen
PureWeen previously approved these changes Sep 11, 2024
mattleibow
mattleibow previously approved these changes Sep 11, 2024
@tj-devel709
Copy link
Member Author

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@tj-devel709
Copy link
Member Author

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@rmarinho
Copy link
Member

Seems IOS test failing

HideSoftInputOnTappedPageTest("Editor",True)

@tj-devel709 tj-devel709 force-pushed the dev/TJ/keyboardScroll-Inset-Improvements_1 branch from 508ad6e to 62f48f0 Compare September 13, 2024 17:56
@tj-devel709 tj-devel709 force-pushed the dev/TJ/keyboardScroll-Inset-Improvements_1 branch from 62f48f0 to ef5db99 Compare September 16, 2024 14:40
@crsillerjp
Copy link

Is there an ETA for this change? We are hoping on a release in the next few days or we have to decide to manually backport it into our branch.

@PureWeen PureWeen merged commit 14c7719 into main Sep 19, 2024
97 checks passed
@PureWeen PureWeen deleted the dev/TJ/keyboardScroll-Inset-Improvements_1 branch September 19, 2024 11:16
@samhouts samhouts added the fixed-in-net8.0-nightly This may be available in a nightly release! label Sep 20, 2024
redducks100 pushed a commit to redducks100/maui that referenced this pull request Sep 24, 2024
… keyboards (dotnet#24589)

* Changes to improve keyboard scrolling with editors, contentInsets, and different keyboards

* picker6 screenshot

* Add entry7 screenshot test

* reset test state

* fix the distance between keyboard and cursor and remove null forgiving

* bottomBoundary should have the TextDistanceFromBottom when the bottom is not the keyboard

* FireAndForget

* new entry7 screenshot

* add back the distance from bottom in the insets

* Allow partial scrolling one direction and then moving the next scrollview the opposite direction

* Adjust insets on UITextView and other scrollviews

* we dont need more inset if no keyboard intersection

* add edge case where we have small editor
@samhouts samhouts added the fixed-in-net9.0-nightly This may be available in a nightly release! label Oct 1, 2024
@samhouts samhouts added fixed-in-9.0.0-rc.2.24503.2 fixed-in-net8.0-nightly This may be available in a nightly release! and removed fixed-in-net9.0-nightly This may be available in a nightly release! fixed-in-net8.0-nightly This may be available in a nightly release! labels Oct 14, 2024
@Toine-db
Copy link

Toine-db commented Oct 16, 2024

Why hasn't this been released yet? meanwhile there have been two releases without this fix (sr9.1 and sr9.2)

We have multiple production releases waiting for this fix.

@jfversluis
Copy link
Member

@Toine-db as indicated by the labels, this is available in the nightlies as well as .NET 9 RC2. With .NET 9 close to being released I'm not sure if this will be backported to .NET 8 still but I'll ask around!

@MartyIX
Copy link
Contributor

MartyIX commented Oct 16, 2024

We have multiple production releases waiting for this fix.

Perhaps you can use a nightly version which should contain this fix.

@Toine-db
Copy link

Toine-db commented Oct 16, 2024

@Toine-db as indicated by the labels, this is available in the nightlies as well as .NET 9 RC2. With .NET 9 close to being released I'm not sure if this will be backported to .NET 8 still but I'll ask around!

My apologies, I meant the releases of .Net8 and I assumed it was backported to Net8 because it was in de Net8 nightly.

I tested it with the nightly build, and that works fine. But I can't release production versions for our apps with nightly or beta versions of any kind.

@jfversluis
Copy link
Member

Ah yeah! I see what you're saying sorry.

Since we're so close to .NET 9, I'm not sure if there will be another "full" .NET 8 SR soon unless we really have to. So I'm afraid this will probably land in .NET 9.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet