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

[SuperEditor][Mobile] Fix downstream image caret position (Resolves #1959) #1967

Merged
merged 7 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -313,8 +313,26 @@ class AndroidControlsDocumentLayerState
}

if (selection.isCollapsed && !_controlsController!.shouldShowExpandedHandles.value) {
Rect caretRect = documentLayout.getEdgeForPosition(selection.extent)!;

// Default caret width used by the Android caret.
const caretWidth = 2;

final layerBox = context.findRenderObject() as RenderBox?;
if (layerBox != null && layerBox.hasSize && caretRect.left + caretWidth >= layerBox.size.width) {
// Ajust the caret position to make it entirely visible because it's currently placed
// partially or entirely outside of the layers' bounds. This can happen for downstream selections
// of block components that take all the available width.
caretRect = Rect.fromLTWH(
Copy link
Contributor

Choose a reason for hiding this comment

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

@angelosilvestre, I don't think this is correct thing to do. computeLayoutDataWithDocumentLayout is called during build, but ContentLayers first calls build on layers and only then layouts them. So when you're accessing the layerBox, the box itself has not been laid out yet for the frame.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can probably get the RenderBox of the document, since it's layout is computed before the layers are built

layerBox.size.width - caretWidth,
caretRect.top,
caretRect.width,
caretRect.height,
);
}

return DocumentSelectionLayout(
caret: documentLayout.getRectForPosition(selection.extent)!,
caret: caretRect,
);
} else {
return DocumentSelectionLayout(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -675,8 +675,26 @@ class IosControlsDocumentLayerState extends DocumentLayoutLayerState<IosHandlesD
}

if (selection.isCollapsed) {
Rect caretRect = documentLayout.getEdgeForPosition(selection.extent)!;

// Default caret width used by IOSCollapsedHandle.
const caretWidth = 2;

final layerBox = context.findRenderObject() as RenderBox?;
if (layerBox != null && layerBox.hasSize && caretRect.left + caretWidth >= layerBox.size.width) {
// Ajust the caret position to make it entirely visible because it's currently placed
// partially or entirely outside of the layers' bounds. This can happen for downstream selections
// of block components that take all the available width.
caretRect = Rect.fromLTWH(
layerBox.size.width - caretWidth,
caretRect.top,
caretRect.width,
caretRect.height,
);
}

return DocumentSelectionLayout(
caret: documentLayout.getRectForPosition(selection.extent)!,
caret: caretRect,
);
} else {
return DocumentSelectionLayout(
Expand Down
2 changes: 1 addition & 1 deletion super_editor/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ dependencies:
attributed_text: ^0.3.0
characters: ^1.2.0
collection: ^1.15.0
follow_the_leader: ^0.0.4+7
follow_the_leader: ^0.0.4+8
http: ">=0.13.1 <2.0.0"
linkify: ^5.0.0
logging: ^1.0.1
Expand Down
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
66 changes: 64 additions & 2 deletions super_editor/test_goldens/editor/supereditor_caret_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,40 @@ void main() {
);
await tester.pump();

await screenMatchesGolden(tester, 'super-editor-image-caret-downstream');
await screenMatchesGolden(tester, 'super-editor-image-caret-downstream-mac');
});

testGoldensOniOS('shows caret at right side of an image', (tester) async {
await _pumpCaretTestApp(tester);

// Tap close to the right edge of the editor to place the caret
// downstream on the image.
await tester.tapAt(
tester.getTopRight(find.byType(SuperEditor)) + const Offset(-20, 20),
);
await tester.pump();

await screenMatchesGolden(tester, 'super-editor-image-caret-downstream-ios');
});

testGoldensOnAndroid(
'shows caret at right side of an image',
(tester) async {
await _pumpCaretTestApp(tester);

// Tap close to the right edge of the editor to place the caret
// downstream on the image.
await tester.tapAt(
tester.getTopRight(find.byType(SuperEditor)) + const Offset(-20, 20),
);
await tester.pumpAndSettle();

await screenMatchesGolden(tester, 'super-editor-image-caret-downstream-android');
},
// TODO: find out why this test fails on CI only.
skip: true,
);

testGoldensOnMac('shows caret at left side of an image', (tester) async {
await _pumpCaretTestApp(tester);

Expand All @@ -31,8 +62,39 @@ void main() {
);
await tester.pump();

await screenMatchesGolden(tester, 'super-editor-image-caret-upstream');
await screenMatchesGolden(tester, 'super-editor-image-caret-upstream-mac');
});

testGoldensOniOS('shows caret at left side of an image', (tester) async {
await _pumpCaretTestApp(tester);

// Tap close to the left edge of the editor to place the caret upstream
// on the image.
await tester.tapAt(
tester.getTopLeft(find.byType(SuperEditor)) + const Offset(20, 20),
);
await tester.pump();

await screenMatchesGolden(tester, 'super-editor-image-caret-upstream-ios');
});

testGoldensOnAndroid(
'shows caret at left side of an image',
(tester) async {
await _pumpCaretTestApp(tester);

// Tap close to the left edge of the editor to place the caret upstream
// on the image.
await tester.tapAt(
tester.getTopLeft(find.byType(SuperEditor)) + const Offset(20, 20),
);
await tester.pump();

await screenMatchesGolden(tester, 'super-editor-image-caret-upstream-android');
},
// TODO: find out why this test fails on CI only.
skip: true,
);
});
}

Expand Down