[fix issue #24048] Fixed a problem, the vertical drag was not recogni…

[haeseoklee]

…zed when the child of GestureDetector is ScrollView

Description

Due to the conflict between GestureDetector and ScrollView, some functions of GestureDetector were not called.
So I fixed it. :)

Before

Performing hot restart...
Syncing files to device iPhone SE (2nd generation)...
Restarted application in 5,353ms.
flutter: Gesture arena 516  ❙ ★ Opening new gesture arena.
flutter: Gesture arena 516  ❙ Adding: VerticalDragGestureRecognizer#4b16e(start behavior: start)
flutter: Gesture arena 516  ❙ Adding: VerticalDragGestureRecognizer#3b37d(debugOwner: GestureDetector, start behavior: start)
flutter: show onVerticalDragDown
flutter: Gesture arena 516  ❙ Closing with 2 members.
flutter: Gesture arena 516  ❙ Accepting: VerticalDragGestureRecognizer#4b16e(start behavior: start)
flutter: Gesture arena 516  ❙ Self-declared winner: VerticalDragGestureRecognizer#4b16e(start behavior: start)
flutter: show onVerticalDragCancel

After

Performing hot restart...
Syncing files to device iPhone SE (2nd generation)...
Restarted application in 5,473ms.
flutter: Gesture arena 515  ❙ ★ Opening new gesture arena.
flutter: Gesture arena 515  ❙ Adding: VerticalDragGestureRecognizer#891e3(start behavior: start)
flutter: Gesture arena 515  ❙ Adding: VerticalDragGestureRecognizer#56a13(debugOwner: GestureDetector, start behavior: start)
flutter: show onVerticalDragDown
flutter: Gesture arena 515  ❙ Closing with 2 members.
flutter: Gesture arena 515  ❙ Accepting: VerticalDragGestureRecognizer#891e3(start behavior: start)
flutter: Gesture arena 515  ❙ Self-declared winner: VerticalDragGestureRecognizer#891e3(start behavior: start)
flutter: show onVerticalDragStart
flutter: show onVerticalDragUpdate
flutter: show onVerticalDragUpdate
flutter: show onVerticalDragUpdate
flutter: show onVerticalDragUpdate
flutter: show onVerticalDragUpdate
flutter: show onVerticalDragUpdate
flutter: show onVerticalDragUpdate
flutter: show onVerticalDragUpdate
flutter: show onVerticalDragUpdate
flutter: show onVerticalDragUpdate
flutter: show onVerticalDragUpdate
flutter: show onVerticalDragUpdate
flutter: show onVerticalDragEnd

Test code

import 'package:flutter/material.dart';
import 'package:flutter/gestures.dart';

void main() {
  debugPrintGestureArenaDiagnostics = true;
  runApp(MaterialApp(home: MyApp()));
}

class MyApp extends StatefulWidget {
  @override
  _MyAppState createState() => _MyAppState();
}

class _MyAppState extends State<MyApp> {
  @override
  Widget build(BuildContext context) {
    return SafeArea(
      child: Scaffold(
          body: GestureDetector(
            onVerticalDragDown: (_) {
              print("show onVerticalDragDown");
            },
            onVerticalDragStart: (_) {
              print("show onVerticalDragStart");
            },
            onVerticalDragUpdate: (_) {
              print("show onVerticalDragUpdate");
            },
            onVerticalDragEnd: (_) {
              print("show onVerticalDragEnd");
            },
            onVerticalDragCancel: () {
              print("show onVerticalDragCancel");
            },
            child: CustomScrollView(
              slivers: <Widget>[
                const SliverAppBar(
                  pinned: true,
                  expandedHeight: 250.0,
                  flexibleSpace: FlexibleSpaceBar(
                    title: Text('Demo'),
                  ),
                ),
                SliverGrid(
                  gridDelegate: SliverGridDelegateWithMaxCrossAxisExtent(
                    maxCrossAxisExtent: 200.0,
                    mainAxisSpacing: 10.0,
                    crossAxisSpacing: 10.0,
                    childAspectRatio: 4.0,
                  ),
                  delegate: SliverChildBuilderDelegate(
                        (BuildContext context, int index) {
                      return Container(
                        alignment: Alignment.center,
                        color: Colors.teal[100 * (index % 9)],
                        child: Text('Grid Item $index'),
                      );
                    },
                    childCount: 30,
                  ),
                ),
              ],
            ),
          )
      ),
    );
  }
}

Related Issues

Use GestureDetector and CustomScrollView together
Fixes #24048

Tests

Checklist

Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I signed the CLA.
• I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I updated/added relevant documentation (doc comments with ///).
• All existing and new tests are passing.
• The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
• I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

• No, no existing tests failed, so this is not a breaking change.

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[goderbauer]

This PR is causing many test failures and it is missing tests.

[haeseoklee]

@goderbauer
I added test code!! Also the rewritten code passed many test failures. Please review :)

[goderbauer]

/cc @dkwingsmt

[dkwingsmt]

Thanks for the PR. Although I'm not a fan of this solution for the following reasons:

• The name addManually is confusing. For a user who only understands gesture detectors, this name does not make much sense without forcing him to learn the low-level mechanism of the arena. Even if you explain what it does, it's hard for users to understand when it should be enabled.
• When it's enabled, the outer detector detects the drag even when the inner drag is not at its end, which violates the rule of only one gesture should be accepted (see below for more thoughts).
• It adds an option to solve a problem that feels should not have existed at all, which means it's better we think about what we did wrong (I'm sure we did) when designing the mechanism.

Correct me if I'm wrong, but I think the demand here is that the inner drag recognizer should "surrender" to the outer detector when but only when it has reached the end of the intended direction. For example, say there are 2 vertical dragging gestures, one nested in the other,

• If the inner is being dragged downwards, and it's not at the top yet, the outer should not be notified.
• If the inner is being dragged downwards, and it's at the top at the start or middle of the drag, the outer should start to take over the drag.

This mechanism has been implemented by PointerSignalResolver for mouse scrolling, and by NestedScrollView for nested scroll views, but in the case you're trying to solve, the outer widget is a detector instead of a scroll view, which means it can take advantage of neither.

We should probably either 1) try to build the demanded feature using NestedScrollView, or in case it's not possible, 2) build a mechanism that can resolve dragging the same way as PointerSignalResolver.

@Piinks Are you familiar with NestedScrollView? Do you think 1) is possible? Or if not, doing 2) will simplify/unify the implementation of NestedScrollView?

[Piinks]

Hey @dkwingsmt thanks for considering the Nested Scroll View. It appears in the issue that this can be accomplished using a NotificationListener: #24048 (comment) The issue was looking for a way to trigger a refresh after lifting their finger instead of dragging to a certain scroll offset.

[goderbauer]

I am going to close this with some of the reasons given in #63179 (comment).