fix: selection is wrong after pressing arrow left / right

Author: LucasXu0

frontend/app_flowy/packages/appflowy_editor/lib/src/infra/flowy_svg.dart

@@ -5,18 +5,23 @@ class FlowySvg extends StatelessWidget {
   const FlowySvg({
     Key? key,
     this.name,
-    this.size = const Size(20, 20),
+    this.width,
+    this.height,
     this.color,
     this.number,
     this.padding,
   }) : super(key: key);
 
   final String? name;
-  final Size size;
+  final double? width;
+  final double? height;
   final Color? color;
   final int? number;
   final EdgeInsets? padding;
 
+  final _defaultWidth = 20.0;
+  final _defaultHeight = 20.0;
+
   @override
   Widget build(BuildContext context) {
     return Padding(
@@ -27,22 +32,21 @@ class FlowySvg extends StatelessWidget {
 
   Widget _buildSvg() {
     if (name != null) {
-      return SizedBox.fromSize(
-        size: size,
-        child: SvgPicture.asset(
-          'assets/images/$name.svg',
-          color: color,
-          package: 'appflowy_editor',
-          fit: BoxFit.fill,
-        ),
+      return SvgPicture.asset(
+        'assets/images/$name.svg',
+        color: color,
+        fit: BoxFit.fill,
+        height: height,
+        width: width,
+        package: 'appflowy_editor',
       );
     } else if (number != null) {
       final numberText =
           '<svg width="200" height="200" xmlns="http://www.w3.org/2000/svg"><text x="30" y="150" fill="black" font-size="160">$number.</text></svg>';
       return SvgPicture.string(
         numberText,
-        width: size.width,
-        height: size.width,
+        width: width ?? _defaultWidth,
+        height: height ?? _defaultHeight,
       );
     }
     return Container();

frontend/app_flowy/packages/appflowy_editor/lib/src/render/rich_text/bulleted_list_text.dart

@@ -47,7 +47,7 @@ class _BulletedListTextNodeWidgetState extends State<BulletedListTextNodeWidget>
   final iconKey = GlobalKey();
 
   final _richTextKey = GlobalKey(debugLabel: 'bulleted_list_text');
-  final _iconSize = 20.0;
+  final _iconWidth = 20.0;
   final _iconRightPadding = 5.0;
 
   @override
@@ -67,7 +67,8 @@ class _BulletedListTextNodeWidgetState extends State<BulletedListTextNodeWidget>
           children: [
             FlowySvg(
               key: iconKey,
-              size: Size.square(_iconSize),
+              width: _iconWidth,
+              height: _iconWidth,
               padding:
                   EdgeInsets.only(top: topPadding, right: _iconRightPadding),
               name: 'point',

frontend/app_flowy/packages/appflowy_editor/lib/src/render/rich_text/checkbox_text.dart

@@ -45,7 +45,7 @@ class _CheckboxNodeWidgetState extends State<CheckboxNodeWidget>
   final iconKey = GlobalKey();
 
   final _richTextKey = GlobalKey(debugLabel: 'checkbox_text');
-  final _iconSize = 20.0;
+  final _iconWidth = 20.0;
   final _iconRightPadding = 5.0;
 
   @override
@@ -74,7 +74,8 @@ class _CheckboxNodeWidgetState extends State<CheckboxNodeWidget>
             GestureDetector(
               key: iconKey,
               child: FlowySvg(
-                size: Size.square(_iconSize),
+                width: _iconWidth,
+                height: _iconWidth,
                 padding: EdgeInsets.only(
                   top: topPadding,
                   right: _iconRightPadding,

frontend/app_flowy/packages/appflowy_editor/lib/src/render/rich_text/number_list_text.dart

@@ -47,7 +47,7 @@ class _NumberListTextNodeWidgetState extends State<NumberListTextNodeWidget>
   final iconKey = GlobalKey();
 
   final _richTextKey = GlobalKey(debugLabel: 'number_list_text');
-  final _iconSize = 20.0;
+  final _iconWidth = 20.0;
   final _iconRightPadding = 5.0;
 
   @override
@@ -66,7 +66,8 @@ class _NumberListTextNodeWidgetState extends State<NumberListTextNodeWidget>
             children: [
               FlowySvg(
                 key: iconKey,
-                size: Size.square(_iconSize),
+                width: _iconWidth,
+                height: _iconWidth,
                 padding:
                     EdgeInsets.only(top: topPadding, right: _iconRightPadding),
                 number: widget.textNode.attributes.number,

frontend/app_flowy/packages/appflowy_editor/lib/src/render/rich_text/quoted_text.dart

@@ -46,7 +46,7 @@ class _QuotedTextNodeWidgetState extends State<QuotedTextNodeWidget>
   final iconKey = GlobalKey();
 
   final _richTextKey = GlobalKey(debugLabel: 'quoted_text');
-  final _iconSize = 20.0;
+  final _iconWidth = 20.0;
   final _iconRightPadding = 5.0;
 
   @override
@@ -60,32 +60,34 @@ class _QuotedTextNodeWidgetState extends State<QuotedTextNodeWidget>
         width: defaultMaxTextNodeWidth,
         child: Padding(
           padding: EdgeInsets.only(bottom: defaultLinePadding),
-          child: Row(
-            crossAxisAlignment: CrossAxisAlignment.start,
-            children: [
-              FlowySvg(
-                key: iconKey,
-                size: Size(_iconSize, _quoteHeight),
-                padding:
-                    EdgeInsets.only(top: topPadding, right: _iconRightPadding),
-                name: 'quote',
-              ),
-              Expanded(
-                child: FlowyRichText(
-                  key: _richTextKey,
-                  placeholderText: 'Quote',
-                  textNode: widget.textNode,
-                  editorState: widget.editorState,
+          child: IntrinsicHeight(
+            child: Row(
+              crossAxisAlignment: CrossAxisAlignment.stretch,
+              children: [
+                FlowySvg(
+                  key: iconKey,
+                  width: _iconWidth,
+                  padding: EdgeInsets.only(
+                      top: topPadding, right: _iconRightPadding),
+                  name: 'quote',
                 ),
-              ),
-            ],
+                Expanded(
+                  child: FlowyRichText(
+                    key: _richTextKey,
+                    placeholderText: 'Quote',
+                    textNode: widget.textNode,
+                    editorState: widget.editorState,
+                  ),
+                ),
+              ],
+            ),
           ),
         ));
   }
 
   double get _quoteHeight {
     final lines =
         widget.textNode.toRawString().characters.where((c) => c == '\n').length;
-    return (lines + 1) * _iconSize;
+    return (lines + 1) * _iconWidth;
   }
 }

frontend/app_flowy/packages/appflowy_editor/lib/src/render/selection/toolbar_widget.dart

@@ -141,7 +141,7 @@ class _ToolbarWidgetState extends State<ToolbarWidget> with ToolbarMixin {
                   Size(toolbarHeight - (width != null ? 20 : 0), toolbarHeight),
               child: Center(
                 child: FlowySvg(
-                  size: Size(width ?? 20, 20),
+                  width: width ?? 20,
                   name: 'toolbar/$name',
                 ),
               ),

frontend/app_flowy/packages/appflowy_editor/lib/src/service/internal_key_event_handlers/arrow_keys_handler.dart

@@ -120,8 +120,9 @@ AppFlowyKeyEventHandler arrowKeysHandler = (editorState, event) {
         editorState.updateCursorSelection(Selection.collapsed(leftPosition));
       }
     } else {
-      editorState
-          .updateCursorSelection(currentSelection.collapse(atStart: true));
+      editorState.updateCursorSelection(
+        currentSelection.collapse(atStart: currentSelection.isBackward),
+      );
     }
     return KeyEventResult.handled;
   } else if (event.logicalKey == LogicalKeyboardKey.arrowRight) {
@@ -131,7 +132,9 @@ AppFlowyKeyEventHandler arrowKeysHandler = (editorState, event) {
         editorState.updateCursorSelection(Selection.collapsed(rightPosition));
       }
     } else {
-      editorState.updateCursorSelection(currentSelection.collapse());
+      editorState.updateCursorSelection(
+        currentSelection.collapse(atStart: !currentSelection.isBackward),
+      );
     }
     return KeyEventResult.handled;
   } else if (event.logicalKey == LogicalKeyboardKey.arrowUp) {

frontend/app_flowy/packages/appflowy_editor/lib/src/service/internal_key_event_handlers/slash_handler.dart

@@ -448,5 +448,6 @@ class PopupListItem {
 Widget _popupListIcon(String name) => FlowySvg(
       name: 'popup_list/$name',
       color: Colors.black,
-      size: const Size.square(18.0),
+      width: 18.0,
+      height: 18.0,
     );

frontend/app_flowy/packages/appflowy_editor/test/infra/test_raw_key_event.dart

@@ -88,6 +88,12 @@ extension on LogicalKeyboardKey {
     if (this == LogicalKeyboardKey.delete) {
       return PhysicalKeyboardKey.delete;
     }
+    if (this == LogicalKeyboardKey.arrowRight) {
+      return PhysicalKeyboardKey.arrowRight;
+    }
+    if (this == LogicalKeyboardKey.arrowLeft) {
+      return PhysicalKeyboardKey.arrowLeft;
+    }
     if (this == LogicalKeyboardKey.pageDown) {
       return PhysicalKeyboardKey.pageDown;
     }

frontend/app_flowy/packages/appflowy_editor/test/service/internal_key_event_handlers/arrow_keys_handler_test.dart

@@ -0,0 +1,84 @@
+import 'package:appflowy_editor/appflowy_editor.dart';
+import 'package:flutter/services.dart';
+import 'package:flutter_test/flutter_test.dart';
+import '../../infra/test_editor.dart';
+
+void main() async {
+  setUpAll(() {
+    TestWidgetsFlutterBinding.ensureInitialized();
+  });
+
+  group('arrow_keys_handler.dart', () {
+    testWidgets('Presses arrow right key, move the cursor from left to right',
+        (tester) async {
+      const text = 'Welcome to Appflowy 😁';
+      final editor = tester.editor
+        ..insertTextNode(text)
+        ..insertTextNode(text);
+      await editor.startTesting();
+
+      await editor.updateSelection(
+        Selection.single(path: [0], startOffset: 0),
+      );
+
+      final textNode = editor.nodeAtPath([0]) as TextNode;
+      for (var i = 0; i < text.length; i++) {
+        await editor.pressLogicKey(LogicalKeyboardKey.arrowRight);
+
+        if (i == text.length - 1) {
+          // Wrap to next node if the cursor is at the end of the current node.
+          expect(
+            editor.documentSelection,
+            Selection.single(
+              path: [1],
+              startOffset: 0,
+            ),
+          );
+        } else {
+          expect(
+            editor.documentSelection,
+            Selection.single(
+              path: [0],
+              startOffset: textNode.delta.nextRunePosition(i),
+            ),
+          );
+        }
+      }
+    });
+  });
+
+  testWidgets(
+      'Presses arrow left/right key since selection is not collapsed and backward',
+      (tester) async {
+    await _testPressArrowKeyInNotCollapsedSelection(tester, true);
+  });
+
+  testWidgets(
+      'Presses arrow left/right key since selection is not collapsed and forward',
+      (tester) async {
+    await _testPressArrowKeyInNotCollapsedSelection(tester, false);
+  });
+}
+
+Future<void> _testPressArrowKeyInNotCollapsedSelection(
+    WidgetTester tester, bool isBackward) async {
+  const text = 'Welcome to Appflowy 😁';
+  final editor = tester.editor
+    ..insertTextNode(text)
+    ..insertTextNode(text);
+  await editor.startTesting();
+
+  final start = Position(path: [0], offset: 5);
+  final end = Position(path: [1], offset: 10);
+  final selection = Selection(
+    start: isBackward ? start : end,
+    end: isBackward ? end : start,
+  );
+  await editor.updateSelection(selection);
+  await editor.pressLogicKey(LogicalKeyboardKey.arrowLeft);
+  expect(editor.documentSelection?.start, start);
+
+  await editor.updateSelection(selection);
+  await editor.pressLogicKey(LogicalKeyboardKey.arrowRight);
+  expect(editor.documentSelection?.end, end);
+}