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

iOS Keyboard Scrolling Improvements and UITests #16201

Closed
wants to merge 12 commits into from

Conversation

tj-devel709
Copy link
Member

@tj-devel709 tj-devel709 commented Jul 17, 2023

Description of Change

MCT Popup

The iOS Keyboard Scrolling solution I posted a few months ago had an issue with the Maui Community Toolkit Popup. The main issue was that the popup has some external scrolling going on. The Maui code would grab the initial position of the cursor, the external scrolling would happen, and then our Maui code would scroll from the initial position making things messy and incorrect. The solution I came up with is refactoring the portion of the code that calculates where the cursor on the editor/entry is to later in the code and monitoring the ContainerView to see if there was a change in position and allotting time until this external scrolling is finished to start our scroll calculations. This should also help in future situations where we are not able to control an external scroll.

Before

PopupScroll-Before.mov

After

PopupScroll-After.mov

Next Return Key fix

When we have an entry with the ReturnType of 'Next' and we press that next button to go into an editor, the editor does not come up high enough. This was due to the additional AccessoryView that is added to the keyboard for editors in Maui. When we hit this scenario of switching between an editor and entry, we should recalculate to see if the keyboard changed sizes.

Scrolling animation glitch

There was a small glitch in the scrolling animation. While we are computing the animation speed the keyboard will use to come up, the screen would scroll a little after it making it a tad jumpy. Since we no longer need the automatic delay in the AdjustPositionDebounce method since the cursor location logic is moved to later in the process, I found that this delay was causing the small jump in the animation.

UITests

Testing the iOS Keyboard scenarios is difficult as there are many different scenarios. I created some UITests that can help keep track of the verification that scrolling is happening correctly.

It is important to note that each of the iOS Scrolling UI Tests are run with four different configurations:

  • Using iOS Large Titles with a scrollable page
  • Not using iOS Large Titles with a scrollable page
  • Using iOS Large Titles with a non-scrollable page
  • Not using iOS Large Titles with a non-scrollable page

Example of the Large Titles and Small Titles respectively:

Screenshot 2023-09-22 at 1 51 33 PM Screenshot 2023-09-22 at 1 52 21 PM

Testing multiple editors in different locations

KeyboardScrollingEditorsDemo.mov

Testing multiple entries in different locations

KeyboardScrollingEntryDemo.mov

Entry Next Editor Test

KeyboardScrollingNextDemo.mov

Issues Fixed

Fixes PureWeen/ShanedlerSamples#13
Fixes #17588
#17559

@tj-devel709 tj-devel709 added platform/iOS 🍎 legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor area-controls-entry Entry labels Jul 17, 2023
jknaudt21
jknaudt21 previously approved these changes Jul 26, 2023
@Eilon
Copy link
Member

Eilon commented Aug 7, 2023

@tj-devel709 / @PureWeen - is there anything else required for this PR to be merged? I see there is a pending suggestion there, but I suggest that if we think this PR is a definite improvement, maybe it's worth merging as-is?

@tj-devel709
Copy link
Member Author

We've had more discussions on this one and want to talk to the MauiCommunityToolkit about their process for this popup and go from there

@tj-devel709 tj-devel709 marked this pull request as draft August 16, 2023 22:07
@samhouts samhouts added this to the Under Consideration milestone Aug 31, 2023
@@ -257,7 +266,26 @@ internal static async Task AdjustPositionDebounce()

var entranceCount = DebounceCount;

await Task.Delay(10);
Copy link
Member Author

Choose a reason for hiding this comment

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

This delay is not really neccessary now. This was added in originally because clicking the entry triggers an event and the keyboard rising triggers an event. These events were sort of racing and we needed information about the cursor coming from the entry trigger. Now, the cursor logic is moved to later in the process and now we don't really need this delay. I would consider leaving this in just in case, but there is a small animation hiccup with this delay, and without it, the scrolling is MUCH more smooth!

@tj-devel709 tj-devel709 changed the title iOS Keyboard Scrolling - MCT Popup Scrolling Fix iOS Keyboard Scrolling Improvements and UITests Sep 22, 2023
@tj-devel709 tj-devel709 marked this pull request as ready for review September 25, 2023 14:23
@tj-devel709 tj-devel709 requested a review from a team as a code owner September 25, 2023 14:23
@tj-devel709
Copy link
Member Author

Trying this PR here instead to help an issue with UITests when creating PR from fork

@github-actions github-actions bot locked and limited conversation to collaborators Dec 8, 2023
@Eilon Eilon removed the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label May 10, 2024
@samhouts samhouts removed this from the Under Consideration milestone Jul 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants