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

Fixed the CollectionView scroll the content inside Editor when tapping to get focus on it #27851

Merged
merged 9 commits into from
Mar 5, 2025
Merged

Fixed the CollectionView scroll the content inside Editor when tapping to get focus on it #27851

merged 9 commits into from
Mar 5, 2025
+75 −1

Conversation

NanthiniMahalingam
Copy link
Contributor

Issue Detail

  • When an editor inside a CollectionView with a footer gets focus, the text content will scroll within the editor.

Root Cause

  • When setting the collection view footer or footer template, collection view content inset value applied for collection view item of editor control. Therefore editor has been scrolled position updated when we tap on the editor.

Description of Change

  • I have ignored the bottom content inset value for CollectionView items when the editor is an inner element and maui collection view is superview.

Issues Fixed

Fixes #27766

Validated the behaviour in the following platforms

  • Android
  • Windows
  • iOS
  • Mac

Output

Before After
27766_Before.mov
27766_After.mov

Sorry, something went wrong.

@dotnet-policy-service dotnet-policy-service bot added community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration labels Feb 17, 2025
@jsuarezruiz jsuarezruiz added the area-controls-collectionview CollectionView, CarouselView, IndicatorView label Feb 17, 2025
@jsuarezruiz
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@NanthiniMahalingam NanthiniMahalingam marked this pull request as ready for review February 21, 2025 14:04
@NanthiniMahalingam NanthiniMahalingam requested a review from a team as a code owner February 21, 2025 14:04
@rmarinho
Copy link
Member

What happens if I wrap the Editor on a Grid layout? I think it will fail to find SuperView as a CollectionView

@rmarinho rmarinho added the s/pr-needs-author-input PR needs an update from the author label Feb 24, 2025
@rmarinho rmarinho self-assigned this Feb 24, 2025
@NanthiniMahalingam
Copy link
Contributor Author

What happens if I wrap the Editor on a Grid layout? I think it will fail to find SuperView as a CollectionView

Hi @rmarinho

  • When I wrap the Editor inside a Grid layout within the CollectionView item template, the SuperView remains the CollectionView by finding the nearest UIScrollView using FindResponder. Could you please verify the snapshot below?

image

image

@PureWeen PureWeen added this to the .NET 9 SR5 milestone Feb 26, 2025
tj-devel709
tj-devel709 requested changes Feb 26, 2025
View reviewed changes
Copy link
Member

@tj-devel709 tj-devel709 left a comment

Choose a reason for hiding this comment

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

I think needs a small change

src/Core/src/Platform/iOS/KeyboardAutoManagerScroll.cs Outdated
Comment on lines 818 to 823
movedInsets.Bottom = bottomInset;
// When the superview is a MauiCollectionView and the scrollView is a MauiTextView, we do not want to change the bottom inset.
bool shouldAdjustBottom = !(scrolledView is UITextView && LastScrollView is UICollectionView);
if (shouldAdjustBottom)
{
movedInsets.Bottom = bottomInset;
}
Copy link
Member

Choose a reason for hiding this comment

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

I think this is pretty close and essentially is saying to ignore the MauiTextView's bottom inset when we are inside the CollectionView. However, we do still want the MauiTextView to be able to change the movedInsets.Bottom so that we can reach the bottom of the editor when the keyboard is up.

With the following code in the MainPage.xaml.cs, you can see the following:

NotScrollingToBottom.mov

I think the below change may be better back on line 808:

		// When the superview is a MauiCollectionView and the scrollView is a MauiTextView, we do not want to consider the Bottom Inset
		// reserved for the Footer.
		bool isMauiTextViewInCV = scrolledView is UITextView && LastScrollView is UICollectionView;

		bottomInset = isMauiTextViewInCV ? bottomInset : nfloat.Max(StartingContentInsets.Bottom, bottomInset);

Then we'll get the behavior below:

ScrollingToBottom.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is pretty close and essentially is saying to ignore the MauiTextView's bottom inset when we are inside the CollectionView. However, we do still want the MauiTextView to be able to change the movedInsets.Bottom so that we can reach the bottom of the editor when the keyboard is up.

Hi @tj-devel709

  • I've refined the fix based on your suggestion, and now the editor can scroll to the bottom when the keyboard is up.

@tj-devel709
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

tj-devel709
tj-devel709 previously approved these changes Feb 27, 2025
View reviewed changes
@jsuarezruiz
Copy link
Contributor

/rebase

@jsuarezruiz
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz
Copy link
Contributor

/rebase

@jsuarezruiz
Copy link
Contributor

Rebased to include the fixes in flaky UITests from #28125

@PureWeen
Copy link
Member

PureWeen commented Mar 4, 2025

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen dismissed jsuarezruiz’s stale review March 4, 2025 19:28

changes applied

@PureWeen
Copy link
Member

PureWeen commented Mar 5, 2025

  • Failing test is from android and the failure came from trying to install appium. Nothing on the lane that failed is related to this PR

@PureWeen PureWeen merged commit 0d1159f into dotnet:main Mar 5, 2025
121 of 123 checks passed
bhavanesh2001 pushed a commit to bhavanesh2001/maui that referenced this pull request Mar 7, 2025
@NanthiniMahalingam @bhavanesh2001
Fixed the CollectionView scroll the content inside Editor when tappin…
07386b9 
…g to get focus on it (dotnet#27851)

* Fixed the editor scroll position updated when we tap on the editor inside the collection view

* Added the test case and updated the fix

* Added the output images for iOS and android platform

* Updated the fix and added the output images

* Updated the test case

* Changed the output images for iOS and android for without cursor

* Removed unwanted namespace

* Modified the fix for editor scrolling to bottom with softkeyboard

* Updated the iOS output image.
rmarinho pushed a commit that referenced this pull request Mar 11, 2025
@NanthiniMahalingam @rmarinho
Fixed the CollectionView scroll the content inside Editor when tappin…
dbbfc0d 
…g to get focus on it (#27851)

* Fixed the editor scroll position updated when we tap on the editor inside the collection view

* Added the test case and updated the fix

* Added the output images for iOS and android platform

* Updated the fix and added the output images

* Updated the test case

* Changed the output images for iOS and android for without cursor

* Removed unwanted namespace

* Modified the fix for editor scrolling to bottom with softkeyboard

* Updated the iOS output image.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PureWeen PureWeen

@jsuarezruiz jsuarezruiz

@tj-devel709 tj-devel709

@mattleibow mattleibow

@rmarinho rmarinho

Assignees

@rmarinho rmarinho

@tj-devel709 tj-devel709

Labels
area-controls-collectionview CollectionView, CarouselView, IndicatorView community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration s/pr-needs-author-input PR needs an update from the author
Projects
MAUI SDK Ongoing
Status: Done
Milestone
.NET 9 SR5
5 participants
@NanthiniMahalingam @jsuarezruiz @rmarinho @tj-devel709 @PureWeen