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

fix: incorrect cursor movement behaviour #751

Merged
merged 6 commits into from
May 2, 2024
Merged
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
166 changes: 122 additions & 44 deletions lib/src/extensions/position_extension.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import 'dart:math' as math;

import 'package:appflowy_editor/appflowy_editor.dart';
import 'package:flutter/material.dart';

Expand Down Expand Up @@ -67,66 +69,142 @@ extension PositionExtension on Position {
EditorState editorState, {
bool upwards = true,
}) {
final selection = editorState.selection;
final node = editorState.document.nodeAtPath(path);
if (node == null) {
return this;
}

final nodeRenderBox = node.renderBox;
if (nodeRenderBox == null) {
return this;
}

final nodeSelectable = node.selectable;
if (nodeSelectable == null) {
return this;
}
Xazin marked this conversation as resolved.
Show resolved Hide resolved

final editorSelection = editorState.selection;
final rects = editorState.selectionRects();
if (rects.isEmpty || selection == null) {
if (rects.isEmpty || editorSelection == null) {
return null;
}

Offset offset;
if (selection.isBackward) {
final rect = rects.reduce(
(current, next) => current.bottom >= next.bottom ? current : next,
// The offset of outermost part of the caret.
// Either the top if moving upwards, or the bottom if moving downwards.
final Offset caretOffset;

final Rect caretRect;
if (editorSelection.isBackward) {
caretRect = rects.reduce(
(current, next) => current.bottom > next.bottom ? current : next,
Xazin marked this conversation as resolved.
Show resolved Hide resolved
);
offset = upwards
? rect.topRight.translate(0, -rect.height)
: rect.centerRight.translate(0, rect.height);
caretOffset = upwards ? caretRect.topRight : caretRect.bottomRight;
} else {
final rect = rects.reduce(
caretRect = rects.reduce(
(current, next) => current.top <= next.top ? current : next,
);
offset = upwards
? rect.topLeft.translate(0, -rect.height)
: rect.centerLeft.translate(0, rect.height);
caretOffset = upwards ? caretRect.topLeft : caretRect.bottomLeft;
}

final position =
editorState.service.selectionService.getPositionInOffset(offset);

if (position != null && !position.path.equals(path)) {
return position;
final nodeConfig = editorState.service.rendererService
.blockComponentBuilder(node.type)
?.configuration;
if (nodeConfig == null) {
// This should not happen.
Xazin marked this conversation as resolved.
Show resolved Hide resolved
return this;
}

if (upwards) {
final previous = selection.start.path.previous;
if (previous.isNotEmpty && !previous.equals(selection.start.path)) {
final node = editorState.document.nodeAtPath(previous);
final selectable = node?.selectable;
var offset = selection.startIndex;
if (selectable != null) {
offset = offset.clamp(
selectable.start().offset,
selectable.end().offset,
);
return Position(path: previous, offset: offset);
}
final padding = nodeConfig.padding(node);
final nodeRect = nodeSelectable.getBlockRect();
final nodeHeight = nodeRect.height;
final textHeight = nodeHeight - padding.vertical;
final caretHeight = caretRect.height;

// Minimum (acceptable) font size
// Consider augmenting this value to increase performance.
const minFontSize = 1.0;
Xazin marked this conversation as resolved.
Show resolved Hide resolved

// If the current node is not multiline, this will be ~= 0
// so the loop will be skipped.
final remainingMultilineHeight = (textHeight - caretHeight);

// Linearly search for a new position.
// It's acceptable to use a linear search because the starting point is
// the most outer part of the caret, so:
// - If the current node is multine:
// - If the caret is NOT in the first/last line: at the first iteration
// the cycle a new position (of the previous/next multiline's line)
// will be found, practically ignoring the complexity of the cycle.
// - If the caret is in the first/last line: this is the worst case
// scenario, but only if the padding choosen by the user is very
// large. (padding >= (multiline's textHeight - caretHeight) / 3
// can start to be considered large. Note that in an average bad case
// scenario the position will be found in 10/12 ms instead of 1/2 ms)
// - If the current node is not multiline: the cycle will be completely
// skipped because `remainingMultilineHeight` would be 0.
Offset newOffset = caretOffset;
Position? newPosition;
for (var y = minFontSize;
Xazin marked this conversation as resolved.
Show resolved Hide resolved
y < remainingMultilineHeight + minFontSize;
y += minFontSize) {
newOffset = caretOffset.translate(0, upwards ? -y : y);

newPosition =
editorState.service.selectionService.getPositionInOffset(newOffset);

// If a position different from the current one is found, return it.
if (newPosition != null && newPosition != this) {
return newPosition;
}
} else {
final next = selection.end.path.next;
if (next.isNotEmpty && !next.equals(selection.end.path)) {
final node = editorState.document.nodeAtPath(next);
final selectable = node?.selectable;
var offset = selection.endIndex;
if (selectable != null) {
offset = offset.clamp(
selectable.start().offset,
selectable.end().offset,
);
return Position(path: next, offset: offset);
}
}

// If a new position has not been found, it means that the current node
// is not multiline (or the caret is in the last line of a multiline and
// the bottom padding is very large).
// In this case, we can manually skip to the previous/next node position
// by translating the new offset by the padding slice to skip.
// Note that the padding slice to skip can exceed the node's bounds.
final maxSkip = upwards ? padding.top : padding.bottom;

// Translate the new offset by the padding slice to skip.
newOffset = newOffset.translate(0, upwards ? -maxSkip : maxSkip);

// Determine node's global position.
final nodeHeightOffset = nodeRenderBox.localToGlobal(Offset(0, nodeHeight));

// Clamp the new offset to the node's bounds.
newOffset = Offset(
newOffset.dx,
math.min(newOffset.dy, nodeHeightOffset.dy),
);

newPosition =
editorState.service.selectionService.getPositionInOffset(newOffset);

if (newPosition != null && newPosition != this) {
return newPosition;
}

// If a new position has not been found, it means that the current node
// is not visible on the screen. It seems happens only if upwards is true (?)
// In this case, we can manually get the previous/next node position.
int offset = editorSelection.end.offset;
final List<int> nodePath = editorSelection.end.path;
final List<int> neighbourPath = upwards ? nodePath.previous : nodePath.next;
if (neighbourPath.isNotEmpty && !neighbourPath.equals(nodePath)) {
final neighbour = editorState.document.nodeAtPath(neighbourPath);
final selectable = neighbour?.selectable;
if (selectable != null) {
offset = offset.clamp(
selectable.start().offset,
selectable.end().offset,
);
return Position(path: neighbourPath, offset: offset);
}
}

// The cursor is already at the top or bottom of the document.
return this;
}
}
Loading