[iOS] Put the UIScrollView into a UIView in a border handler by kubaflo · Pull Request #25098 · dotnet/maui

kubaflo · 2024-10-04T23:05:24Z

Description of Change

Putting UIScroll view in a UIView is necessary when applying masking which is a case with the border control. However, checking from border handler is better than checking up from UiScrollViews like in here #14096

Issues Fixed

Fixes #8923
Fixes #14091
Fixes #7524
Fixes #14091
Fixes #20736
Fixes #18486
Closes #24683

jsuarezruiz · 2024-10-07T11:01:10Z

/azp run

azure-pipelines · 2024-10-07T11:01:30Z

Azure Pipelines successfully started running 3 pipeline(s).

PureWeen · 2024-10-07T23:29:07Z

src/Core/src/Handlers/Border/BorderHandler.iOS.cs

+				// If the content is a UIScrollView, we need a container to handle masks and clip shapes effectively.
+				if (platformContent is UIScrollView && content.Handler is not null)
+				{
+					content.Handler.HasContainer = true;


I think with how HasContainer works this might be risky

Something might cause Container to reset back to false

I would be curious at some point to explore a way for the parent to flag the child as needing a container but for now I think we'll just have to add a view here and then add the content to that view.

rmarinho · 2024-10-10T10:29:08Z

/azp run

azure-pipelines · 2024-10-10T10:29:28Z

Azure Pipelines successfully started running 3 pipeline(s).

kubaflo · 2024-10-11T08:37:35Z

Seems like an unrelated test failure

maonaoda · 2024-10-29T05:07:08Z

This fix seems to cause the Border size to change strangely in certain situations (background back to the foreground, page deleted).
The Green one is the containerView
and The Red is Border`s Stroke

jsuarezruiz · 2024-11-12T14:41:25Z

/rebase

jsuarezruiz · 2024-11-12T14:42:10Z

This fix seems to cause the Border size to change strangely in certain situations (background back to the foreground, page deleted). The Green one is the containerView and The Red is Border`s Stroke

Could you share a piece of XAML or C# to reproduce your UI and add an UITest to validate it?

kubaflo · 2024-11-12T18:51:18Z

This fix seems to cause the Border size to change strangely in certain situations (background back to the foreground, page deleted). The Green one is the containerView and The Red is Border`s Stroke

As @jsuarezruiz wrote could you please share a code with us, so we can try to replicate it? Also, I've modified this PR a bit so it would be cool if you try again and let us know the outcome. Thanks!

PureWeen · 2024-11-12T19:08:20Z

/azp run

azure-pipelines · 2024-11-12T19:08:42Z

Azure Pipelines successfully started running 3 pipeline(s).

PureWeen · 2024-11-12T22:03:11Z

/azp run

azure-pipelines · 2024-11-28T16:11:19Z

Azure Pipelines successfully started running 3 pipeline(s).

PureWeen · 2024-12-04T18:28:59Z

/azp run

azure-pipelines · 2024-12-04T18:29:22Z

Azure Pipelines successfully started running 3 pipeline(s).

src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue20736.cs

PureWeen · 2024-12-07T02:54:55Z

/azp run

azure-pipelines · 2024-12-07T02:55:16Z

Azure Pipelines successfully started running 3 pipeline(s).

PureWeen · 2025-01-06T16:06:04Z

/rebase

Changes Applied

PureWeen · 2025-01-06T16:07:01Z

/azp run

azure-pipelines · 2025-01-06T16:07:24Z

Azure Pipelines successfully started running 3 pipeline(s).

PureWeen · 2025-01-06T20:32:10Z

src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue20736.cs

+		[Test]
+		[Category(UITestCategories.Editor)]
+		[Category(UITestCategories.Border)]
+		public void EditorScrollingWhenEnclosedInBorder()


It looks like this is a little bit flakey on Android

https://dev.azure.com/xamarin/public/_build/results?buildId=131616&view=ms.vss-test-web.build-test-results-tab&runId=3552379&resultId=100044&paneView=debug

Yea, it the screenshots look different on various android devices. Maybe I should disable Android for this?

PureWeen · 2025-01-07T16:51:57Z

/azp run

azure-pipelines · 2025-01-07T16:52:21Z

Azure Pipelines successfully started running 3 pipeline(s).

src/Core/src/PublicAPI/net-ios/PublicAPI.Shipped.txt

PureWeen · 2025-01-08T00:18:47Z

/azp run

azure-pipelines · 2025-01-08T00:19:11Z

Azure Pipelines successfully started running 3 pipeline(s).

Copilot

Copilot reviewed 7 out of 10 changed files in this pull request and generated no comments.

Files not reviewed (3)

src/Controls/tests/TestCases.HostApp/Issues/Issue20736.xaml: Language not supported
src/Core/src/PublicAPI/net-ios/PublicAPI.Unshipped.txt: Language not supported
src/Core/src/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt: Language not supported

Comments suppressed due to low confidence (2)

src/Core/src/Handlers/Border/BorderHandler.iOS.cs:2

The 'using System.Formats.Asn1;' directive is unnecessary and should be removed.

using System.Formats.Asn1;

src/Core/src/Handlers/ScrollView/ScrollViewHandler.iOS.cs:12

The removal of the NeedsContainer override affects the public API. This is potentially a breaking change.

public override bool NeedsContainer

PureWeen · 2025-01-09T22:11:16Z

failing tests are unrelated

kubaflo requested a review from a team as a code owner October 4, 2024 23:05

kubaflo requested review from Eilon and tj-devel709 October 4, 2024 23:05

dotnet-policy-service bot added the community ✨ Community Contribution label Oct 4, 2024

kubaflo requested review from PureWeen, jsuarezruiz and rmarinho and removed request for Eilon and tj-devel709 October 4, 2024 23:05

kubaflo mentioned this pull request Oct 4, 2024

[iOS] Fix Editor control does not scroll properly on iOS when enclosed in a Border control. #24683

Closed

4 tasks

jsuarezruiz added platform/ios area-controls-scrollview ScrollView labels Oct 7, 2024

PureWeen previously requested changes Oct 7, 2024

View reviewed changes

kubaflo force-pushed the scrollview-inside-border branch from 549a89f to ef14890 Compare October 8, 2024 16:04

kubaflo mentioned this pull request Nov 10, 2024

What is the status of the Editor control for iOS when using multi-line? #25759

Closed

kubaflo force-pushed the scrollview-inside-border branch from ef14890 to fbf3fe3 Compare November 12, 2024 13:22

github-actions bot force-pushed the scrollview-inside-border branch from fbf3fe3 to 6c5e001 Compare November 12, 2024 14:41

kubaflo force-pushed the scrollview-inside-border branch from 6c5e001 to 9b5f31b Compare November 12, 2024 19:12

PureWeen requested changes Dec 6, 2024

View reviewed changes

src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue20736.cs Show resolved Hide resolved

PureWeen modified the milestones: .NET 9 SR2, .NET 9 SR3 Dec 6, 2024

kubaflo force-pushed the scrollview-inside-border branch from 33551b3 to e7e9fa4 Compare December 7, 2024 01:06

kubaflo mentioned this pull request Dec 11, 2024

[iOS] Moved GeneralWrapperView from Controls to Core #26535

Merged

github-actions bot force-pushed the scrollview-inside-border branch from e7e9fa4 to 22ade23 Compare January 6, 2025 16:06

PureWeen requested changes Jan 6, 2025

View reviewed changes

kubaflo force-pushed the scrollview-inside-border branch from 22ade23 to e0023b4 Compare January 7, 2025 14:16

PureWeen requested changes Jan 7, 2025

View reviewed changes

src/Core/src/PublicAPI/net-ios/PublicAPI.Shipped.txt Show resolved Hide resolved

kubaflo force-pushed the scrollview-inside-border branch from cb5794f to c47c3ff Compare January 8, 2025 00:05

Put the UIScrollView into a generalWrapperView in a border handler

aa275e1

kubaflo force-pushed the scrollview-inside-border branch from c47c3ff to aa275e1 Compare January 8, 2025 00:17

PureWeen requested a review from Copilot January 8, 2025 18:13

Copilot AI reviewed Jan 8, 2025

View reviewed changes

PureWeen merged commit da0f504 into dotnet:main Jan 9, 2025

github-actions bot locked and limited conversation to collaborators Feb 9, 2025

Conversation

kubaflo commented Oct 4, 2024

Description of Change

Issues Fixed

Uh oh!

jsuarezruiz commented Oct 7, 2024

Uh oh!

azure-pipelines bot commented Oct 7, 2024

Uh oh!

PureWeen Oct 7, 2024

Choose a reason for hiding this comment

Uh oh!

rmarinho commented Oct 10, 2024

Uh oh!

azure-pipelines bot commented Oct 10, 2024

Uh oh!

kubaflo commented Oct 11, 2024

Uh oh!

maonaoda commented Oct 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jsuarezruiz commented Nov 12, 2024

Uh oh!

jsuarezruiz commented Nov 12, 2024

Uh oh!

kubaflo commented Nov 12, 2024

Uh oh!

PureWeen commented Nov 12, 2024

Uh oh!

azure-pipelines bot commented Nov 12, 2024

Uh oh!

PureWeen commented Nov 12, 2024

Uh oh!

azure-pipelines bot commented Nov 28, 2024

Uh oh!

PureWeen commented Dec 4, 2024

Uh oh!

azure-pipelines bot commented Dec 4, 2024

Uh oh!

Uh oh!

PureWeen commented Dec 7, 2024

Uh oh!

azure-pipelines bot commented Dec 7, 2024

Uh oh!

PureWeen commented Jan 6, 2025

Uh oh!

PureWeen commented Jan 6, 2025

Uh oh!

azure-pipelines bot commented Jan 6, 2025

Uh oh!

PureWeen Jan 6, 2025

Choose a reason for hiding this comment

Uh oh!

kubaflo Jan 7, 2025

Choose a reason for hiding this comment

Uh oh!

PureWeen commented Jan 7, 2025

Uh oh!

azure-pipelines bot commented Jan 7, 2025

Uh oh!

Uh oh!

PureWeen commented Jan 8, 2025

Uh oh!

azure-pipelines bot commented Jan 8, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

PureWeen commented Jan 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

maonaoda commented Oct 29, 2024 •

edited

Loading