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: respect default and last direction on new line and indent #482

Merged
merged 3 commits into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);

Expand All @@ -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());
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}
}

Expand All @@ -106,6 +95,51 @@ 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;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
final direction = _determineTextDirection(text);
if (direction != null) {
return direction;
}
return defaultTextDirection?.toTextDirection() ?? layoutDirection;
return _determineTextDirection(text) ??
defaultTextDirection?.toTextDirection() ??
layoutDirection;

}

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;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
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? _getDirectionFromPreviousOrParentNode(
Node node,
String? defaultTextDirection,
) {
if (node.previous != null) {
return _getDirectionFromNode(node.previous!, defaultTextDirection);
}
if (node.parent != null) {
return _getDirectionFromNode(node.parent!, defaultTextDirection);
}
return null;
}


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();
}
}

return null;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
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();
}
}
return null;
}
TextDirection? _getDirectionFromNode(Node node, String? defaultTextDirection) {
final nodeDirection = node.direction(
defaultTextDirection == blockComponentTextDirectionAuto
? blockComponentTextDirectionAuto
: null,
);
if (nodeDirection == blockComponentTextDirectionAuto) {
return node.selectable?.textDirection();
} else {
return nodeDirection?.toTextDirection();
}
}


TextDirection? _determineTextDirection(String text) {
final matches = _regex.firstMatch(text);
if (matches != null) {
if (matches.group(1) != null) {
Expand All @@ -114,23 +148,14 @@ TextDirection calculateNodeDirection({
return TextDirection.ltr;
}
}

return defaultTextDirection?.toTextDirection() ?? layoutDirection;
return null;
}

Node? previousOrParentNodeWithTextDirection(Node node) {
bool textDirectionCheck(node) =>
node != null &&
node.attributes.containsKey(blockComponentTextDirection) &&
node.attributes[blockComponentTextDirection] != null;

if (textDirectionCheck(node.previous)) {
return node.previous;
} else if (textDirectionCheck(node.parent)) {
return node.parent;
extension on Node {
String? direction(String? defaultDirection) {
return (attributes[blockComponentTextDirection] as String?) ??
defaultDirection;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
extension on Node {
String? direction(String? defaultDirection) {
return (attributes[blockComponentTextDirection] as String?) ??
defaultDirection;
extension on Node {
String? direction(String? defaultDirection) =>
attributes[blockComponentTextDirection] as String? ?? defaultDirection;

}

return null;
}

extension on String {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,5 +88,7 @@ mixin DefaultSelectableMixin {

Position end() => forward.end();

TextDirection textDirection() => forward.textDirection();
TextDirection textDirection() => forwardKey.currentState != null
? forward.textDirection()
: TextDirection.ltr;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
123 changes: 122 additions & 1 deletion test/new/block_component/text_direction_mixin_test.dart
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -358,5 +358,126 @@ 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();
});

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();
});
});
}
9 changes: 7 additions & 2 deletions test/new/infra/testable_editor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class TestableEditor {
ScrollController? scrollController,
Widget Function(Widget child)? wrapper,
TargetPlatform? platform,
String? defaultTextDirection,
}) async {
await AppFlowyEditorLocalizations.load(locale);

Expand Down Expand Up @@ -68,8 +69,12 @@ class TestableEditor {
.testableFindAndReplaceCommands,
],
editorStyle: inMobile
? const EditorStyle.mobile()
: const EditorStyle.desktop(),
? EditorStyle.mobile(
defaultTextDirection: defaultTextDirection,
)
: EditorStyle.desktop(
defaultTextDirection: defaultTextDirection,
),
);
},
);
Expand Down