[iOS] SwipeView: Fix reenabling parent scrolling after cancelled swipe

[filipnavara]

Description of Change

Fixes an issue where scrolling of SwipeView's parent is disabled during swipe but not reenabled when the swipe is cancelled by manually moving the touch below the threshold.

Issues Fixed

Fixes #23441
Fixes #16856

[dotnet-policy-service]

Hey there @filipnavara! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[PureWeen]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[mattleibow]

I think this PR is correct, but I think this may need a nice (complicated) UI test :)

If you are feeling up to it, you can write a test emulating the human, but I can also give that a go and see who wins :)

[filipnavara]

Technically the PR is probably correct with regards to the code intention.

After few iterations on our app and with our in-house testers we ended up doing something different though. We ended up fixing all iOS gesture recognisers to correctly report back state to the system and to interact with other gesture recognisers. I’ll try to write up the details tomorrow. The issue with that approach is that it changes the behaviour for minimum swipe needed to open the SwipeView (which is desirable in our case but may not be desirable in general).

[filipnavara]

Sorry, took me a little while longer, but let's expand on my previous comment.

iOS uses the UIGestureRecognizer and its subclasses to implement the gesture recognition. These classes implement a state machine and a model for cooperation between multiple gesture recognizers in the UI hierarchy. Apple documentation is quite extension on how this should work: 1, 2.

Naturally, the preferred model for cooperation of multiple gesture recognizers (as is the case here with swiping and collection view scrolling) is to properly implement the state machines and UIGestureRecognizer[Delegate].ShouldBegin / UIGestureRecognizer[Delegate].ShouldRecognizeSimultaneously callbacks. This is particularly important on older iPhone models, such as iPhone SE (1st and 2nd gen), iPhone 6s, and possibly even models up to iPhone 11, which happen to have imprecise digitizer which gives back phantom touches. The native logic UIGestureRecognizer hides these tiny "phantom moves" from you.

So, how would implementing that look for MauiSwipeView? Pretty much like this diff:

• It removes all the IsParentScrollEnabled logic to deal with CollectionView.
• ShouldBegin recognizes horizontal swipe on the UIPanGestureRecognizer and returns true. For vertical swipe it returns false and the system automatically knows that it has to try other gesture recognizers in the hierarchy and continues with the collection view one used for scrolling.
• ShouldRecognizeSimultaneously is no longer overridden and returns the default false value. This means that for the pan gesture either the collection view scrolling gesture recognizer will trigger, or the swipe one will, not both.

Pretty simple, right? Where's the catch? Well, there are multiple catches with this approach:

• The swipe gesture is now recognized immediately when the user stars swiping, not respecting the MinimumOpenSwipeThresholdPercentage constant. It would likely be possible to support that with more advanced logic in the Should* gesture recognition logic. We opted not to do it because other iOS apps start the swiping gesture in collection view immediately, so this was a desired change in our case.
• CommunityToolkit.Maui's TouchBehavior fails to implement the UIGestureRecognizer state machine which leads to all kinds of incorrect interaction between the gesture recognizers if it's used (Correctly implement the UIGestureRecognizer state machine CommunityToolkit/Maui#2004).

I'm not quite sure how to follow up from here. We definitely should create a test, as you said. UI tests of this kind are not exactly my turf. I'd be happy to leave it up for someone else to do it, or at least point me in the right direction where a similar test already exists.

Going forward we have few options:

• Go forward with this PR since it's relatively safe way to fix an obvious bug, even if it doesn't fix every possible issue with the pan gesture interaction.
• Implement the UIGestureRecognizer cooperation logic correctly instead of the mess in IsParentScrollEnabled. This is relatively simple if we're okay with changing the "minimum swipe to open" behavior. It may be relatively complex if we decide to keep it.
• The new CollectionViewHandler2 in .NET 9 branch uses compositional layout. Compositional layout has native support for swipe actions, so long term it would make sense to explore whether SwipeView inside CollectionView can be translated to the leadingSwipeActionsConfigurationProvider/trailingSwipeActionsConfigurationProvider concepts in the compositional layout.

[mattleibow]

Wow! Thanks for that. I really like the idea for removing our hacks even if it loses the threshold stuff, but that is just me and we can't just break/change things. So I have pinged the team to make sure we have a chat on this.

[mattleibow]

With regards to the nice fix using the ShouldBegin in your diff, how does the method args fire? If you return false for a small threshold will it fire again so we can say yes later?

For example, the line is this return Math.Abs(translation.Y) <= Math.Abs(translation.X);

Can that line be update to include a threshold check and then when it is reached we return true? Or is it too late? You said

It may be relatively complex if we decide to keep it.

What makes you think that? I am no iOS gesture expert so I don't really know how they work.

[filipnavara]

With regards to the nice fix using the ShouldBegin in your diff, how does the method args fire? If you return false for a small threshold will it fire again so we can say yes later?

It only fires once, unfortunately. That means detecting a bigger threshold makes the task much more difficult since you have to start dealing with two gesture recognizers operating at the same time and then use ShouldRecognizeSimultaneously (or other mechanism) to handle the "collisions". Alternatively, it's also possible to implement the whole panning logic as a completely custom UIGestureRecognizer and simply transit the state machine between "Possible" and "Recognized" states once you made big enough swipe. I didn't really spend enough time on trying to implement this, so there may be some gotchas.

[PureWeen]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[Redth]

Tested here both before and after the changes.

It was a bit tricky but definitely possible to get things into a bad state before the changes, and I was unable to break things after testing with the changes.

Appreciate the thorough explanation of things here, and while yes it would be great to eventually do this more properly, this fixes a current issue with a minimal change.

Given the nature of trying to create a test for this (and I think anything we created could likely be inconsistent and unreliable), we should bring this change in now.