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

Long Tap Selection Doesn't Trigger Selection (also Selection Handles) in iOS #2391

Open
1 task done
ataberkw opened this issue Nov 21, 2024 · 5 comments
Open
1 task done
Labels
bug Something isn't working moderate Issues that are important for improving functionality or user experience.

Comments

@ataberkw
Copy link

ataberkw commented Nov 21, 2024

Is there an existing issue for this?

Flutter Quill version

10.8.5, also 11.0.0-dev.13

Steps to reproduce

Build a basic QuillEditor. Then build it on iOS, double tap to select works good, also triggers handles. But long tap to select will only show magnifier and copy, select all options after you end selecting. Same behavior in iPad. Mouse selection works good on iPad.

This is how my QuillEditor looks like:

      final json = jsonDecode(widget.data) as Json;
      final ops = (json['ops'] as List<dynamic>).map((e) => e as Json).toList();
      final operations =
          ops.map(Operation.fromJson).map(QuillUtils.mapColor).toList();

      _controller = QuillController(
        document: Document.fromDelta(Delta.fromOperations(operations)),
        selection: const TextSelection.collapsed(offset: 0),
        readOnly: true,
      );

DefaultTextStyle(
      style: GlobalUIData.defaultStyle!.copyWith(color: widget.textColor),
      child: QuillEditor(
        focusNode: FocusNode(),
        controller: _controller!,
        scrollController: ScrollController(),
        config: QuillEditorConfig(
          textSelectionControls: materialTextSelectionControls,
          textSelectionThemeData: TextSelectionThemeData(
            cursorColor: textSelectionTheme2.cursorColor,
            selectionColor: textSelectionTheme2.selectionColor,
            selectionHandleColor: textSelectionTheme2.selectionHandleColor,
          ),
          enableInteractiveSelection: true,
          showCursor: false,
          autoFocus: false,
          embedBuilders: [
            CustomFormulaEmbedBuilder(widget.textColor),
            CustomImageEmbedBuilder(onImageTap: widget.onImageTap),
            VideoEmbedBuilder(onVideoInit: null),
            CodeHighlightingEmbedBuilder(),
          ],
          expands: false,
        ),
      ),
    )

Expected results

Everything is good on Android.

Recording_2024-11-21-14-36-45.mp4

Actual results

I double tap at 00:15

Untitled.mp4

Additional Context

When I long tap to select, it selects the text as collapsed

TextSelection.collapsed(offset: 244, affinity: TextAffinity.upstream, isDirectional: false)

@ataberkw ataberkw added the bug Something isn't working label Nov 21, 2024
@EchoEllet EchoEllet added the moderate Issues that are important for improving functionality or user experience. label Nov 21, 2024
@EchoEllet
Copy link
Collaborator

Can you confirm if this issue exists on older versions (before the magnifier feature #2026)?

flutter_quill: 9.5.23

@ataberkw
Copy link
Author

ataberkw commented Nov 21, 2024

As I checked the code, this is because when device is iOS the library coded to not select in range when long tapped. It just pops up magnifier. But at the end of selection, it asks to copy or select all. Which doesn't seem like a good flow.

I checked Cupertino behavior in other iOS apps. Long tap to select shows magnifier and also select the text in range.

lib/src/editor/editor.dart:

  @override
  void onSingleLongTapMoveUpdate(LongPressMoveUpdateDetails details) {
    if (_state.configurations.onSingleLongTapMoveUpdate != null) {
      if (renderEditor != null &&
          _state.configurations.onSingleLongTapMoveUpdate!(
            details,
            renderEditor!.getPositionForOffset,
          )) {
        return;
      }
    }
    if (!delegate.selectionEnabled) {
      return;
    }

   //  bool get isCupertino =>  {TargetPlatform.iOS, TargetPlatform.macOS}.contains(platform);
    if (Theme.of(_state.context).isCupertino) { // <== When device is iOS, this can not be changed.
      renderEditor!.selectPositionAt(
        from: details.globalPosition,
        cause: SelectionChangedCause.longPress,
      );
    } else {
      renderEditor!.selectWordsInRange(
        details.globalPosition - details.offsetFromOrigin,
        details.globalPosition,
        SelectionChangedCause.longPress,
      );
    }
    editor?.updateMagnifier(details.globalPosition);
  }

A solution may be adding a parameter to decide the long tap select whether ranged or collapsed. Or maybe whatever the better for Cupertino behavior.

@ataberkw
Copy link
Author

Can you confirm if this issue exists on older versions (before the magnifier feature #2026)?

flutter_quill: 9.5.23

My project is not compatible with older intl. So I couldn't directly test it. But it seems like the problem still was there on that version. Because it still selects position on long tap if it is iOS. Instead, i think it should select range. Am I wrong?

  @override
  void onSingleLongTapStart(LongPressStartDetails details) {
    if (_state.configurations.onSingleLongTapStart != null) {
      if (renderEditor != null &&
          _state.configurations.onSingleLongTapStart!(
            details,
            renderEditor!.getPositionForOffset,
          )) {
        return;
      }
    }

    if (delegate.selectionEnabled) {
      if (Theme.of(_state.context).isCupertino) {
        renderEditor!.selectPositionAt(
          from: details.globalPosition,
          cause: SelectionChangedCause.longPress,
        );
      } else {
        renderEditor!.selectWord(SelectionChangedCause.longPress);
        Feedback.forLongPress(_state.context);
      }
    }

    _showMagnifierIfSupported(details.globalPosition);
  }```

@EchoEllet
Copy link
Collaborator

if (Theme.of(_state.context).isCupertino)

I'm not sure about this check either, it seems this is a platform-specific check that allows the use of Theme which allows overriding the platform, leading to broken behavior, it probably should use isIOSApp instead which will be only true if running on an iOS mobile app (not browser or other platforms that uses Cupertino). I didn't change it to avoid regressions.

My project is not compatible with older intl. So I couldn't directly test it.

Use pub dependency overrides:

dependency_overrides:
  flutter_quill: 9.5.23

Or try the example with version 9.4.0:

$ git clone --depth 1 --branch v9.4.0 https://github.com/singerdmx/flutter-quill.git
$ (cd flutter-quill/example && flutter run)

If you confirm this is not an issue on older versions, we will have enough regressions to revert the magnifier feature in v11 (breaking change). We're still discussing this change though.

@ataberkw
Copy link
Author

ataberkw commented Nov 22, 2024

I can confirm it was already not working on 9.5.23.

  flutter_quill:
    dependency: "direct overridden"
    description:
      name: flutter_quill
      sha256: dbbe4190e6fdf6d4225ae89ae53f3dfe4e6d813c813889ac427b44131e8a0c71
      url: "https://pub.dev"
    source: hosted
    version: "9.5.23"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working moderate Issues that are important for improving functionality or user experience.
Projects
None yet
Development

No branches or pull requests

2 participants