-
Notifications
You must be signed in to change notification settings - Fork 249
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
[SuperEditor][SuperTextField][iOS] Allow caret to slide over characters (Resolves #2103) #2245
base: main
Are you sure you want to change the base?
Conversation
@matthew-carroll Could you take a look to see if I'm on the right direction here? So far I implemented this for SuperEditor only and I didn't add tests yet. |
return DocumentSelectionLayout( | ||
caret: caretRect, | ||
ghostCaret: ghostCaretRect, |
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.
Can you describe why we need to provide the ghost caret to the DocumentSelectionLayout
? Maybe the Dart Doc for DocumentSelectionLayout
isn't quite accurate, but it says "Visual layout bounds related to a user selection..." - that sounds like its just tracking geometry so that other things can also track that geometry. If so, I'm curious what other parts of Super Editor needs to track the ghost caret inside of DocumentSelectionLayout
?
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.
The DocumentSelectionLayout
is how we expose the information of where the caret should be displayed. The IosControlsDocumentLayerState
uses it to size and position the caret. Other parts of the editor won't need this.
Maybe we should create an IosDocumentSelectionLayout
to expose this additional property that only iOS uses?
_docLayout.getRectForPosition(docPositionToMagnify!)!.center, | ||
); | ||
|
||
_magnifierFocalPointInDocumentSpace.value = centerOfContentAtOffset; |
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.
Can you mention why we're making changes to magnifier stuff in a couple places in this PR?
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.
The magnifier focal point is aligned with the caret. Since now the caret isn't snapped to character edges, the magnifier should follow the same policies.
floatingCursorController: SuperEditorIosControlsScope.rootOf(context).floatingCursorController, | ||
shouldCaretBlink: controlsScope.shouldCaretBlink, | ||
floatingCursorController: controlsScope.floatingCursorController, | ||
caretDragOffset: controlsScope.caretDragOffset, |
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.
Should we de-dup this with floating cursor? I know you mentioned sharing some code over on Discord, but I wasn't sure if you already did that you wanted, or if that task still remains.
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 looked into de-duping that, but I've found it would only make things more complicated, and we wouldn't be de-duping that much code.
super_editor/lib/src/infrastructure/platforms/ios/ios_document_controls.dart
Outdated
Show resolved
Hide resolved
@@ -621,13 +642,35 @@ class IosControlsDocumentLayerState extends DocumentLayoutLayerState<IosHandlesD | |||
@visibleForTesting | |||
bool get isDownstreamHandleDisplayed => layoutData?.downstream != null; | |||
|
|||
Offset? _latestCaretDragOfffset; | |||
|
|||
/// Controls the animation of the caret moving from the position where |
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.
Please clarify the difference between "the position where the user released" and "the selected position" - how are these different? what are they, literally?
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.
Updated.
[SuperEditor][SuperTextField][iOS] Allow caret to slide over characters. Resolves #2103
Currently, as the user drags the caret on iOS, SuperEditor and SuperTextField snap the caret to the nearest legal caret position.
On native iOS, dragging the caret moves the caret by the exact drag amount in the current line of text, regardless of whether that x-offset is a legal caret position or not. When the user releases, the caret animates to the nearest legal position. Also, when the user is dragging the caret far from the text, a gray caret is displayed at the nearest legal position.
This PR implements this behavior.
Simulator.Screen.Recording.-.iPhone.15.Pro.Max.-.2024-08-14.at.22.27.06.mp4
Implementation details:
Our iOS caret is displayed in
IosHandlesDocumentLayer
. Currently, this widget doesn't have access to the position where the caret is being dragged. I added aValueListenable
so the widget can listen be notified when the user drags the caret.I added a
ghostCaret
(for the lack of a better name) inDocumentSelectionLayout
to hold the position for the gray caret.DocumentSelectionLayout
isn't specific to iOS, so this isn't ideal.