Adds custom LongPress gesture recognizer (Resolves #54)

[rutvik110]

Adds custom LongPress gesture recognizer (Resolves #54)

The Issue:

When the user executes tiny swipes (imagine a flick of approximately 10 pixels of translation distance) the viewport will seemingly stutter and start the scrolling movement with a noticeable delay of the order of 100ms.

Demo:
https://www.loom.com/share/82d9eeff4f1c4928825fcfb489c13a6c

Removing onLongPress start as a callback on the PageListViewportGestures resolves the stutter and delay issue.

The presence of the onLongPress recognizer in the Flutter gesture arena is not eliminated until the gesture exceeds a certain translation distance delta(18.0px) and/or enough time(500ms) has passed.

Gesture metrics as found here.

We would like the swiping gestures to be recognized as onScale as soon as possible to reduce the stutter and lag being noticed.

The Solution:

To achieve this, this PR adds CustomLongPressGestureRecognizer which is a LongPressGestureRecognizer but with adjusted deadline period (250ms) and preAcceptSlopTolerance(5.0) to make it loose out early in the arena and let other gestures win.

With these changes, the swiping gesture events are recognized early on as onScale to start the scrolling as soon as possible without any stutter and delay.

Demo (Tested on Ipad):

viewport_scroll.mp4

Changes:

• Adds CustomLongPressGestureRecognizer, a custom long press gesture recognizer with adjusted gesture deadline period and preAcceptSlopTolerance.
• Adds RawGestureDetector within the PageListViewportGestures and includes CustomLongPressGestureRecognizer to handle long press gestures,
  • onLongPressStart,
  • onLongPressMoveUpdate
  • onLongPressEnd.

[rutvik110]

@dodgog Could you please confirm if this resolves the stutter and lag issue you encountered?

[matthew-carroll]

@kirkbyo can you check if this PR solves the problem?

[matthew-carroll]

@rutvik110 your videos shows small pans, but it doesn't show you activating a long press. Can you show a video demonstrating that long presses are easy to use, too? For example, maybe this change makes it difficult to long press because your finger naturally moves more than 5 pixels.

[matthew-carroll]

Let's avoid naming things as "Custom" - that doesn't tell us anything about why we created this, or what it does differently.

Also, always include useful Dart Docs for public classes.

[matthew-carroll]

Let's stick constants below the class. No need to litter the top of this file with random values.

Please don't use leading "k"s, those "k"s don't tell us anything useful.

Please document each global property.

[matthew-carroll]

Is this "TODO" still valid?

[rutvik110]

No. Will be removed.

[dodgog]

@kirkbyo and I tested this PR, thank you. It looks like a huge improvement: the long tap gesture is very rarely in the way now.
We are wondering if the slop tolerance and the timer could be exposed as consumer properties with the currently set default settings? It would be great if depending on the frequency and purpose of long taps in the viewport workflow, they could also adjust the sensitivity of the arena. For example, interfaces with text selection versus ones with just a pop-up menu upon long taps should probably have different sensitivities for long tap detection.

[rutvik110]

We are wondering if the slop tolerance and the timer could be exposed as consumer properties with the currently set default settings?

@dodgog We are talking abt exposing those at PageListViewportGestures level, right? If that's the case, we could do it but I'm not sure why and when we would need to do it. Are there any particular examples you can think of where we may need to expose this to the user?
Otherwise it might only add to the confusion for the user to know how this properties will affect the long press gestures behaviour on the PageListViewport. And we'll have to rely on users making the right decision unless they know exactly how this properties affect the page viewport gestures.

[rutvik110]

It would be great if depending on the frequency and purpose of long taps in the viewport workflow, they could also adjust the sensitivity of the arena. For example, interfaces with text selection versus ones with just a pop-up menu upon long taps should probably have different sensitivities for long tap detection.

I think this needs to be there. Currently, there's an issue where the long press gesture set on PageListViewport wins every time over any other long press gesture recognizers set by other widgets within the PageListViewport.
For e.g. the white box is never able to detect the long press gesture on it. This is because, our custom long press gesture set on PageListViewport wins early as it's deadline period(250ms) is far lower than the deadline period(500ms) for default long press gesture settings in Flutter.

s_demo.mp4

We may need to introduce something within PageListViewport that would let us adjust the sensitivity of the long press gestures on the PageListViewport if user is trying to interact with other widgets in the arena who are also listening for long press gestures on them.

Probably a way to know if the render object being hit tested is related to our PageListViewport. I've something in mind which I believe should help us adjust this easily within PageListViewport. Will give it a try and update here.

[rutvik110]

@rutvik110 your videos shows small pans, but it doesn't show you activating a long press. Can you show a video demonstrating that long presses are easy to use, too? For example, maybe this change makes it difficult to long press because your finger naturally moves more than 5 pixels.

@matthew-carroll The usual long press gestures on the PageListViewport works as expected. Nothing unusual I saw there.

PXL_20230930_063751229.2.mp4

But there's this another issue that I came across which affects long press on other widgets within PageListViewport.

#56 (comment)

[matthew-carroll]

@rutvik110 it's not clear to me what the problem is that you claim you found. Can you state the problem explicitly?

[rutvik110]

Sure!

So currently the CustomLongPressGestureRecognizer we've added on PageListViewport is winning over any other long press gestures defined in the widget tree. This is due to the adjusted deadline period(250ms) for it instead of the default(500ms) that's set by Flutter for long press gestures.
With this change, we exceed the deadline period of the CustomLongPressGestureRecognizer earlier before other long press gestures, so they never get a chance to win in the arena unless we exceed the preAcceptSlopTolerance(5.0px) of the CustomLongPressGestureRecognizer before its deadline period ends.

[matthew-carroll]

@rutvik110 I see a description of things that can happen but I still don't see a clear problem statement.

What's the desired behavior? What's the actual behavior?

[rutvik110]

@matthew-carroll Lets go back to this example. The white container is one of the widgets within the PageListViewport. It has a GestureDetector wrapped around it to recognize any long press gestures on it. When I perform long press on it, usually I would expect the long press to be detected on it and its long press callback to be called.

But with the current implementation of our custom long press recognizer within the PageListViewportGestures , when you try to perform a long press on the white container, it's recognized within our custom recognizer and not on the white container.

This is because when both the gestures are in the gesture arena, our custom one wins early as it's exceeding its deadline period(250ms) before the default deadline period(500ms) set for long press gestures recognizers in Flutter. Due to this, any standard long press gestures on any other widgets never get a chance to win in the arena and thus their long press callbacks are never called.

This creates issues when we want to detect long press on any other widgets within the PageListViewport such as long press to text select or other actions.

One way to tackle this is that our custom recognizer should only recognize the long press but shouldn't go into accepted state when it exceeds it deadline. This will give other recognizers a chance to win in the arena and not block them. I think for our case, we should be fine with that. But I wanna hear from others before I go further.

PXL_20230930_063751229.2.mp4

[matthew-carroll]

Ok, the concise answer to my question is this:

Situation: a page in the viewport has a long press callback, in addition to the custom long press callback that's installed on the viewport, itself.

Expected behavior: ??? - this is actually unclear. If you have two long press callbacks, which one is supposed to win in that scenario?

Actual behavior: The viewport long-press always gets the callback because its time limit is 250ms vs standard Flutter 500ms.

[rutvik110]

So, what should we do in this case?

Do we want to detect the long press on the PageListViewport at all times or not when the long press is being performed on some other widget within the PageListViewport?

IMO, It's likely that we don't want that. If we take an example of text selection within the PageListViewport, then user is likely to drag his finger around to update the text selection, and in that case we don't want the side effect of scrolling the viewport. Same goes for any other actions that depend on long press and user dragging his finger.