From 759544aef0581ffc68627d19a0d828d7b47dd963 Mon Sep 17 00:00:00 2001 From: Mohammad Zolfaghari Date: Mon, 18 Sep 2023 19:33:53 +0330 Subject: [PATCH 1/3] fix: new line direction base on previous or parent fix to use previous or parent direction when the editor default direction is auto. in that case use the calculated text direction of previous or parent node. --- .../base_component/text_direction_mixin.dart | 85 ++++++++++++------- .../text_direction_mixin_test.dart | 24 ++++++ test/new/infra/testable_editor.dart | 9 +- 3 files changed, 84 insertions(+), 34 deletions(-) diff --git a/lib/src/editor/block_component/base_component/text_direction_mixin.dart b/lib/src/editor/block_component/base_component/text_direction_mixin.dart index 5d930df39..c618f0439 100644 --- a/lib/src/editor/block_component/base_component/text_direction_mixin.dart +++ b/lib/src/editor/block_component/base_component/text_direction_mixin.dart @@ -76,24 +76,13 @@ TextDirection calculateNodeDirection({ } if (value == blockComponentTextDirectionAuto) { - // previous line direction - final previousNodeContainsTextDirection = - previousOrParentNodeWithTextDirection(node); - if (lastDirection != null) { defaultTextDirection = lastDirection.name; - } else if (previousNodeContainsTextDirection != null) { - final String previousValue = previousNodeContainsTextDirection - .attributes[blockComponentTextDirection]; - if (previousValue == blockComponentTextDirectionAuto) { - defaultTextDirection = previousNodeContainsTextDirection.selectable - ?.textDirection() - .name ?? - defaultTextDirection; - } else { - defaultTextDirection = - previousValue.toTextDirection()?.name ?? defaultTextDirection; - } + } else { + defaultTextDirection = + _getDirectionFromPreviousOrParentNode(node, defaultTextDirection) + ?.name ?? + defaultTextDirection; } } @@ -106,33 +95,65 @@ TextDirection calculateNodeDirection({ // if the value is auto and the text isn't null or empty, // calculate the text direction by the text - final matches = _regex.firstMatch(text); - if (matches != null) { - if (matches.group(1) != null) { - return TextDirection.rtl; - } else if (matches.group(2) != null) { - return TextDirection.ltr; - } + final direction = _determineTextDirection(text); + if (direction != null) { + return direction; } return defaultTextDirection?.toTextDirection() ?? layoutDirection; } -Node? previousOrParentNodeWithTextDirection(Node node) { - bool textDirectionCheck(node) => - node != null && - node.attributes.containsKey(blockComponentTextDirection) && - node.attributes[blockComponentTextDirection] != null; +TextDirection? _getDirectionFromPreviousOrParentNode( + Node node, + String? defaultTextDirection, +) { + TextDirection? prevOrParentNodeDirection; + if (node.previous != null) { + prevOrParentNodeDirection = + _getDirectionFromNode(node.previous!, defaultTextDirection); + } + if (node.parent != null && prevOrParentNodeDirection == null) { + prevOrParentNodeDirection = + _getDirectionFromNode(node.parent!, defaultTextDirection); + } + + return prevOrParentNodeDirection; +} + +TextDirection? _getDirectionFromNode(Node node, String? defaultTextDirection) { + String? nodeDirection; + if (defaultTextDirection == blockComponentTextDirectionAuto) { + nodeDirection = blockComponentTextDirectionAuto; + } + + if (node.attributes.containsKey(blockComponentTextDirection) && + node.attributes[blockComponentTextDirection] != null) { + nodeDirection = node.attributes[blockComponentTextDirection]; + } - if (textDirectionCheck(node.previous)) { - return node.previous; - } else if (textDirectionCheck(node.parent)) { - return node.parent; + if (nodeDirection != null) { + if (nodeDirection == blockComponentTextDirectionAuto) { + return node.selectable?.textDirection(); + } else { + return nodeDirection.toTextDirection(); + } } return null; } +TextDirection? _determineTextDirection(String text) { + final matches = _regex.firstMatch(text); + if (matches != null) { + if (matches.group(1) != null) { + return TextDirection.rtl; + } else if (matches.group(2) != null) { + return TextDirection.ltr; + } + } + return null; +} + extension on String { TextDirection? toTextDirection() { if (this == blockComponentTextDirectionLTR) { diff --git a/test/new/block_component/text_direction_mixin_test.dart b/test/new/block_component/text_direction_mixin_test.dart index 04908c7b4..9df05b57d 100644 --- a/test/new/block_component/text_direction_mixin_test.dart +++ b/test/new/block_component/text_direction_mixin_test.dart @@ -358,5 +358,29 @@ void main() { await editor.dispose(); }); + + testWidgets( + 'use previous node direction calculated value (rtl) when its set by default text direction', + (tester) async { + final editor = tester.editor + ..addNode( + paragraphNode( + text: 'سلام', + ), + ) + ..addNode( + paragraphNode( + text: '\$', + ), + ); + await editor.startTesting( + defaultTextDirection: blockComponentTextDirectionAuto, + ); + + final node = editor.nodeAtPath([1])!; + expect(node.selectable?.textDirection(), TextDirection.rtl); + + await editor.dispose(); + }); }); } diff --git a/test/new/infra/testable_editor.dart b/test/new/infra/testable_editor.dart index af24fc20a..2eca4c1c7 100644 --- a/test/new/infra/testable_editor.dart +++ b/test/new/infra/testable_editor.dart @@ -41,6 +41,7 @@ class TestableEditor { ScrollController? scrollController, Widget Function(Widget child)? wrapper, TargetPlatform? platform, + String? defaultTextDirection, }) async { await AppFlowyEditorLocalizations.load(locale); @@ -68,8 +69,12 @@ class TestableEditor { .testableFindAndReplaceCommands, ], editorStyle: inMobile - ? const EditorStyle.mobile() - : const EditorStyle.desktop(), + ? EditorStyle.mobile( + defaultTextDirection: defaultTextDirection, + ) + : EditorStyle.desktop( + defaultTextDirection: defaultTextDirection, + ), ); }, ); From f434858b1014f871502abe84e8d1220001047933 Mon Sep 17 00:00:00 2001 From: Mohammad Zolfaghari Date: Tue, 19 Sep 2023 14:53:33 +0330 Subject: [PATCH 2/3] fix: indent respect default and last direction --- .../base_component/text_direction_mixin.dart | 20 ++-- .../rich_text/default_selectable_mixin.dart | 4 +- .../renderer/block_component_widget.dart | 9 ++ .../text_direction_mixin_test.dart | 99 ++++++++++++++++++- 4 files changed, 122 insertions(+), 10 deletions(-) diff --git a/lib/src/editor/block_component/base_component/text_direction_mixin.dart b/lib/src/editor/block_component/base_component/text_direction_mixin.dart index c618f0439..901297bc5 100644 --- a/lib/src/editor/block_component/base_component/text_direction_mixin.dart +++ b/lib/src/editor/block_component/base_component/text_direction_mixin.dart @@ -18,11 +18,12 @@ mixin BlockComponentTextDirectionMixin { // defaultTextDirection will be ltr if caller hasn't passed any value. TextDirection calculateTextDirection({TextDirection? layoutDirection}) { layoutDirection ??= TextDirection.ltr; + final defaultTextDirection = editorState.editorStyle.defaultTextDirection; final direction = calculateNodeDirection( node: node, layoutDirection: layoutDirection, - defaultTextDirection: editorState.editorStyle.defaultTextDirection, + defaultTextDirection: defaultTextDirection, lastDirection: lastDirection, ); @@ -35,7 +36,7 @@ mixin BlockComponentTextDirectionMixin { // recalculate the indent padding. if (node.level > 1 && direction != lastDirection && - node.attributes[blockComponentTextDirection] == + node.direction(defaultTextDirection) == blockComponentTextDirectionAuto) { WidgetsBinding.instance .addPostFrameCallback((_) => node.parent?.notify()); @@ -66,8 +67,7 @@ TextDirection calculateNodeDirection({ }) { // if the block component has a text direction attribute which is not auto, // use it - final value = (node.attributes[blockComponentTextDirection] as String?) ?? - defaultTextDirection; + final value = node.direction(defaultTextDirection); if (value != null && value != blockComponentTextDirectionAuto) { final direction = value.toTextDirection(); if (direction != null) { @@ -126,10 +126,7 @@ TextDirection? _getDirectionFromNode(Node node, String? defaultTextDirection) { nodeDirection = blockComponentTextDirectionAuto; } - if (node.attributes.containsKey(blockComponentTextDirection) && - node.attributes[blockComponentTextDirection] != null) { - nodeDirection = node.attributes[blockComponentTextDirection]; - } + nodeDirection = node.direction(nodeDirection); if (nodeDirection != null) { if (nodeDirection == blockComponentTextDirectionAuto) { @@ -154,6 +151,13 @@ TextDirection? _determineTextDirection(String text) { return null; } +extension on Node { + String? direction(String? defaultDirection) { + return (attributes[blockComponentTextDirection] as String?) ?? + defaultDirection; + } +} + extension on String { TextDirection? toTextDirection() { if (this == blockComponentTextDirectionLTR) { diff --git a/lib/src/editor/block_component/rich_text/default_selectable_mixin.dart b/lib/src/editor/block_component/rich_text/default_selectable_mixin.dart index 5940aae96..e81c15d35 100644 --- a/lib/src/editor/block_component/rich_text/default_selectable_mixin.dart +++ b/lib/src/editor/block_component/rich_text/default_selectable_mixin.dart @@ -88,5 +88,7 @@ mixin DefaultSelectableMixin { Position end() => forward.end(); - TextDirection textDirection() => forward.textDirection(); + TextDirection textDirection() => forwardKey.currentState != null + ? forward.textDirection() + : TextDirection.ltr; } diff --git a/lib/src/editor/editor_component/service/renderer/block_component_widget.dart b/lib/src/editor/editor_component/service/renderer/block_component_widget.dart index 1d10291fb..e3b19dff6 100644 --- a/lib/src/editor/editor_component/service/renderer/block_component_widget.dart +++ b/lib/src/editor/editor_component/service/renderer/block_component_widget.dart @@ -77,9 +77,18 @@ mixin NestedBlockComponentStatefulWidgetMixin< TextDirection direction = Directionality.maybeOf(context) ?? TextDirection.ltr; if (node.children.isNotEmpty) { + TextDirection? lastDirection; + if (node.children.first.key.currentState != null) { + lastDirection = (node.children.first.key.currentState + as BlockComponentTextDirectionMixin) + .lastDirection; + } + direction = calculateNodeDirection( node: node.children.first, layoutDirection: direction, + defaultTextDirection: editorState.editorStyle.defaultTextDirection, + lastDirection: lastDirection, ); } return configuration.indentPadding(node, direction); diff --git a/test/new/block_component/text_direction_mixin_test.dart b/test/new/block_component/text_direction_mixin_test.dart index 9df05b57d..5a7d0920a 100644 --- a/test/new/block_component/text_direction_mixin_test.dart +++ b/test/new/block_component/text_direction_mixin_test.dart @@ -1,5 +1,5 @@ import 'package:appflowy_editor/appflowy_editor.dart'; -import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import '../infra/testable_editor.dart'; @@ -382,5 +382,102 @@ void main() { await editor.dispose(); }); + + testWidgets('indent padding on rtl direction', (tester) async { + final node = paragraphNode( + text: 'سلام', + textDirection: blockComponentTextDirectionRTL, + children: [ + paragraphNode( + text: 'س', + textDirection: blockComponentTextDirectionRTL, + ) + ], + ); + final editor = tester.editor..addNode(node); + await editor.startTesting(); + + final nestedBlock = + node.key.currentState as NestedBlockComponentStatefulWidgetMixin; + + expect( + nestedBlock.indentPadding, + const BlockComponentConfiguration() + .indentPadding(node, TextDirection.rtl), + ); + + await editor.dispose(); + }); + + testWidgets('indent padding on fallback to default direction auto', + (tester) async { + final node = paragraphNode( + text: 'سلام', + children: [paragraphNode(text: 'س')], + ); + final editor = tester.editor..addNode(node); + await editor.startTesting( + defaultTextDirection: blockComponentTextDirectionAuto, + ); + + final nestedBlock = + node.key.currentState as NestedBlockComponentStatefulWidgetMixin; + + expect( + nestedBlock.indentPadding, + const BlockComponentConfiguration() + .indentPadding(node, TextDirection.rtl), + ); + + await editor.dispose(); + }); + + testWidgets('indent padding respect last direction', (tester) async { + final editor = tester.editor + ..addNode( + paragraphNode( + text: 'سلام', + children: [paragraphNode()], + ), + ); + await editor.startTesting( + defaultTextDirection: blockComponentTextDirectionAuto, + ); + + final node = editor.editorState.getNodeAtPath([0])!; + + var nestedBlock = + node.key.currentState as NestedBlockComponentStatefulWidgetMixin; + expect( + nestedBlock.indentPadding, + const BlockComponentConfiguration() + .indentPadding(node, TextDirection.rtl), + ); + + final selection = Selection.single(path: [0, 0], startOffset: 0); + await editor.updateSelection(selection); + await editor.ime.typeText('a'); + + nestedBlock = + node.key.currentState as NestedBlockComponentStatefulWidgetMixin; + expect( + nestedBlock.indentPadding, + const BlockComponentConfiguration() + .indentPadding(node, TextDirection.ltr), + ); + + await simulateKeyDownEvent(LogicalKeyboardKey.backspace); + await tester.pumpAndSettle(); + + nestedBlock = + node.key.currentState as NestedBlockComponentStatefulWidgetMixin; + expect( + nestedBlock.indentPadding, + const BlockComponentConfiguration() + .indentPadding(node, TextDirection.ltr), + ); + + await editor.dispose(); + }); }); } From a389e7f263f5a01c8d8effaa87550d4fe155c5e7 Mon Sep 17 00:00:00 2001 From: "Lucas.Xu" Date: Wed, 20 Sep 2023 11:22:35 +0800 Subject: [PATCH 3/3] chore: refactor code --- .../base_component/text_direction_mixin.dart | 52 ++++++++----------- .../renderer/block_component_widget.dart | 23 ++++---- 2 files changed, 33 insertions(+), 42 deletions(-) diff --git a/lib/src/editor/block_component/base_component/text_direction_mixin.dart b/lib/src/editor/block_component/base_component/text_direction_mixin.dart index 901297bc5..c8043077a 100644 --- a/lib/src/editor/block_component/base_component/text_direction_mixin.dart +++ b/lib/src/editor/block_component/base_component/text_direction_mixin.dart @@ -95,12 +95,9 @@ TextDirection calculateNodeDirection({ // if the value is auto and the text isn't null or empty, // calculate the text direction by the text - final direction = _determineTextDirection(text); - if (direction != null) { - return direction; - } - - return defaultTextDirection?.toTextDirection() ?? layoutDirection; + return _determineTextDirection(text) ?? + defaultTextDirection?.toTextDirection() ?? + layoutDirection; } TextDirection? _getDirectionFromPreviousOrParentNode( @@ -109,34 +106,31 @@ TextDirection? _getDirectionFromPreviousOrParentNode( ) { TextDirection? prevOrParentNodeDirection; if (node.previous != null) { - prevOrParentNodeDirection = - _getDirectionFromNode(node.previous!, defaultTextDirection); + prevOrParentNodeDirection = _getDirectionFromNode( + node.previous!, + defaultTextDirection, + ); } if (node.parent != null && prevOrParentNodeDirection == null) { - prevOrParentNodeDirection = - _getDirectionFromNode(node.parent!, defaultTextDirection); + prevOrParentNodeDirection = _getDirectionFromNode( + node.parent!, + defaultTextDirection, + ); } - return prevOrParentNodeDirection; } TextDirection? _getDirectionFromNode(Node node, String? defaultTextDirection) { - String? nodeDirection; - if (defaultTextDirection == blockComponentTextDirectionAuto) { - nodeDirection = blockComponentTextDirectionAuto; - } - - nodeDirection = node.direction(nodeDirection); - - if (nodeDirection != null) { - if (nodeDirection == blockComponentTextDirectionAuto) { - return node.selectable?.textDirection(); - } else { - return nodeDirection.toTextDirection(); - } + final nodeDirection = node.direction( + defaultTextDirection == blockComponentTextDirectionAuto + ? blockComponentTextDirectionAuto + : null, + ); + if (nodeDirection == blockComponentTextDirectionAuto) { + return node.selectable?.textDirection(); + } else { + return nodeDirection?.toTextDirection(); } - - return null; } TextDirection? _determineTextDirection(String text) { @@ -152,10 +146,8 @@ TextDirection? _determineTextDirection(String text) { } extension on Node { - String? direction(String? defaultDirection) { - return (attributes[blockComponentTextDirection] as String?) ?? - defaultDirection; - } + String? direction(String? defaultDirection) => + attributes[blockComponentTextDirection] as String? ?? defaultDirection; } extension on String { diff --git a/lib/src/editor/editor_component/service/renderer/block_component_widget.dart b/lib/src/editor/editor_component/service/renderer/block_component_widget.dart index e3b19dff6..a925c8cd6 100644 --- a/lib/src/editor/editor_component/service/renderer/block_component_widget.dart +++ b/lib/src/editor/editor_component/service/renderer/block_component_widget.dart @@ -77,19 +77,18 @@ mixin NestedBlockComponentStatefulWidgetMixin< TextDirection direction = Directionality.maybeOf(context) ?? TextDirection.ltr; if (node.children.isNotEmpty) { - TextDirection? lastDirection; - if (node.children.first.key.currentState != null) { - lastDirection = (node.children.first.key.currentState - as BlockComponentTextDirectionMixin) - .lastDirection; + final firstChild = node.children.first; + final currentState = + firstChild.key.currentState as BlockComponentTextDirectionMixin?; + if (currentState != null) { + final lastDirection = currentState.lastDirection; + direction = calculateNodeDirection( + node: firstChild, + layoutDirection: direction, + defaultTextDirection: editorState.editorStyle.defaultTextDirection, + lastDirection: lastDirection, + ); } - - direction = calculateNodeDirection( - node: node.children.first, - layoutDirection: direction, - defaultTextDirection: editorState.editorStyle.defaultTextDirection, - lastDirection: lastDirection, - ); } return configuration.indentPadding(node, direction); }