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

Copy TapAndPanGestureRecognizer from TextField #2128

Merged
merged 5 commits into from
Aug 20, 2024

Conversation

demoYang
Copy link
Contributor

@demoYang demoYang commented Aug 20, 2024

Description

  1. Copy TapAndPanGestureRecognizer from TextField;
  2. Change _keyboardVisible to hasFcouse : the mangifier will not dismiss, that the editor reponse to only long tap gesture, on ios and andriod ;
  3. Change _updateOrDisposeSelectionOverlayIfNeeded: same reason upper;

Related Issues

Fix #2124

Type of Change

  • New feature: Adds new functionality without breaking existing features.
  • 🛠️ Bug fix: Resolves an issue without altering current behavior.
  • 🧹 Code refactor: Code restructuring that does not affect behavior.
  • Breaking change: Alters existing functionality and requires updates.
  • 🧪 Tests: Adds new tests or modifies existing tests.
  • 📝 Documentation: Updates or additions to documentation.
  • 🗑️ Chore: Routine tasks, or maintenance.
  • Build configuration change: Changes to build or deploy processes.

Suggestions

Copy link
Collaborator

@EchoEllet EchoEllet left a comment

Choose a reason for hiding this comment

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

I do plan on reverting this PR due to too many regressions. Could you explain what it's fixing? It seems that it fixes a bug related to the magnifier feature, which added recently, but how does this relate? It has unrelated changes to the fix.

Comment on lines +911 to +917
case TargetPlatform.linux:
// From observation, these platform's reset their tap count to 0 when
// the number of consecutive taps exceeds 3. For example on Debian Linux
// with GTK, when going past a triple click, on the fourth click the
// selection is moved to the precise click position, on the fifth click
// the word at the position is selected, and on the sixth click the
// paragraph at the position is selected.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are those comments part of the original code?

Comment on lines +263 to +282
final isShiftPressedValid =
_isShiftPressed && renderEditor?.selection.baseOffset != null;
switch (defaultTargetPlatform) {
case TargetPlatform.android:
case TargetPlatform.fuchsia:
editor?.hideToolbar(false);
case TargetPlatform.iOS:
// On mobile platforms the selection is set on tap up.
break;
case TargetPlatform.macOS:
editor?.hideToolbar();
// On macOS, a shift-tapped unfocused field expands from 0, not from the
// previous selection.
if (isShiftPressedValid) {
final fromSelection = renderEditor?.hasFocus == true
? null
: const TextSelection.collapsed(offset: 0);
_expandSelection(
details.globalPosition, SelectionChangedCause.tap, fromSelection);
return;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know what any of this code does in this file but it seems well-written, consistent, and clean with good documentation and in Flutter repo style, copying code directly without explaining the changes is not very encouraged. Unfortunately, I have to revert.

Comment on lines +453 to +465
// Q: why ?
// A: cannot access QuillRawEditorState.updateFloatingCursor
//
// if (defaultTargetPlatform == TargetPlatform.iOS &&
// delegate.selectionEnabled &&
// editor?.textEditingValue.selection.isCollapsed == true) {
// // Update the floating cursor.
// final cursorPoint =
// RawFloatingCursorPoint(state: FloatingCursorDragState.End);
// // !.updateFloatingCursor(cursorPoint);
// (editor as QuillRawEditorState?)?.updateFloatingCursor(cursorPoint);
// }
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the question? We usually don't leave comments like way.

EchoEllet added a commit that referenced this pull request Dec 11, 2024
EchoEllet added a commit that referenced this pull request Jan 7, 2025
EchoEllet added a commit that referenced this pull request Jan 18, 2025
EchoEllet added a commit that referenced this pull request Jan 19, 2025
* revert: mobile magnifier feature (#2026)

This reverts commit af691e6.

* revert: Copying TapAndPanGestureRecognizer from TextField (#2128)

This reverts commit 2937dc8.

* docs(migration): document removal of magnifier

* chore: remove magnifierConfiguration from QuillEditorConfig

* docs(changelog): reflect the change

* chore: remove unused _replaceText() from raw_editor_state.dart as it is in editor_keyboard_shortcut_actions_manager.dart and added back due to git revert

* chore: update onTapDown and onTapUp, document the revert in CHANGELOG.md and in the migration guide
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Android] Context menu disappears during text selection
3 participants