superlistapp / super_editor

A Flutter toolkit for building document editors and readers
https://superlist.com/SuperEditor/
MIT License
1.58k stars 230 forks source link

[SuperEdiror][Android] Selection Handle touch area is too small #2075

Open BazinC opened 3 weeks ago

BazinC commented 3 weeks ago

Package Version stable

See the difference of touch area between the native selection handle and the SuperEditor ones. Native message app, Android simulator:

https://github.com/superlistapp/super_editor/assets/6229343/7be996bb-553f-4a47-bedf-f71010501770

SuperEditor demo app, Android Simulator:

https://github.com/superlistapp/super_editor/assets/6229343/9330bdac-dd49-4849-9f24-85240101cc5d

I heard users complaining about selection and I think this issue can explain some of their frustration.

BazinC commented 3 weeks ago

For some reason I don't feel the same when using iOS. I tried to troubleshoot the issue.

From what I understand, Android and iOS have 2 totally different ways of managing touch gestures.

On ios, IosDocumentTouchInteractor listens to gestures on the whole Document via its RawGestureDetector, tests if user tapped on a handle, remembers which handle, then onUpdate it will update the editor selection accordingly. It is not responsible to draw the drag handle. It mimics touch area empirically with:

  bool _isOverBaseHandle(Offset interactorOffset) {
    final basePosition = widget.selection.value?.base;
    if (basePosition == null) {
      return false;
    }

    final baseRect = _docLayout.getRectForPosition(basePosition)!;
    // The following caretRect offset and size were chosen empirically, based
    // on trying to drag the handle from various locations near the handle.
    final caretRect = Rect.fromLTWH(baseRect.left - 24, baseRect.top - 24, 48, baseRect.height + 48);

    final docOffset = _interactorOffsetToDocumentOffset(interactorOffset);
    return caretRect.contains(docOffset);
  }

On Android, AndroidDocumentTouchInteractor doesn't take care of the handles touch. Instead SuperEditorAndroidControlsOverlayManager draws handles with 2 AndroidSelectionHandle and wrap them with a GestureDetector to react to gestures and update the editor selection. Ex with Android upstream handle:

return Follower.withOffset(
            link: _controlsController!.upstreamHandleFocalPoint,
            leaderAnchor: Alignment.bottomLeft,
            followerAnchor: Alignment.topRight,
            child: GestureDetector(
              onTapDown: (_) {
                // Register tap down to win gesture arena ASAP.
                // final caretRect = Rect.fromLTWH(baseRect.left - 24, baseRect.top - 24, 48, baseRect.height + 48);
              },
              onPanStart: (details) => _onHandlePanStart(details, HandleType.upstream),
              onPanUpdate: _onHandlePanUpdate,
              onPanEnd: (details) => _onHandlePanEnd(details, HandleType.upstream),
              onPanCancel: () => _onHandlePanCancel(HandleType.upstream),
              dragStartBehavior: DragStartBehavior.down,
              child: AndroidSelectionHandle(
                key: DocumentKeys.upstreamHandle,
                handleType: HandleType.upstream,
                color: _controlsController!.controlsColor ?? Theme.of(context).primaryColor,
              ),
            ),
          );

This is where the issue happens imo. The touch area is equal to the AndroidSelectionHandle size.

Do you have any plan to unify the way Android and iOS handle gestures @matthew-carroll? I read some issues and //TODO in the code suggesting it.

BazinC commented 3 weeks ago

Another difference I notice between Android and iOS: IosHandlesDocumentLayer lays out and display the handles (IOSSelectionHandle)

AndroidHandlesDocumentLayer lays out the handles only, via Leader/link. Handles are displayed via SuperEditorAndroidControlsOverlayManager OverlayPortal. I think it leads to the issue where handles can be visible on top of the toolbar when scrolling down on Android (not a problem with iOS).

matthew-carroll commented 3 weeks ago

I heard users complaining about selection and I think this issue can explain some of their frustration.

What were the specific complaints?

matthew-carroll commented 3 weeks ago

I think it leads to the issue where handles can be visible on top of the toolbar when scrolling down on Android (not a problem with iOS).

What problem is this? Is there already an issue filed for this?

BazinC commented 3 weeks ago

I heard users complaining about selection and I think this issue can explain some of their frustration.

What were the specific complaints?

Selection on Android are difficult to interact with. I didn't get what would be the problem at first, I was using iOS mostly, and it feels ok. If you use an Android device and compare super editor vs native, you should feel the difference. I am convinced it is because the touch area is too small, as described above.

BazinC commented 3 weeks ago

I think it leads to the issue where handles can be visible on top of the toolbar when scrolling down on Android (not a problem with iOS).

What problem is this? Is there already an issue filed for this?

I found a couple of bug already filled related to overlay issue, like this one https://github.com/superlistapp/super_editor/issues/893 or this one https://github.com/superlistapp/super_editor/issues/1204

The problem persists today with Android handle s(not caret) because it style uses overlay instead of the document layer. I can fill another issue if you want.