Flutter-Bounty-Hunters / follow_the_leader

MIT License
13 stars 5 forks source link

Ensure that global coordinates respect offsets in `Follower`s #44

Closed matthew-carroll closed 4 months ago

matthew-carroll commented 4 months ago

I recently added an invisible tap area to Android-style mobile handles in Super Editor. When I added the invisible tap area, I had to nudge the handles by the inverse of that tap area so that the handles still aligned with the selection they were following. Something about this change lead to failures in Super Editor goldens where our rendered drag-line moved down.

What we want: mobile-selection_android_drag-extent-upstream_masterImage

Where we ended up: mobile-selection_android_drag-extent-upstream_testImage

This line is positioned by accessing a GlobalKey on the handle widget and querying the global bounds:

final dragDelta = SuperEditorInspector.findDeltaBetweenCharactersInTextNode("1", 34, 28);
final handleRectGlobal = SuperEditorInspector.findMobileCaretDragHandle().globalRect;
await tester.dragFrom(handleRectGlobal.center, dragDelta);

// Update the drag line for debug purposes
dragLine.value = _Line(handleRectGlobal.center, handleRectGlobal.center + dragDelta);

My hypothesis is that the local to global transformation, or some other coordinate mapping within Follower isn't correctly accounting for the offset property of the Follower. The handle appears visually where it should, but the calculation of the globalRect is behaving as if the offset isn't applied to the handle.

Here's the full Follower widget subtree for the Android handle:

// Note: If we pass this widget as the `child` property, it causes repeated starts and stops
        // of the pan gesture. By building it here, pan events work as expected.
        return Follower.withOffset(
          link: _controlsController!.collapsedHandleFocalPoint,
          leaderAnchor: Alignment.bottomCenter,
          followerAnchor: Alignment.topCenter,
          // Use the offset to account for the invisible expanded touch region around the handle.
          offset: -Offset(0, AndroidSelectionHandle.defaultTouchRegionExpansion.top) *
              MediaQuery.devicePixelRatioOf(context),
          child: AnimatedOpacity(
            // When the controller doesn't want the handle to be visible, hide it.
            opacity: shouldShow ? 1.0 : 0.0,
            duration: const Duration(milliseconds: 150),
            child: IgnorePointer(
              // Don't let the handle respond to touch events when the handle shouldn't
              // be visible. This is needed because we don't remove the handle from the
              // tree, we just make it invisible. In theory, invisible widgets aren't
              // supposed to be hit-testable, but in tests I found that without this
              // explicit IgnorePointer, gestures were still being captured by this handle.
              ignoring: !shouldShow,
              child: GestureDetector(
                onTapDown: (_) {
                  // Register tap down to win gesture arena ASAP.
                },
                onTap: _toggleToolbarOnCollapsedHandleTap,
                onPanStart: (details) => _onHandlePanStart(details, HandleType.collapsed),
                onPanUpdate: _onHandlePanUpdate,
                onPanEnd: (details) => _onHandlePanEnd(details, HandleType.collapsed),
                onPanCancel: () => _onHandlePanCancel(HandleType.collapsed),
                dragStartBehavior: DragStartBehavior.down,
                child: AndroidSelectionHandle(
                  key: DocumentKeys.androidCaretHandle,
                  handleType: HandleType.collapsed,
                  color: _controlsController!.controlsColor ?? Theme.of(context).primaryColor,
                ),
              ),
            ),
          ),
        );
matthew-carroll commented 4 months ago

Nope. I was wrong about the root cause. The problem is that the invisible touch area padding only adds space below the handle, not above it. As a result the "center" of the handle gets pushed further down because its an average between the height of the handle and the height of the invisible touch area beneath it. That's a Super Editor problem.