Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
BazinC opened this issue Jun 7, 2024 · 6 comments · May be fixed by #2079
Open

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

BazinC opened this issue Jun 7, 2024 · 6 comments · May be fixed by #2079
Assignees
Labels
area_super_reader Related to SuperReader area_supereditor Pertains to SuperEditor area_supertextfield Pertains to SuperTextField bounty_junior f:clickup platform_android Applies to use on Android time:2 type_bug Something isn't working

Comments

@BazinC
Copy link
Contributor

BazinC commented Jun 7, 2024

Package Version
stable

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

native.mov

SuperEditor demo app, Android Simulator:

supereditor.mov

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

@BazinC BazinC added the type_bug Something isn't working label Jun 7, 2024
@BazinC
Copy link
Contributor Author

BazinC commented Jun 7, 2024

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
Copy link
Contributor Author

BazinC commented Jun 7, 2024

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
Copy link
Contributor

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

What were the specific complaints?

@matthew-carroll
Copy link
Contributor

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
Copy link
Contributor Author

BazinC commented Jun 7, 2024

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
Copy link
Contributor Author

BazinC commented Jun 8, 2024

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 #893 or this one #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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area_super_reader Related to SuperReader area_supereditor Pertains to SuperEditor area_supertextfield Pertains to SuperTextField bounty_junior f:clickup platform_android Applies to use on Android time:2 type_bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants