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: indent padding from left when rtl #318

Merged
merged 2 commits into from
Jul 19, 2023
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.
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 @@ -11,50 +11,103 @@ final _regex = RegExp(
mixin BlockComponentTextDirectionMixin {
Node get node;

TextDirection? lastDirection;

/// Calculate the text direction of a block component.
// defaultTextDirection will be ltr if caller hasn't passed any value.
TextDirection calculateTextDirection({
TextDirection? defaultTextDirection,
}) {
defaultTextDirection = defaultTextDirection ?? TextDirection.ltr;
// if the block component has a text direction attribute, use it
final value = node.attributes[blockComponentTextDirection] as String?;
if (value != null && value != blockComponentTextDirectionAuto) {
return value.toTextDirection(fallback: defaultTextDirection);
}
defaultTextDirection ??= TextDirection.ltr;

// if the block component doesn't has a text direction attribute, but has a
// parent, use the text direction of the parent
final previousNodeContainsTextDirection = node.previousNodeWhere(
(element) => element.attributes.containsKey(blockComponentTextDirection),
final direction = calculateNodeDirection(
node: node,
defaultTextDirection: defaultTextDirection,
lastDirection: lastDirection,
);
if (value == blockComponentTextDirectionAuto &&
previousNodeContainsTextDirection != null) {
final String previousValue = previousNodeContainsTextDirection
.attributes[blockComponentTextDirection];
defaultTextDirection =
previousValue.toTextDirection(fallback: defaultTextDirection);
}

// if the value is null or the text is null or empty,
// use the default text direction
final text = node.delta?.toPlainText();
if (value == null || text == null || text.isEmpty) {
return defaultTextDirection;
// node indent padding is added by parent node and the padding direction
// is equal to the node text direction. when the node direction is auto
// there is a special case which on typing text, the node direction could
// change without any change to parent node, because no attribute of the
// node changes as the direction attribute is auto but the calculated can
// change to rtl or ltr. in this cases we should notify parent node to
// recalculate the indent padding.
if (node.level > 1 &&
direction != lastDirection &&
node.attributes[blockComponentTextDirection] ==
blockComponentTextDirectionAuto) {
WidgetsBinding.instance
.addPostFrameCallback((_) => node.parent?.notify());
}
lastDirection = direction;

return direction;
}
}

/// Calculate the text direction of a node.
// If the textDirection attribute is not set we will use defaultTextDirection.
// If the textDirection is ltr or rtl we will apply that.
// If the textDirection is auto we go by these priorities:
// 1. Determine the direction by first character with strong directionality
// 2. lastDirection which is the node last determined direction
// 3. previous line direction
// 4. defaultTextDirection
// We will move from first priority when for example the node text is empty or
// it only has characters without strong directionality e.g. '@'.
TextDirection calculateNodeDirection({
required Node node,
required TextDirection defaultTextDirection,
TextDirection? lastDirection,
}) {
defaultTextDirection = lastDirection ?? defaultTextDirection;

// if the block component has a text direction attribute which is not auto,
// use it
final value = node.attributes[blockComponentTextDirection] as String?;
if (value != null && value != blockComponentTextDirectionAuto) {
return value.toTextDirection(fallback: defaultTextDirection);
}

// 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;
}
// previous line direction
final previousNodeContainsTextDirection = node.previousNodeWhere(
(element) => element.attributes.containsKey(blockComponentTextDirection),
);
if (lastDirection == null &&
value == blockComponentTextDirectionAuto &&
previousNodeContainsTextDirection != null) {
final String previousValue = previousNodeContainsTextDirection
.attributes[blockComponentTextDirection];
if (previousValue == blockComponentTextDirectionAuto) {
defaultTextDirection =
previousNodeContainsTextDirection.selectable?.textDirection() ??
defaultTextDirection;
} else {
defaultTextDirection =
previousValue.toTextDirection(fallback: defaultTextDirection);
}
}

// if the value is null or the text is null or empty,
// use the default text direction
final text = node.delta?.toPlainText();
if (value == null || text == null || text.isEmpty) {
return defaultTextDirection;
}

// 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;
}
}

return defaultTextDirection;
}

extension on String {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,6 @@ class _BulletedListBlockComponentWidgetState
@override
Node get node => widget.node;

@override
EdgeInsets get indentPadding => configuration.indentPadding(
node,
calculateTextDirection(
defaultTextDirection: Directionality.maybeOf(context),
),
);

@override
Widget buildComponent(BuildContext context) {
final textDirection = calculateTextDirection(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,6 @@ class _NumberedListBlockComponentWidgetState
@override
Node get node => widget.node;

@override
EdgeInsets get indentPadding => configuration.indentPadding(
node,
calculateTextDirection(
defaultTextDirection: Directionality.maybeOf(context),
),
);

@override
Widget buildComponent(BuildContext context) {
final textDirection = calculateTextDirection(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,6 @@ class _TextBlockComponentWidgetState extends State<TextBlockComponentWidget>
@override
Node get node => widget.node;

@override
EdgeInsets get indentPadding => configuration.indentPadding(
node,
calculateTextDirection(
defaultTextDirection: Directionality.maybeOf(context),
),
);

@override
Widget buildComponent(BuildContext context) {
final textDirection = calculateTextDirection(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,6 @@ class _TodoListBlockComponentWidgetState
@override
Node get node => widget.node;

@override
EdgeInsets get indentPadding => configuration.indentPadding(
node,
calculateTextDirection(
defaultTextDirection: Directionality.maybeOf(context),
),
);

bool get checked => widget.node.attributes[TodoListBlockKeys.checked];

@override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,19 @@ mixin NestedBlockComponentStatefulWidgetMixin<
on State<T>, BlockComponentBackgroundColorMixin {
late final editorState = Provider.of<EditorState>(context, listen: false);

EdgeInsets get indentPadding;
BlockComponentConfiguration get configuration;

EdgeInsets get indentPadding {
TextDirection direction =
Directionality.maybeOf(context) ?? TextDirection.ltr;
if (node.children.isNotEmpty) {
direction = calculateNodeDirection(
node: node.children.first,
defaultTextDirection: direction,
);
}
return configuration.indentPadding(node, direction);
}

double? cachedLeft;

Expand Down
79 changes: 76 additions & 3 deletions test/new/block_component/text_direction_mixin_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import 'package:appflowy_editor/appflowy_editor.dart';
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';

import '../infra/testable_editor.dart';

class TextDirectionTest with BlockComponentTextDirectionMixin {
TextDirectionTest({
required this.node,
Expand Down Expand Up @@ -40,13 +42,15 @@ void main() {
expect(direction, TextDirection.ltr);
});

test('auto empty text with lastDirection', () {
test('auto empty text with last direction', () {
final node = paragraphNode(
text: '',
textDirection: blockComponentTextDirectionAuto,
);
final direction = TextDirectionTest(node: node).calculateTextDirection(
defaultTextDirection: TextDirection.rtl,
final textDirectionTest = TextDirectionTest(node: node);
textDirectionTest.lastDirection = TextDirection.rtl;
final direction = textDirectionTest.calculateTextDirection(
defaultTextDirection: TextDirection.ltr,
);
expect(direction, TextDirection.rtl);
});
Expand Down Expand Up @@ -115,6 +119,25 @@ void main() {
expect(direction, TextDirection.rtl);
});

test('use previous node direction (rtl) only when current is auto', () {
final node = pageNode(
children: [
paragraphNode(
text: 'سلام',
textDirection: blockComponentTextDirectionRTL,
),
paragraphNode(
text: '\$',
),
],
);
final direction =
TextDirectionTest(node: node.children.last).calculateTextDirection(
defaultTextDirection: TextDirection.ltr,
);
expect(direction, TextDirection.ltr);
});

test(
'auto empty text don\'t use previous node direction because we can determine by the node text',
() {
Expand All @@ -136,5 +159,55 @@ void main() {
);
expect(direction, TextDirection.ltr);
});

test(
'auto empty text don\'t use previous node direction when we have last direction',
() {
final node = pageNode(
children: [
paragraphNode(
text: 'سلام',
textDirection: blockComponentTextDirectionRTL,
),
paragraphNode(
text: '',
textDirection: blockComponentTextDirectionAuto,
),
],
);

final textDirectionTest = TextDirectionTest(node: node.children.last);
textDirectionTest.lastDirection = TextDirection.ltr;

final direction = textDirectionTest.calculateTextDirection(
defaultTextDirection: TextDirection.rtl,
);
expect(direction, TextDirection.ltr);
});
});

group('text_direction_mixin - widget test', () {
testWidgets('use previous node direction (auto) calculated value (rtl)',
(tester) async {
final editor = tester.editor
..addNode(
paragraphNode(
text: 'سلام',
textDirection: blockComponentTextDirectionAuto,
),
)
..addNode(
paragraphNode(
text: '\$',
textDirection: blockComponentTextDirectionAuto,
),
);
await editor.startTesting();

final node = editor.nodeAtPath([1])!;
expect(node.selectable?.textDirection(), TextDirection.rtl);

await editor.dispose();
});
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -467,5 +467,48 @@ void main() async {

await editor.dispose();
});

testWidgets("clear text but keep the old direction", (tester) async {
final editor = tester.editor
..addNode(
paragraphNode(
text: 'Hello',
textDirection: blockComponentTextDirectionLTR,
),
)
..addNode(
paragraphNode(
text: 'س',
textDirection: blockComponentTextDirectionAuto,
),
);
await editor.startTesting();

Node node = editor.nodeAtPath([1])!;
expect(
node.selectable?.textDirection().name,
blockComponentTextDirectionRTL,
);

final selection = Selection.collapsed(
Position(path: [1], offset: 1),
);
await editor.updateSelection(selection);

await simulateKeyDownEvent(LogicalKeyboardKey.backspace);
await tester.pumpAndSettle();

node = editor.nodeAtPath([1])!;
expect(
node.delta?.toPlainText().isEmpty,
true,
);
expect(
node.selectable?.textDirection().name,
blockComponentTextDirectionRTL,
);

await editor.dispose();
});
});
}
Loading