-
Notifications
You must be signed in to change notification settings - Fork 862
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
Conversation
There was a problem hiding this 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.
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. |
There was a problem hiding this comment.
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?
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; |
There was a problem hiding this comment.
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.
// 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); | ||
// } | ||
} |
There was a problem hiding this comment.
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.
This reverts commit 2937dc8.
This reverts commit 2937dc8.
* 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
Description
Related Issues
Fix #2124
Type of Change
Suggestions