From 456502586bc31a99f512f14eb8e6e657cf2dcde2 Mon Sep 17 00:00:00 2001 From: Vincent Chan Date: Thu, 11 Aug 2022 19:50:51 +0800 Subject: [PATCH] fix: undo attributest --- .../lib/src/document/attributes.dart | 16 +++++----- .../flowy_editor/lib/src/document/node.dart | 31 ++++++++----------- .../lib/src/document/state_tree.dart | 9 +++--- .../lib/src/document/text_delta.dart | 4 +-- .../src/operation/transaction_builder.dart | 6 ++-- .../flowy_editor/test/delta_test.dart | 14 ++++++++- .../flowy_editor/test/flowy_editor_test.dart | 5 ++- 7 files changed, 46 insertions(+), 39 deletions(-) diff --git a/frontend/app_flowy/packages/flowy_editor/lib/src/document/attributes.dart b/frontend/app_flowy/packages/flowy_editor/lib/src/document/attributes.dart index 39a5f4e2ff..1a846eec2c 100644 --- a/frontend/app_flowy/packages/flowy_editor/lib/src/document/attributes.dart +++ b/frontend/app_flowy/packages/flowy_editor/lib/src/document/attributes.dart @@ -22,11 +22,15 @@ Attributes invertAttributes(Attributes? attr, Attributes? base) { }); } -Attributes? composeAttributes(Attributes? a, Attributes? b) { +Attributes? composeAttributes(Attributes? a, Attributes? b, + [bool keepNull = false]) { a ??= {}; b ??= {}; - final Attributes attributes = {}; - attributes.addAll(Map.from(b)..removeWhere((_, value) => value == null)); + Attributes attributes = {...b}; + + if (!keepNull) { + attributes = Map.from(attributes)..removeWhere((_, value) => value == null); + } for (final entry in a.entries) { if (!b.containsKey(entry.key)) { @@ -34,9 +38,5 @@ Attributes? composeAttributes(Attributes? a, Attributes? b) { } } - if (attributes.isEmpty) { - return null; - } - - return attributes; + return attributes.isNotEmpty ? attributes : null; } diff --git a/frontend/app_flowy/packages/flowy_editor/lib/src/document/node.dart b/frontend/app_flowy/packages/flowy_editor/lib/src/document/node.dart index ae516478ae..24c3035e90 100644 --- a/frontend/app_flowy/packages/flowy_editor/lib/src/document/node.dart +++ b/frontend/app_flowy/packages/flowy_editor/lib/src/document/node.dart @@ -8,7 +8,7 @@ class Node extends ChangeNotifier with LinkedListEntry { Node? parent; final String type; final LinkedList children; - final Attributes attributes; + Attributes _attributes; GlobalKey? key; // TODO: abstract a selectable node?? @@ -16,22 +16,24 @@ class Node extends ChangeNotifier with LinkedListEntry { String? get subtype { // TODO: make 'subtype' as a const value. - if (attributes.containsKey('subtype')) { - assert(attributes['subtype'] is String?, + if (_attributes.containsKey('subtype')) { + assert(_attributes['subtype'] is String?, 'subtype must be a [String] or [null]'); - return attributes['subtype'] as String?; + return _attributes['subtype'] as String?; } return null; } Path get path => _path(); + Attributes get attributes => _attributes; + Node({ required this.type, required this.children, - required this.attributes, + required Attributes attributes, this.parent, - }) { + }) : _attributes = attributes { for (final child in children) { child.parent = this; } @@ -84,16 +86,9 @@ class Node extends ChangeNotifier with LinkedListEntry { } void updateAttributes(Attributes attributes) { - bool shouldNotifyParent = - this.attributes['subtype'] != attributes['subtype']; + bool shouldNotifyParent = _attributes['subtype'] != attributes['subtype']; - for (final attribute in attributes.entries) { - if (attribute.value == null) { - this.attributes.remove(attribute.key); - } else { - this.attributes[attribute.key] = attribute.value; - } - } + _attributes = composeAttributes(_attributes, attributes) ?? {}; // Notify the new attributes // if attributes contains 'subtype', should notify parent to rebuild node // else, just notify current node. @@ -149,8 +144,8 @@ class Node extends ChangeNotifier with LinkedListEntry { if (children.isNotEmpty) { map['children'] = children.map((node) => node.toJson()); } - if (attributes.isNotEmpty) { - map['attributes'] = attributes; + if (_attributes.isNotEmpty) { + map['attributes'] = _attributes; } return map; } @@ -214,7 +209,7 @@ class TextNode extends Node { TextNode( type: type ?? this.type, children: children ?? this.children, - attributes: attributes ?? this.attributes, + attributes: attributes ?? _attributes, delta: delta ?? this.delta, ); diff --git a/frontend/app_flowy/packages/flowy_editor/lib/src/document/state_tree.dart b/frontend/app_flowy/packages/flowy_editor/lib/src/document/state_tree.dart index 199736234a..b356880310 100644 --- a/frontend/app_flowy/packages/flowy_editor/lib/src/document/state_tree.dart +++ b/frontend/app_flowy/packages/flowy_editor/lib/src/document/state_tree.dart @@ -65,16 +65,15 @@ class StateTree { } } - Attributes? update(Path path, Attributes attributes) { + bool update(Path path, Attributes attributes) { if (path.isEmpty) { - return null; + return false; } final updatedNode = root.childAtPath(path); if (updatedNode == null) { - return null; + return false; } - final previousAttributes = Attributes.from(updatedNode.attributes); updatedNode.updateAttributes(attributes); - return previousAttributes; + return true; } } diff --git a/frontend/app_flowy/packages/flowy_editor/lib/src/document/text_delta.dart b/frontend/app_flowy/packages/flowy_editor/lib/src/document/text_delta.dart index 6cbc015569..6a4e4f3c80 100644 --- a/frontend/app_flowy/packages/flowy_editor/lib/src/document/text_delta.dart +++ b/frontend/app_flowy/packages/flowy_editor/lib/src/document/text_delta.dart @@ -383,8 +383,8 @@ class Delta extends Iterable { final length = min(thisIter.peekLength(), otherIter.peekLength()); final thisOp = thisIter.next(length); final otherOp = otherIter.next(length); - final attributes = - composeAttributes(thisOp.attributes, otherOp.attributes); + final attributes = composeAttributes( + thisOp.attributes, otherOp.attributes, thisOp is TextRetain); if (otherOp is TextRetain && otherOp.length > 0) { TextOperation? newOp; if (thisOp is TextRetain) { diff --git a/frontend/app_flowy/packages/flowy_editor/lib/src/operation/transaction_builder.dart b/frontend/app_flowy/packages/flowy_editor/lib/src/operation/transaction_builder.dart index 305791b519..5a8dc68b3b 100644 --- a/frontend/app_flowy/packages/flowy_editor/lib/src/operation/transaction_builder.dart +++ b/frontend/app_flowy/packages/flowy_editor/lib/src/operation/transaction_builder.dart @@ -40,10 +40,12 @@ class TransactionBuilder { updateNode(Node node, Attributes attributes) { beforeSelection = state.cursorSelection; + + final inverted = invertAttributes(attributes, node.attributes); add(UpdateOperation( node.path, - Attributes.from(node.attributes)..addAll(attributes), - node.attributes, + {...attributes}, + inverted, )); } diff --git a/frontend/app_flowy/packages/flowy_editor/test/delta_test.dart b/frontend/app_flowy/packages/flowy_editor/test/delta_test.dart index 48182912a3..a835471b55 100644 --- a/frontend/app_flowy/packages/flowy_editor/test/delta_test.dart +++ b/frontend/app_flowy/packages/flowy_editor/test/delta_test.dart @@ -1,3 +1,4 @@ +import 'package:flowy_editor/src/document/attributes.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:flowy_editor/src/document/text_delta.dart'; @@ -240,7 +241,8 @@ void main() { ..retain(3, {'bold': null}); final inverted = delta.invert(base); expect(expected, inverted); - expect(base.compose(delta).compose(inverted), base); + final t = base.compose(delta).compose(inverted); + expect(t, base); }); }); group('json', () { @@ -314,4 +316,14 @@ void main() { expect(delta.prevRunePosition(0), -1); }); }); + group("attributes", () { + test("compose", () { + final attrs = composeAttributes({"a": null}, {"b": null}, true); + expect(attrs != null, true); + expect(attrs!.containsKey("a"), true); + expect(attrs.containsKey("b"), true); + expect(attrs["a"], null); + expect(attrs["b"], null); + }); + }); } diff --git a/frontend/app_flowy/packages/flowy_editor/test/flowy_editor_test.dart b/frontend/app_flowy/packages/flowy_editor/test/flowy_editor_test.dart index e1b97a591d..e070b7f820 100644 --- a/frontend/app_flowy/packages/flowy_editor/test/flowy_editor_test.dart +++ b/frontend/app_flowy/packages/flowy_editor/test/flowy_editor_test.dart @@ -68,9 +68,8 @@ void main() { final String response = await rootBundle.loadString('assets/document.json'); final data = Map.from(json.decode(response)); final stateTree = StateTree.fromJson(data); - final attributes = stateTree.update([1, 1], {'text-type': 'heading1'}); - expect(attributes != null, true); - expect(attributes!['text-type'], 'checkbox'); + final test = stateTree.update([1, 1], {'text-type': 'heading1'}); + expect(test, true); final updatedNode = stateTree.nodeAtPath([1, 1]); expect(updatedNode != null, true); expect(updatedNode!.attributes['text-type'], 'heading1');