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 (not from fork) #17670

Merged
merged 21 commits into from
Jan 12, 2024

Conversation

tj-devel709
Copy link
Member

@tj-devel709 tj-devel709 commented Sep 26, 2023

Description of Change

Note: recreating this PR from a branch not in my fork for sake of UITests Issue.

Grid with Star Row Resizing

When there is a grid at the top-level of the content page, and there is at least one star-sized row, the grid would resize when the keyboard came up. This was due to safe-area of the grid on the bottom no longer needing to size down for the bottom SafeAreaInset since the grid was raised higher than the bottom of the device. To handle this, logic was added so that we can call the scrolling code again after the grid (or other view) expands.

Grid with Star Row Resizing Example (click me!)

Before After
OldGridStarScrolling.mov
NewGridStarExample.mov
    <Grid RowDefinitions="40,*,40,40, 40">
        <Button Grid.Row="0" Background="blue" Text="Print CollectionView Height"/>

        <TableView Background="lightblue" x:Name="ColView" Grid.Row="1"/>

        <Button Grid.Row="2" Background="blue" Text="Print CollectionView Height"/>
        <Editor
            x:Name="myEditor"
            Grid.Row="3"
            BackgroundColor="MediumSeaGreen"
            Placeholder="Send Chat"
            PlaceholderColor="White"
            TextColor="White"/>

        <Button Grid.Row="4" Text="Fill collection"/>
    </Grid>

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 sometimes 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.

Scrolling animation glitch Example (click me!)

After
NewScrollingAnimation.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 would sometimes 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.


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 most of the iOS Scrolling UITests 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 (click me!)

Large Titles Small Titles

Testing multiple editors in different locations (click me!)

EntryScrollingTest.mov

Testing multiple entries in different locations (click me!)

EntryScrollingTest.mov

Testing entry next button to editor (click me!)

EntryNextScrollingTest.mov

Testing grid with star row resizing (click me!)

GridStarScrollingTest.mov

Issues Fixed

Fixes #17588
Fixes #17559
Fixes #18354
Fixes #19245

@tj-devel709 tj-devel709 added platform/iOS 🍎 legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor area-controls-entry Entry area-keyboard Keyboard, soft keyboard labels Sep 26, 2023
@tj-devel709 tj-devel709 added this to the Under Consideration milestone Sep 26, 2023
@tj-devel709 tj-devel709 requested a review from a team as a code owner September 26, 2023 21:22
@tj-devel709 tj-devel709 force-pushed the dev/TJ/KeyboardScrolling-Improvements1 branch from 6abc38f to 930833e Compare September 27, 2023 00:51
@samhouts samhouts added the stale Indicates a stale issue/pr and will be closed soon label Oct 18, 2023
@MalmoIt
Copy link

MalmoIt commented Nov 9, 2023

Any progress on this?

@tj-devel709
Copy link
Member Author

Any progress on this?

Back on it now :)

@tj-devel709 tj-devel709 removed the stale Indicates a stale issue/pr and will be closed soon label Nov 21, 2023
@tj-devel709 tj-devel709 added the partner/cat 😻 this is an issue that impacts one of our partners or a customer our advisory team is engaged with label Dec 6, 2023
@andyzukunft
Copy link

Is this accessible with a nightly nuget?

Copy link
Contributor

@emaf emaf left a comment

Choose a reason for hiding this comment

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

I've been testing how swift behaves and it seems to scroll till the bottom of the control instead of the cursor position. Also, when the keyboard is up, you can't keep scrolling till the end of the scroll view so in some cases your text can end up under the keyboard.

Keyboard over text when editor is the last item + scrolling to the cursor position instead of end of the control:

Keyboard.over.text.mov

Can't scroll to last item when keyboard is opened because one of the first ones:

Scrolling.can.t.reach.last.item.mov

Swift sample:

Swift.mov

emaf
emaf previously approved these changes Dec 19, 2023
Copy link
Contributor

@emaf emaf left a comment

Choose a reason for hiding this comment

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

I'm approving after chatting with @PureWeen and @tj-devel709. Both issues I mentioned are known and will be tackled in a separate PR as this one contains great fixes and improvements we want to get merged!

@NielsKnaap
Copy link
Contributor

@tj-devel709 is there any progress on this PR?

@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).

@PureWeen PureWeen merged commit f0353d1 into main Jan 12, 2024
47 checks passed
@PureWeen PureWeen deleted the dev/TJ/KeyboardScrolling-Improvements1 branch January 12, 2024 13:42
@rjhind
Copy link

rjhind commented Jan 13, 2024

Will this make it in to the next bug fix release of Maui?

@MartyIX
Copy link
Contributor

MartyIX commented Jan 13, 2024

Will this make it in to the next bug fix release of Maui?

The next one is .NET 8 Service Release 1 (SR1) and that one is IMHO finished but not yet released. So my guess is that this can be in SR2 (see).

The MAUI team can tell for sure though.

rmarinho added a commit that referenced this pull request Jan 15, 2024
* Fix issue 18110

* Fix SetProgressViewOffset on SwipeRefreshLayout (#19767)

* Fix SetProgressViewOffset on SwipeRefreshLayout

* - update refresh icon to a better color to trigger screen shot comparison

* - remove extra code

* - add better exception

* Fix incorrect defaults when adding non-BindableObject as AbsoluteLayout child (#19839)

* [Windows] Show the Raw HTML for Html Label in case of error (#17470)

* Show the Raw HTML in case of error

* Try to add UITest for Broken HTML

* added optional ReceiveSms permission (#19659)

* iOS Keyboard Scrolling Improvements and UITests (not from fork) (#17670)

* Squash all the iOS Keyboard Improvements

* delay was enough manually, but not for tests

* remove debugging

* Add logic for Disconnect to be more useful

* fix spacing from github merge conflict commit

* accidently removed a var declaration

* Improve UITests and add double scroll for LargeTitles

* Add UITest for the grid star

* Style changes

* more style

* add fixtureTeardown

* only check for LargeTitles in portrait mode

* Add changes for LargeTitles on iPad

* remove the popover code until we get tests

* [ci] Bump XCode to 15.2.0 (#19836)

* [ci] Bump XCode to 15.1.0

* Go to Xcode 15.2

* New xcode 15.1 doesn't support 14.5 simulators

* Add missing AbsoluteLayout section to layout architecture doc (#19841)

* [ci] Don't install xharness globally (#19863)

* dont install xharness globally

* Fix restore

* Update README.md (#19797)

* [xamlg] improve error for Xamarin.Forms namespace (#19683)

* [xamlg] improve error for Xamarin.Forms namespace

Fixes: #18637

Using the old namespace `http://xamarin.com/schemas/2014/forms` was
causing a NRE:

    System.NullReferenceException: Object reference not set to an instance of an object.
    at Microsoft.Maui.Controls.Xaml.XmlTypeXamlExtensions.GetTypeReference[T](XmlType xmlType, IEnumerable`1 xmlnsDefinitions, String defaultAssemblyName, Func`2 refFromTypeInfo)
    at Microsoft.Maui.Controls.SourceGen.CodeBehindGenerator.GetTypeNameFromCustomNamespace(XmlType xmlType, Compilation compilation, AssemblyCaches caches)
    at Microsoft.Maui.Controls.SourceGen.CodeBehindGenerator.GetTypeName(XmlType xmlType, Compilation compilation, AssemblyCaches caches)
    at Microsoft.Maui.Controls.SourceGen.CodeBehindGenerator.TryParseXaml(SourceText text, String uid, Compilation compilation, AssemblyCaches caches, CancellationToken cancellationToken, String targetFramework, String& accessModifier, String& rootType, String& rootClrNamespace, Boolean& generateDefaultCtor, Boolean& addXamlCompilationAttribute, Boolean& hideFromIntellisense, Boolean& xamlResourceIdOnly, String& baseType, IEnumerable`1& namedFields, Exception& exception)
    at Microsoft.Maui.Controls.SourceGen.CodeBehindGenerator.GenerateXamlCodeBehind(ProjectItem projItem, Compilation compilation, SourceProductionContext context, AssemblyCaches caches)
    at Microsoft.Maui.Controls.SourceGen.CodeBehindGenerator.<>c.<Initialize>b__1_1(SourceProductionContext sourceProductionContext, ValueTuple`3 provider)
    at Microsoft.CodeAnalysis.UserFunctionExtensions.<>c__DisplayClass3_0`2.<WrapUserAction>b__0(TInput1 input1, TInput2 input2, CancellationToken token)

In f939049, I inadvertently introduced this NRE, but even after fixing
it, I would instead get:

    FormsNamespace.xaml.sg.cs(15,48): error CS1001: Identifier expected

Due to the namespace, type, and base type all being `null` in this case,
the generated C# is invalid.

Instead of generating C# *at all*, let's emit an error message that says
to use the correct namespace.

I also provided a `Location` for the error message, so we will now know
what file the error originated from.

I attempted to write some test here, but adding an invalid file like
this causes the XAML unit tests project to fail to build. It feels like
we actually need to create a unit test project for the source generator
-- as there isn't one now.

* Add `[Obsolete]` to `FormsUri`

---------

Co-authored-by: Mike Parker <mikeparker104>
Co-authored-by: Shane Neuville <[email protected]>
Co-authored-by: E.Z. Hart <[email protected]>
Co-authored-by: Tim Miller <[email protected]>
Co-authored-by: Mike Parker <[email protected]>
Co-authored-by: Georgy <[email protected]>
Co-authored-by: TJ Lambert <[email protected]>
Co-authored-by: James Montemagno <[email protected]>
Co-authored-by: Jonathan Peppers <[email protected]>
@samhouts samhouts removed the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label Jan 31, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Mar 2, 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.
Labels
area-controls-entry Entry area-keyboard Keyboard, soft keyboard fixed-in-8.0.7 fixed-in-9.0.100-preview.1.9973 partner/cat 😻 this is an issue that impacts one of our partners or a customer our advisory team is engaged with platform/iOS 🍎
Projects
None yet
10 participants