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

Fix iOS/Win InputTransparent layouts and add several UI tests #17286

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

mattleibow
Copy link
Member

@mattleibow mattleibow commented Sep 8, 2023

Description of Change

This PR is just adding a bunch of input transparency tests so we can be suer all the combinations are working - without the need for manual tests.

This should have been part of #14846, but we did not have any UI tests at the time. This finally makes the world of InputTransparent and CascadeInputTransparent complete.

There was also a bug in the way some input transparent controls did not allow taps to pass through because the container was not transparent. And, the layout always blocked taps - even when the controls were transparent - because it was not first checking if the controls inside should block.

@mattleibow
Copy link
Member Author

/azp run MAUI-UITests

@azure-pipelines
Copy link

azure-pipelines bot commented Sep 8, 2023

@PureWeen PureWeen requested a review from sbanni September 9, 2023 22:19
@mattleibow mattleibow marked this pull request as draft September 11, 2023 17:04
@samhouts samhouts added this to the .NET 8 GA milestone Sep 11, 2023
Comment on lines +196 to +201
#if ANDROID
// TODO: Android is broken with everything passing through
// https://github.com/dotnet/maui/issues/10252
bottom.Clicked += (s, e) => t.ViewContainer.ReportSuccessEvent();
top.Clicked += (s, e) => t.ViewContainer.ReportFailEvent();
#else
Copy link
Member Author

@mattleibow mattleibow Sep 13, 2023

Choose a reason for hiding this comment

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

Android currently is semi-broken: #10252 but fixed in #13725

Comment on lines +50 to +62
else if (Device == TestDevice.Android)
{
// TODO: Android is broken with everything passing through so we just use that
// to test the bottom button was clickable
// https://github.com/dotnet/maui/issues/10252
Assert.AreEqual($"Event: {test} (SUCCESS 1)", textAfterClick);
}
Copy link
Member Author

@mattleibow mattleibow Sep 13, 2023

Choose a reason for hiding this comment

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

Android currently is semi-broken: #10252 but fixed in #13725

Comment on lines +325 to +330
#if IOS || MACCATALYST
// Containers on iOS/Mac Catalyst may be hit testable, so we need to
// propagate the the view's values to its container view.
if (handler.ContainerView is WrapperView wrapper)
wrapper.UpdateInputTransparent(handler, view);
#endif
Copy link
Member Author

Choose a reason for hiding this comment

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

There is a change for iOS to ensure taps pass through the containers if they should.

Comment on lines +40 to +54
if (!result.UserInteractionEnabled)
{
// If the child also has user interaction disabled (IOW the child is InputTransparent),
// then we also want to exclude it from the hit testing.

return null!;
}

if (result is LayoutView layoutView && !layoutView.UserInteractionEnabledOverride)
{
// If the child is a layout then we need to check the UserInteractionEnabledOverride
// since layouts always have user interaction enabled.

return null!;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

And we need to ensure that transparent views/layouts also allow taps to pass through.

@mattleibow mattleibow changed the title Add several InputTransparent UI tests Fix iOS InputTransparent layouts and add several InputTransparent UI tests Sep 13, 2023
@mattleibow mattleibow changed the title Fix iOS InputTransparent layouts and add several InputTransparent UI tests Fix iOS/Win InputTransparent layouts and add several UI tests Sep 15, 2023
@mattleibow
Copy link
Member Author

/azp run MAUI-UITests

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

Comment on lines -126 to 134
static void MapInputTransparent(ILayoutHandler handler, ILayout layout)
public static partial void MapBackground(ILayoutHandler handler, ILayout layout)
{
if (handler.PlatformView is LayoutPanel layoutPanel && layout != null)
{
layoutPanel.UpdatePlatformViewBackground(layout);
}
handler.PlatformView?.UpdatePlatformViewBackground(layout);
}

public static partial void MapInputTransparent(ILayoutHandler handler, ILayout layout)
{
handler.PlatformView?.UpdatePlatformViewBackground(layout);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Seems Windows does IT right, but then the background just wipes it all out. Fixed this by making Background call the correct method.

@mattleibow
Copy link
Member Author

/azp run MAUI-UITests

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@mattleibow
Copy link
Member Author

/azp run MAUI-UITests

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@mattleibow
Copy link
Member Author

The failing UI tests are unrelated and are failing on main before my PR:

image

@mattleibow mattleibow force-pushed the dev/input-transparency-tests branch 2 times, most recently from e9fef23 to f5d64af Compare September 18, 2023 20:54
@samhouts samhouts modified the milestones: .NET 8 GA, Under Consideration Sep 19, 2023
@mattleibow
Copy link
Member Author

/rebase

@mattleibow mattleibow marked this pull request as ready for review September 21, 2023 09:15
Windows and iOs correctly handled transparency with regards to children, but did not allow taps to pass through
@PureWeen PureWeen merged commit c29e7b3 into main Sep 26, 2023
47 checks passed
@PureWeen PureWeen deleted the dev/input-transparency-tests branch September 26, 2023 19:17
@PureWeen
Copy link
Member

/backport to release/8.0.1xx-rc2

@github-actions
Copy link
Contributor

Started backporting to release/8.0.1xx-rc2: https://github.com/dotnet/maui/actions/runs/6628524257

@github-actions
Copy link
Contributor

@PureWeen backporting to release/8.0.1xx-rc2 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Add tests and fix Windows + iOS
Using index info to reconstruct a base tree...
M	src/Controls/samples/Controls.Sample.UITests/CoreViews/CorePageView.cs
M	src/Core/src/Platform/iOS/LayoutView.cs
M	src/Core/src/PublicAPI/net-android/PublicAPI.Unshipped.txt
Falling back to patching base and 3-way merge...
Auto-merging src/Core/src/PublicAPI/net-android/PublicAPI.Unshipped.txt
Auto-merging src/Core/src/Platform/iOS/LayoutView.cs
CONFLICT (content): Merge conflict in src/Core/src/Platform/iOS/LayoutView.cs
Auto-merging src/Controls/samples/Controls.Sample.UITests/CoreViews/CorePageView.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Add tests and fix Windows + iOS
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@github-actions
Copy link
Contributor

@PureWeen an error occurred while backporting to release/8.0.1xx-rc2, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

PureWeen pushed a commit that referenced this pull request Oct 24, 2023
Windows and iOs correctly handled transparency with regards to children, but did not allow taps to pass through
# Conflicts:
#	src/Core/src/Platform/iOS/LayoutView.cs
@github-actions github-actions bot locked and limited conversation to collaborators Dec 6, 2023
@samhouts samhouts removed this from the Under Consideration milestone Jul 1, 2024
@samhouts samhouts added the fixed-in-8.0.0-rc.2.9511 Look for this fix in 8.0.0-rc.2.9511 label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants