singerdmx / flutter-quill

Rich text editor for Flutter
https://pub.dev/packages/flutter_quill
MIT License
2.6k stars 839 forks source link

[Android] Long click at the end of text selects previous text #2121

Closed stepushchik closed 3 months ago

stepushchik commented 3 months ago

Is there an existing issue for this?

Flutter Quill version

10.3.1

Steps to reproduce

  1. Add some text
  2. Select and Copy
  3. Move cursor to the end
  4. Try to Paste the text via Long click

Expected results

Text pasted at the end

Actual results

Long click selects previous text. Please see the attached screenshot.

Code sample

Code sample ```dart [Paste your code here] ```

Additional Context

Screenshots / Video demonstration ![Screenshot 2024-08-19 at 11 36 38](https://github.com/user-attachments/assets/60c7f79d-c1bd-47cb-86b7-f39865fd2a5f)
Logs ```console [Paste your logs here] ```
CatHood0 commented 3 months ago

That screenshot doesn't let me understand correctly what is the issue. If you can provide a video it would be useful. That's the expected behavior. If the both words are NOT divided by a whitespaces then, them will be counted as the same word.

stepushchik commented 3 months ago

Ok with v10.1.5, so relates to https://github.com/singerdmx/flutter-quill/pull/2086

Screenshot 2024-08-19 at 12 03 49

stepushchik commented 3 months ago

That's the expected behavior.

@CatHood0 but you can't Paste text without one whitespace now as before.

v10.1.6: And long click after whitespace selects the whitespace. Screenshot 2024-08-19 at 12 13 21

CatHood0 commented 3 months ago

@AtlasAutocode can you take a look about this issue?

AtlasAutocode commented 3 months ago

It certainly looks like PR2086 caused the problem and temporarily reverting the fix did seem to cure this issue.

I see that PR2086 was submitted to cure a problem with double clicking to select Chinese characters.

@li8607 since you submitted PR2086, and understand this more than we do, can you take a look at this issue please?

@CatHood0 if we revert the PR, it will fix this issue but break the previous purpose of the PR. What is the procedure for handling this kind of conflict?

CatHood0 commented 3 months ago

@AtlasAutocode i'll take a look. It seems more a problem with logic implemented at the PR #2086.

AtlasAutocode commented 3 months ago

@singerdmx @EchoEllet How do we handle this situation?

PR 2086 seems to be responsible for the current issue of text selection on mobile devices. Reverting the (single) change in the PR cures this problem.

We have not gotten any response from @li8607 who originated the PR to solve an issue with double click selecting of chinese text. Do we revert the PR change and reintroduce the chinese problem?

EchoEllet commented 3 months ago

How do we handle this situation?

I suggest we revert this for now. We should consider writing a test for this feature before/after fixing the bug to prevent future regressions.

Not related to the issue I’m currently unable to take this on due to time constraints and will probably not be able to do so for a while.
singerdmx commented 3 months ago

reverted

https://github.com/singerdmx/flutter-quill/commit/dcdfcea268c17df5292a0332428839931a8b620f

singerdmx commented 3 months ago

Published as https://github.com/singerdmx/flutter-quill/releases/tag/v10.4.2

AtlasAutocode commented 3 months ago

@singerdmx Thank you for reverting the change.

I was trying to add a test to prevent future regressions as suggested by @EchoEllet . I tried to create an instance of RenderEditor but it failed because of the lack of a context. So, I looked into using testWidgets (very impressive - totally blew my mind) but I could not find a way to access RenderEditor. I could access the QuillEditor, I could access EditorTextSelectionGestureDetector, but I guess RenderEditor is not a widget!

Any suggestions on how I might be able to create a test?

singerdmx commented 3 months ago

I guess current code is not constructed in a way that is friendly to testWidgets. You may have to do some surgery on the current code base to expose some methods in order to test.