Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Fix selection in readOnly mode, Add magnifier via RawMagnifier widget#2529

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
EchoEllet merged 13 commits intosingerdmx:masterfrommtallenca:selection_updates
Apr 23, 2025

Conversation

@mtallenca
Copy link
Contributor

@mtallencamtallenca commentedApr 2, 2025
edited by EchoEllet
Loading

Description

  1. Allow selection in read only mode
  2. Displays magnifier when dragging on iOS/Android

Related Issues

Type of Change

  • Feature: New functionality without breaking existing features.
  • 🛠️Bug fix: Resolves an issue without altering current behavior.

@CatHood0
Copy link
Collaborator

From what I see, this restores the magnifier behavior, right?

We've had issues with this implementation before, and it led to some problems during rollbacks.

For the reasons mentioned above, it's best to test this PR until we know it won't cause us problems, like it did last time.

In any case, thank you for taking the time and effort to make these changes.

EchoEllet reacted with thumbs up emoji

@mtallenca
Copy link
ContributorAuthor

This does restore magnifier behavior. I did not create the previous magnifier implementation, but I did fix some of it's bugs that had been merged. However, this is a completely different implementation that is using Flutter'sRawMagnifier widget in the Widget tree (see text_selection.dart) instead of changing events / config / etc throughout the editor. The other dart changes in this PR are just passing a ValueNotifier around, and that ValueNotifier is only used in text_selection.dart.

(This PR also has some selection fixes when in read only mode)

I agree needs to be tested. I have an internal team of 8, 1.2 million app users creating just over 10K notes a day. They have been running the 10.8.5 version that had my previous magnifier fixes. Prior to the fixes, we received several support tickets about the magnifier bugs, since the fixes no reports.

After we finish internal testing, we will do a rollout to our users and the feature will be used there. We have finished updating to flutter_quill 11.x and require the selection and magnification changes.

EchoEllet, CatHood0, and jcbjs reacted with thumbs up emoji

@EchoElletEchoEllet changed the titleAllow selection in readonly mode, added magnifier via RawMagnifier widgetFix selection in readOnly mode, Add magnifier via RawMagnifier widgetApr 8, 2025
@EchoEllet
Copy link
Collaborator

EchoEllet commentedApr 8, 2025
edited
Loading

As a minor note, we should update the v11 migration guide to inform clients that the magnifier has been restored in version X afterit was removed. This is not part of this PR since it should be done after releasing the new version. Done incba1788.

realth000 added a commit to realth000/tsdm_client that referenced this pull requestApr 17, 2025
This reverts commit45844a3.Reasons to revert:* For full text selectable readonly editor contents, `SelectableText`  only works on `TextSpan` children. `WidgetSpan` will make a gap for  text selection and implementing a `SelectableWidgetSpan` combines both  kinds of span is not easy to do: assert failure everywhere.* `SelectableArea` handles `onTap` event where we only want  the gesture passed to children so that users could activate reply  state by a single tap, to fix the issue we need a custom ignore pointer  widget.* Rendered style not consists comparing the editor when editing and  viewing page contents because the later one uses custom html parser  while the former one uses editor which renders custom bbcode tags, they  are not the same, both in history and in the future, as long as the  process differs. And the html parser is quite limited and not stable,  endless html layout rules are not clearly implemented, especially in  line break. Frequently used layout are supported but not always be,  massive special logic in the custom html parser makes it hard to  maintain, let alone the selectable text feature we want to land in a  future version. I think it's time to sunset the current html parser,  or at least render into quill_delta so that we can have consistent  contents appearance when editing and viewing contents.Next step:* Other implementation may not work, like use `Wrap` to simulate the  original rich text features in flutter SDK, I don't think the result  could be better than the one in SDK.* We need selectable text and consistent appearance when editing and  viewing contents so render it in the bbcode editor is the best choice.  Waiting for upstream fix selection in readOnly mode here:singerdmx/flutter-quill#2529  After that, parse supported tags here:https://github.com/realth000/tsdm_client/blob/81395ca8f5f5ee9ebe63cff1d223f585a895bf9a/lib/utils/html/html_muncher.dart#L335  into custom bbcode tags to let the bbcode editor render it as common  quill_delta tags, hard but worth it.
@EchoEllet
Copy link
Collaborator

Sorry for the delay, is the fix of selection in readOnly mode related to the PR#2488?

});
}

controller.addListener(_didChangeTextEditingValueListener);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

It's looks like this PRremoves the lines added by #2488 but keet some of the other changes, does this PR handle any related issues or conflicts with that PR? If no, then we need to revert that PR first, then merge this but revert the selection fix changes.

@mtallenca
Copy link
ContributorAuthor

Read only mode needs the listener for selection changes. Only changes I see in my branch is

controller.addListener(_didChangeTextEditingValueListener); and controller.removeListener(_didChangeTextEditingValueListener);

are called in readonly mode.

#2488 moves the handle focus change listener inside if (!widget.config.readOnly) {} - Could that have been the fix for focus changes bug?

@mtallenca
Copy link
ContributorAuthor

FYI - we've been internal testing with 20+ daily users for a couple of weeks with no issues.

EchoEllet reacted with thumbs up emoji

@EchoElletEchoEllet merged commitbfd23f1 intosingerdmx:masterApr 23, 2025
7 checks passed
@EchoEllet
Copy link
Collaborator

Thanks for your contribution.

EchoEllet added a commit that referenced this pull requestApr 24, 2025
@md-rifatkhan
Copy link
Contributor

The default Android apps magnifier stays fixed to the text selection line, but this one moves freely with the finger's position.
and if the magnifier is above of the editor body, then text doesn't show anymore in magnifier.

wangjinshan pushed a commit to wangjinshan/flutter-quill that referenced this pull requestApr 29, 2025
* master: (83 commits)  Add copyWith methods to DefaultStyles, HorizontalSpacing, and VerticalSpacing (singerdmx#2550)  docs: update migration guide to relfectsingerdmx#2529  chore(release): prepare to publish 11.4.0  feat: allow to override link validation check, and accept mailto and other links by default (singerdmx#2525)  chore(release): prepare to publish 11.3.0  Fix selection in readOnly mode, Add magnifier via RawMagnifier widget (singerdmx#2529)  Clarify platform support of QuillEditorConfig.onKeyPressed (singerdmx#2542)  chore(release): prepare to publish 11.2.0  feat: cache for toPlainText in Document to avoid unnecessary text computing (singerdmx#2482)  chore(example): update deps  chore(release): prepare to publish 11.1.2  fix: QuillEditor doesn't respect the system keyboard brightness by default on iOS (singerdmx#2522)  chore(release): prepare to publish flutter_quill_test  chore(release): prepare to publish 11.1.1  Chore: general improvements in flutter_quill_tests API (singerdmx#2512)  Explicitly schedule frame on secondary click. (singerdmx#2507)  chore(release): prepare to publish 11.1.0  fix: unpredictable endless loop of '_handleFocusChanged' in the phase of page route changing when editor is set to readonly. (singerdmx#2488)  feat: Enable BoxDecoration for DefaultTextBlockStyle of header Attribute (singerdmx#2438)  Expose Rule type so that Document.setCustomRules can be used (singerdmx#2484)  ...
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@EchoElletEchoElletEchoEllet left review comments

@CatHood0CatHood0Awaiting requested review from CatHood0

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Can't select text when readOnly to true Text magnification feature for the Quill Editor

4 participants

@mtallenca@CatHood0@EchoEllet@md-rifatkhan

[8]ページ先頭

©2009-2025 Movatter.jp