From e265fa9cf544180c6140f853ed412f663b49bf6a Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Wed, 15 Jan 2025 15:12:24 -0800 Subject: [PATCH 1/3] [Quill] - Make consecutive node merges configurable (Resolves #2510) --- super_editor/clones/quill/lib/app.dart | 8 +- super_editor_quill/README.md | 5 +- .../lib/src/parsing/parser.dart | 84 ++++++-- .../test/parsing/multiline_parsing_test.dart | 184 ++++++++++++++++++ .../test/{ => parsing}/parsing_test.dart | 68 +------ 5 files changed, 266 insertions(+), 83 deletions(-) create mode 100644 super_editor_quill/test/parsing/multiline_parsing_test.dart rename super_editor_quill/test/{ => parsing}/parsing_test.dart (89%) diff --git a/super_editor/clones/quill/lib/app.dart b/super_editor/clones/quill/lib/app.dart index 69f6d10d0c..d46592ec63 100644 --- a/super_editor/clones/quill/lib/app.dart +++ b/super_editor/clones/quill/lib/app.dart @@ -106,12 +106,18 @@ This is regular text right below header 2. Some **bold** text. > This is a blockquote. +> It can span multiple lines. -* This is a list item. +* This is a bulleted list item. + +1. This is a numerical list item. ``` This is a code block. +It can span multiple lines. ``` + +The end. '''); } diff --git a/super_editor_quill/README.md b/super_editor_quill/README.md index b56a59b3d0..13523b1e7e 100644 --- a/super_editor_quill/README.md +++ b/super_editor_quill/README.md @@ -34,4 +34,7 @@ Quill Delta document. Also, a `SuperEditor` document can be serialized to a Quil Regardless of the incoming or outgoing document format, the actual editing pipeline within `SuperEditor` remains the same. Thus, you could start a document from Markdown, and then export a document to -Quill Delta, or vis-a-versa. `SuperEditor` internals are format agnostic. \ No newline at end of file +Quill Delta, or vis-a-versa. `SuperEditor` internals are format agnostic. + +## References + * Interactive Playground: https://v1.quilljs.com/playground/ \ No newline at end of file diff --git a/super_editor_quill/lib/src/parsing/parser.dart b/super_editor_quill/lib/src/parsing/parser.dart index 48b4859771..7e77c522a1 100644 --- a/super_editor_quill/lib/src/parsing/parser.dart +++ b/super_editor_quill/lib/src/parsing/parser.dart @@ -22,12 +22,48 @@ import 'package:super_editor_quill/src/parsing/inline_formats.dart'; /// and a [MutableComposer]. The document must be empty. /// {@endtemplate} /// +/// {@template merge_consecutive_blocks} +/// ### Merging consecutive blocks +/// +/// The Delta format creates some ambiguity around when multiple lines should +/// be combined into a single block vs one block per line. E.g., a code block +/// with multiple lines of code vs a series of independent code blocks. +/// +/// [consecutiveBlockTypesToMerge] explicitly tells the parser which consecutive +/// [DocumentNode]s should be merged together when not separated by an unstyled +/// newline in the given deltas. If you don't want to ever merge consecutive +/// blocks, pass an empty set `{}`. If you want the default block merge behavior, +/// pass nothing or pass `null`. If you want to choose a custom set of blocks +/// to merge, then pass a set with those [DocumentNode] block type attributions. +/// +/// Example of consecutive code blocks that would be merged (if requested): +/// +/// [ +/// { "insert": "Code line one" }, +/// { "insert": "\n", "attributed": { "code-block": "plain"} }, +/// { "insert": "Code line two" }, +/// { "insert": "\n", "attributed": { "code-block": "plain"} }, +/// ] +/// +/// Example of code blocks, separated by an unstyled newline, that wouldn't be merged: +/// +/// [ +/// { "insert": "Code line one" }, +/// { "insert": "\n", "attributed": { "code-block": "plain"} }, +/// { "insert": "\n" }, +/// { "insert": "Code line two" }, +/// { "insert": "\n", "attributed": { "code-block": "plain"} }, +/// ] +/// +/// {@endtemplate} +/// /// For more information about the Quill Delta format, see the official /// documentation: https://quilljs.com/docs/delta/ MutableDocument parseQuillDeltaDocument( Map deltaDocument, { Editor? customEditor, List blockFormats = defaultBlockFormats, + Set? consecutiveBlockTypesToMerge, List inlineFormats = defaultInlineFormats, List inlineEmbedFormats = const [], List embedBlockFormats = defaultEmbedBockFormats, @@ -35,6 +71,7 @@ MutableDocument parseQuillDeltaDocument( return parseQuillDeltaOps( deltaDocument["ops"], customEditor: customEditor, + consecutiveBlockTypesToMerge: consecutiveBlockTypesToMerge, blockFormats: blockFormats, inlineFormats: inlineFormats, inlineEmbedFormats: inlineEmbedFormats, @@ -42,7 +79,7 @@ MutableDocument parseQuillDeltaDocument( ); } -/// Parses a list Quill Delta operations (as JSON) into a [MutableDocument]. +/// Parses a list of Quill Delta operations (as JSON) into a [MutableDocument]. /// /// This parser is the same as [parseQuillDeltaDocument] except that this method /// directly accepts the operations list instead of the whole document map. This @@ -50,14 +87,19 @@ MutableDocument parseQuillDeltaDocument( /// operations are exchanged, rather than the whole document object. /// /// {@macro parse_deltas_custom_editor} +/// +/// {@macro merge_consecutive_blocks} MutableDocument parseQuillDeltaOps( List deltaOps, { Editor? customEditor, List blockFormats = defaultBlockFormats, + Set? consecutiveBlockTypesToMerge, List inlineFormats = defaultInlineFormats, List inlineEmbedFormats = const [], List embedBlockFormats = defaultEmbedBockFormats, }) { + consecutiveBlockTypesToMerge ??= defaultConsecutiveBlockTypesToMerge; + // Deserialize the delta operations JSON into a Dart data structure. final deltaDocument = Delta.fromJson(deltaOps); @@ -117,6 +159,7 @@ MutableDocument parseQuillDeltaOps( delta.applyToDocument( editor, blockFormats: blockFormats, + consecutiveBlockTypesToMerge: consecutiveBlockTypesToMerge, inlineFormats: inlineFormats, inlineEmbedFormats: inlineEmbedFormats, embedBlockFormats: embedBlockFormats, @@ -179,10 +222,13 @@ extension OperationParser on Operation { void applyToDocument( Editor editor, { required List blockFormats, + Set? consecutiveBlockTypesToMerge, required List inlineFormats, required List inlineEmbedFormats, required List embedBlockFormats, }) { + consecutiveBlockTypesToMerge ??= defaultConsecutiveBlockTypesToMerge; + final document = editor.context.find(Editor.documentKey); final composer = editor.context.find(Editor.composerKey); @@ -197,44 +243,47 @@ extension OperationParser on Operation { _doInsertMedia(editor, composer, inlineEmbedFormats, embedBlockFormats); } - // Deduplicate all back-to-back code blocks. + // Merge consecutive blocks as desired by the given node types. final document = editor.context.find(Editor.documentKey); if (document.nodeCount < 3) { - // Minimum of 3 nodes: code, code, newline. + // Minimum of 3 nodes: block, block, newline. break; } - var codeBlocks = []; + var blocksToMerge = []; for (int i = document.nodeCount - 2; i >= 0; i -= 1) { final node = document.getNodeAt(i)!; if (node is! ParagraphNode) { break; } - if (node.getMetadataValue("blockType") != codeAttribution) { + + final blockType = node.getMetadataValue("blockType"); + if (!consecutiveBlockTypesToMerge.contains(blockType)) { break; } - codeBlocks.add(node); + blocksToMerge.add(node); } - if (codeBlocks.length < 2) { + if (blocksToMerge.length < 2) { break; } - codeBlocks = codeBlocks.reversed.toList(); - final mergeNode = codeBlocks.first; - var codeToMove = codeBlocks[1].text.insertString(textToInsert: "\n", startOffset: 0); - for (int i = 2; i < codeBlocks.length; i += 1) { - codeToMove = codeToMove.copyAndAppend(codeBlocks[i].text.insertString(textToInsert: "\n", startOffset: 0)); + blocksToMerge = blocksToMerge.reversed.toList(); + final mergeNode = blocksToMerge.first; + var nodeContentToMove = blocksToMerge[1].text.insertString(textToInsert: "\n", startOffset: 0); + for (int i = 2; i < blocksToMerge.length; i += 1) { + nodeContentToMove = + nodeContentToMove.copyAndAppend(blocksToMerge[i].text.insertString(textToInsert: "\n", startOffset: 0)); } editor.execute([ InsertAttributedTextRequest( DocumentPosition(nodeId: mergeNode.id, nodePosition: mergeNode.endPosition), - codeToMove, + nodeContentToMove, ), - for (int i = 1; i < codeBlocks.length; i += 1) // - DeleteNodeRequest(nodeId: codeBlocks[i].id), + for (int i = 1; i < blocksToMerge.length; i += 1) // + DeleteNodeRequest(nodeId: blocksToMerge[i].id), ]); case DeltaOperationType.retain: @@ -510,3 +559,8 @@ enum DeltaOperationType { retain, delete, } + +final defaultConsecutiveBlockTypesToMerge = { + blockquoteAttribution, + codeAttribution, +}; diff --git a/super_editor_quill/test/parsing/multiline_parsing_test.dart b/super_editor_quill/test/parsing/multiline_parsing_test.dart new file mode 100644 index 0000000000..6cf681c7d7 --- /dev/null +++ b/super_editor_quill/test/parsing/multiline_parsing_test.dart @@ -0,0 +1,184 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:super_editor/super_editor.dart'; +import 'package:super_editor_quill/super_editor_quill.dart'; + +void main() { + group("Delta document parsing > test > multiline >", () { + test("parses inline newline characters into multiple document nodes", () { + final document = parseQuillDeltaDocument( + { + "ops": [ + {"insert": "\nLine one\nLine two\nLine three\nLine four\n\n"} + ], + }, + ); + + expect((document.getNodeAt(0)! as TextNode).text.toPlainText(), ""); + expect((document.getNodeAt(1)! as TextNode).text.toPlainText(), "Line one"); + expect((document.getNodeAt(2)! as TextNode).text.toPlainText(), "Line two"); + expect((document.getNodeAt(3)! as TextNode).text.toPlainText(), "Line three"); + expect((document.getNodeAt(4)! as TextNode).text.toPlainText(), "Line four"); + expect((document.getNodeAt(5)! as TextNode).text.toPlainText(), ""); + expect((document.getNodeAt(6)! as TextNode).text.toPlainText(), ""); + + // A note on the length of the document. If this document is placed in a + // Quill editor, there will only be 6 lines the user can edit. This seems + // like it's probably a part of Quill specified behavior. I think that + // because every Quill document must always end with a newline, the final + // newline of a document is ignored by a Quill editor. But in our case + // we parse every newline, including the trailing newline, so the length + // of this document is 7. If that's a problem, we can make the parser + // more intelligent about this later. + expect(document.nodeCount, 7); + }); + + group("block merging >", () { + group("default merging >", () { + test("merges consecutive blockquote", () { + final document = parseQuillDeltaDocument( + { + "ops": [ + {"insert": "This is a blockquote"}, + { + "attributes": { + "blockquote": {"blockquote": true} + }, + "insert": "\n" + }, + {"insert": "This is line two"}, + { + "attributes": { + "blockquote": {"blockquote": true} + }, + "insert": "\n" + }, + ], + }, + ); + + expect( + (document.getNodeAt(0)! as ParagraphNode).text.toPlainText(), + "This is a blockquote\nThis is line two", + ); + expect((document.getNodeAt(0)! as ParagraphNode).getMetadataValue("blockType"), blockquoteAttribution); + expect((document.getNodeAt(1)! as ParagraphNode).text.toPlainText(), ""); + expect(document.nodeCount, 2); + }); + + test("does not merge blockquotes separated by an unstyled insert", () { + final document = parseQuillDeltaDocument( + { + "ops": [ + {"insert": "This is a blockquote"}, + { + "attributes": { + "blockquote": {"blockquote": true} + }, + "insert": "\n" + }, + {"insert": "\n"}, + {"insert": "This is line two"}, + { + "attributes": { + "blockquote": {"blockquote": true} + }, + "insert": "\n" + }, + ], + }, + ); + + expect( + (document.getNodeAt(0)! as ParagraphNode).text.toPlainText(), + "This is a blockquote", + ); + expect( + (document.getNodeAt(1)! as ParagraphNode).text.toPlainText(), + "", + ); + expect( + (document.getNodeAt(2)! as ParagraphNode).text.toPlainText(), + "This is line two", + ); + expect((document.getNodeAt(0)! as ParagraphNode).getMetadataValue("blockType"), blockquoteAttribution); + expect((document.getNodeAt(1)! as ParagraphNode).text.toPlainText(), ""); + expect((document.getNodeAt(2)! as ParagraphNode).getMetadataValue("blockType"), blockquoteAttribution); + expect(document.nodeCount, 4); + }); + + test("merges consecutive code blocks", () { + // Notice that Delta encodes each line of a code block as a separate attributed + // delta. But when a Quill editor renders the code block, it's rendered as one + // block. This test ensures that Super Editor accumulates back-to-back code + // lines into a single code node. + final document = parseQuillDeltaDocument( + { + "ops": [ + {"insert": "This is a code block"}, + { + "attributes": {"code-block": "plain"}, + "insert": "\n" + }, + {"insert": "This is line two"}, + { + "attributes": {"code-block": "plain"}, + "insert": "\n" + }, + {"insert": "This is line three"}, + { + "attributes": {"code-block": "plain"}, + "insert": "\n" + }, + ], + }, + ); + + expect( + (document.getNodeAt(0)! as ParagraphNode).text.toPlainText(), + "This is a code block\nThis is line two\nThis is line three", + ); + expect((document.getNodeAt(0)! as ParagraphNode).getMetadataValue("blockType"), codeAttribution); + expect((document.getNodeAt(1)! as ParagraphNode).text.toPlainText(), ""); + expect(document.nodeCount, 2); + }); + + test("does not merge code blocks separated by an unstyled insert", () { + final document = parseQuillDeltaDocument( + { + "ops": [ + {"insert": "This is a code block"}, + { + "attributes": {"code-block": "plain"}, + "insert": "\n" + }, + {"insert": "\n"}, + {"insert": "This is line two"}, + { + "attributes": {"code-block": "plain"}, + "insert": "\n" + }, + ], + }, + ); + + expect( + (document.getNodeAt(0)! as ParagraphNode).text.toPlainText(), + "This is a code block", + ); + expect( + (document.getNodeAt(1)! as ParagraphNode).text.toPlainText(), + "", + ); + expect( + (document.getNodeAt(2)! as ParagraphNode).text.toPlainText(), + "This is line two", + ); + expect((document.getNodeAt(0)! as ParagraphNode).getMetadataValue("blockType"), codeAttribution); + expect((document.getNodeAt(1)! as ParagraphNode).text.toPlainText(), ""); + expect((document.getNodeAt(2)! as ParagraphNode).getMetadataValue("blockType"), codeAttribution); + expect(document.nodeCount, 4); + }); + }); + }); + }); +} diff --git a/super_editor_quill/test/parsing_test.dart b/super_editor_quill/test/parsing/parsing_test.dart similarity index 89% rename from super_editor_quill/test/parsing_test.dart rename to super_editor_quill/test/parsing/parsing_test.dart index 0f4ed31c50..79c31f512c 100644 --- a/super_editor_quill/test/parsing_test.dart +++ b/super_editor_quill/test/parsing/parsing_test.dart @@ -3,43 +3,15 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:super_editor/super_editor.dart'; import 'package:super_editor_quill/super_editor_quill.dart'; -import 'test_documents.dart'; +import '../test_documents.dart'; // Useful links: -// - create a Delta document in the browser: https://quilljs.com/docs/delta +// - create a Delta document in the browser: https://quilljs.com/playground/snow // - list of supported formats (bold, italics, header, etc): https://quilljs.com/docs/formats void main() { group("Delta document parsing >", () { group("text >", () { - test("newlines", () { - final document = parseQuillDeltaDocument( - { - "ops": [ - {"insert": "\nLine one\nLine two\nLine three\nLine four\n\n"} - ], - }, - ); - - expect((document.getNodeAt(0)! as TextNode).text.toPlainText(), ""); - expect((document.getNodeAt(1)! as TextNode).text.toPlainText(), "Line one"); - expect((document.getNodeAt(2)! as TextNode).text.toPlainText(), "Line two"); - expect((document.getNodeAt(3)! as TextNode).text.toPlainText(), "Line three"); - expect((document.getNodeAt(4)! as TextNode).text.toPlainText(), "Line four"); - expect((document.getNodeAt(5)! as TextNode).text.toPlainText(), ""); - expect((document.getNodeAt(6)! as TextNode).text.toPlainText(), ""); - - // A note on the length of the document. If this document is placed in a - // Quill editor, there will only be 6 lines the user can edit. This seems - // like it's probably a part of Quill specified behavior. I think that - // because every Quill document must always end with a newline, the final - // newline of a document is ignored by a Quill editor. But in our case - // we parse every newline, including the trailing newline, so the length - // of this document is 7. If that's a problem, we can make the parser - // more intelligent about this later. - expect(document.nodeCount, 7); - }); - test("plain text followed by block format", () { final document = parseQuillDeltaDocument( { @@ -67,42 +39,6 @@ void main() { expect(document.nodeCount, 3); }); - test("multiline code block", () { - // Notice that Delta encodes each line of a code block as a separate attributed - // delta. But when a Quill editor renders the code block, it's rendered as one - // block. This test ensures that Super Editor accumulates back-to-back code - // lines into a single code node. - final document = parseQuillDeltaDocument( - { - "ops": [ - {"insert": "This is a code block"}, - { - "attributes": {"code-block": "plain"}, - "insert": "\n" - }, - {"insert": "This is line two"}, - { - "attributes": {"code-block": "plain"}, - "insert": "\n" - }, - {"insert": "This is line three"}, - { - "attributes": {"code-block": "plain"}, - "insert": "\n" - }, - ], - }, - ); - - expect( - (document.getNodeAt(0)! as ParagraphNode).text.toPlainText(), - "This is a code block\nThis is line two\nThis is line three", - ); - expect((document.getNodeAt(0)! as ParagraphNode).getMetadataValue("blockType"), codeAttribution); - expect((document.getNodeAt(1)! as ParagraphNode).text.toPlainText(), ""); - expect(document.nodeCount, 2); - }); - test("all text blocks and styles", () { final document = parseQuillDeltaOps(allTextStylesDeltaDocument); From aea08414a990422b03ef063236bd29166097659b Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Wed, 15 Jan 2025 15:56:34 -0800 Subject: [PATCH 2/3] Changed block merging from block type identities to rules so that we can, for example, merge block types that can have different properties (like a BannerAttribution with Orange vs with Green). --- .../lib/src/parsing/parser.dart | 93 ++++++++++++++----- 1 file changed, 71 insertions(+), 22 deletions(-) diff --git a/super_editor_quill/lib/src/parsing/parser.dart b/super_editor_quill/lib/src/parsing/parser.dart index 7e77c522a1..83405c11be 100644 --- a/super_editor_quill/lib/src/parsing/parser.dart +++ b/super_editor_quill/lib/src/parsing/parser.dart @@ -29,12 +29,9 @@ import 'package:super_editor_quill/src/parsing/inline_formats.dart'; /// be combined into a single block vs one block per line. E.g., a code block /// with multiple lines of code vs a series of independent code blocks. /// -/// [consecutiveBlockTypesToMerge] explicitly tells the parser which consecutive +/// [blockMergeRules] explicitly tells the parser which consecutive /// [DocumentNode]s should be merged together when not separated by an unstyled -/// newline in the given deltas. If you don't want to ever merge consecutive -/// blocks, pass an empty set `{}`. If you want the default block merge behavior, -/// pass nothing or pass `null`. If you want to choose a custom set of blocks -/// to merge, then pass a set with those [DocumentNode] block type attributions. +/// newline in the given deltas. /// /// Example of consecutive code blocks that would be merged (if requested): /// @@ -63,7 +60,7 @@ MutableDocument parseQuillDeltaDocument( Map deltaDocument, { Editor? customEditor, List blockFormats = defaultBlockFormats, - Set? consecutiveBlockTypesToMerge, + List blockMergeRules = defaultBlockMergeRules, List inlineFormats = defaultInlineFormats, List inlineEmbedFormats = const [], List embedBlockFormats = defaultEmbedBockFormats, @@ -71,7 +68,7 @@ MutableDocument parseQuillDeltaDocument( return parseQuillDeltaOps( deltaDocument["ops"], customEditor: customEditor, - consecutiveBlockTypesToMerge: consecutiveBlockTypesToMerge, + blockMergeRules: blockMergeRules, blockFormats: blockFormats, inlineFormats: inlineFormats, inlineEmbedFormats: inlineEmbedFormats, @@ -93,13 +90,11 @@ MutableDocument parseQuillDeltaOps( List deltaOps, { Editor? customEditor, List blockFormats = defaultBlockFormats, - Set? consecutiveBlockTypesToMerge, + List blockMergeRules = defaultBlockMergeRules, List inlineFormats = defaultInlineFormats, List inlineEmbedFormats = const [], List embedBlockFormats = defaultEmbedBockFormats, }) { - consecutiveBlockTypesToMerge ??= defaultConsecutiveBlockTypesToMerge; - // Deserialize the delta operations JSON into a Dart data structure. final deltaDocument = Delta.fromJson(deltaOps); @@ -159,7 +154,7 @@ MutableDocument parseQuillDeltaOps( delta.applyToDocument( editor, blockFormats: blockFormats, - consecutiveBlockTypesToMerge: consecutiveBlockTypesToMerge, + blockMergeRules: blockMergeRules, inlineFormats: inlineFormats, inlineEmbedFormats: inlineEmbedFormats, embedBlockFormats: embedBlockFormats, @@ -222,13 +217,11 @@ extension OperationParser on Operation { void applyToDocument( Editor editor, { required List blockFormats, - Set? consecutiveBlockTypesToMerge, + List blockMergeRules = defaultBlockMergeRules, required List inlineFormats, required List inlineEmbedFormats, required List embedBlockFormats, }) { - consecutiveBlockTypesToMerge ??= defaultConsecutiveBlockTypesToMerge; - final document = editor.context.find(Editor.documentKey); final composer = editor.context.find(Editor.composerKey); @@ -250,6 +243,29 @@ extension OperationParser on Operation { break; } + final nodeBeforeTrailingNewline = document.getNodeBefore(document.last)!; + final blockTypeToMerge = nodeBeforeTrailingNewline.getMetadataValue(NodeMetadata.blockType); + var shouldMerge = false; + for (final rule in blockMergeRules) { + final ruleShouldMerge = rule.shouldMerge(blockTypeToMerge); + if (ruleShouldMerge == true) { + // The rule says we definitely want to merge. + shouldMerge = true; + break; + } + if (ruleShouldMerge == false) { + // The rule says we definitely don't want to merge. + shouldMerge = false; + break; + } + } + if (!shouldMerge) { + // Our merge rules don't want us to merge this node. + break; + } + + // The most recently inserted node supports merging. Check if we should merge with + // the previous node. var blocksToMerge = []; for (int i = document.nodeCount - 2; i >= 0; i -= 1) { final node = document.getNodeAt(i)!; @@ -257,8 +273,9 @@ extension OperationParser on Operation { break; } - final blockType = node.getMetadataValue("blockType"); - if (!consecutiveBlockTypesToMerge.contains(blockType)) { + if (node.getMetadataValue("blockType") != blockTypeToMerge) { + // This node doesn't have the same block type as the one(s) we want + // to merge. Break out the of loop. break; } @@ -506,7 +523,7 @@ extension OperationParser on Operation { // The caret wants to move beyond this paragraph. unitsToMove -= selectedNode.text.length - currentPosition.offset; - selectedNode = document.getNodeAfter(selectedNode)!; + selectedNode = document.getNodeAfterById(selectedNode.id)!; caretPosition = DocumentPosition( nodeId: selectedNode.id, nodePosition: selectedNode.beginningPosition, @@ -525,7 +542,7 @@ extension OperationParser on Operation { // The deltas want to retain more beyond this node. unitsToMove -= 1; - selectedNode = document.getNodeAfter(selectedNode)!; + selectedNode = document.getNodeAfterById(selectedNode.id)!; caretPosition = DocumentPosition( nodeId: selectedNode.id, nodePosition: selectedNode.beginningPosition, @@ -560,7 +577,39 @@ enum DeltaOperationType { delete, } -final defaultConsecutiveBlockTypesToMerge = { - blockquoteAttribution, - codeAttribution, -}; +/// The standard set of [DeltaBlockMergeRule]s used when parsing Quill Deltas. +const defaultBlockMergeRules = [ + MergeBlock(blockquoteAttribution), + MergeBlock(codeAttribution), +]; + +/// A rule that decides whether a given [DocumentNode] should be merged into +/// the node before it, when creating a [Document] from Quill Deltas. +/// +/// This is useful, for example, to place multiple lines of code within a +/// single code block. +abstract interface class DeltaBlockMergeRule { + /// Returns `true` if the block type should merge consecutive blocks, + /// `false` if it shouldn't, or `null` if this rule has no opinion + /// about the given [blockType]. + bool? shouldMerge(Attribution blockType); +} + +/// A [DeltaBlockMergeRule] that chooses to merge two blocks whose block +/// types are `==`. +class MergeBlock implements DeltaBlockMergeRule { + const MergeBlock(this._blockType); + + final Attribution _blockType; + + @override + bool? shouldMerge(Attribution blockType) { + if (blockType == _blockType) { + // Yes, try to merge them. + return true; + } + + // This isn't our block type. We don't have an opinion. + return null; + } +} From 74b63b80c23ed5ceef8332c41d165601c8c86885 Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Wed, 15 Jan 2025 22:30:11 -0800 Subject: [PATCH 3/3] Change block merge rule to take both block types in question, instead of just the next one. This is necessary to, e.g., compare the color of two BannerAttributions. --- .../lib/src/parsing/parser.dart | 57 ++++++++----------- 1 file changed, 25 insertions(+), 32 deletions(-) diff --git a/super_editor_quill/lib/src/parsing/parser.dart b/super_editor_quill/lib/src/parsing/parser.dart index 83405c11be..32bd5d8047 100644 --- a/super_editor_quill/lib/src/parsing/parser.dart +++ b/super_editor_quill/lib/src/parsing/parser.dart @@ -243,29 +243,10 @@ extension OperationParser on Operation { break; } + // Beginning with the last non-empty node, move backwards, collecting all + // nodes that should be merged into one. final nodeBeforeTrailingNewline = document.getNodeBefore(document.last)!; final blockTypeToMerge = nodeBeforeTrailingNewline.getMetadataValue(NodeMetadata.blockType); - var shouldMerge = false; - for (final rule in blockMergeRules) { - final ruleShouldMerge = rule.shouldMerge(blockTypeToMerge); - if (ruleShouldMerge == true) { - // The rule says we definitely want to merge. - shouldMerge = true; - break; - } - if (ruleShouldMerge == false) { - // The rule says we definitely don't want to merge. - shouldMerge = false; - break; - } - } - if (!shouldMerge) { - // Our merge rules don't want us to merge this node. - break; - } - - // The most recently inserted node supports merging. Check if we should merge with - // the previous node. var blocksToMerge = []; for (int i = document.nodeCount - 2; i >= 0; i -= 1) { final node = document.getNodeAt(i)!; @@ -273,9 +254,22 @@ extension OperationParser on Operation { break; } - if (node.getMetadataValue("blockType") != blockTypeToMerge) { - // This node doesn't have the same block type as the one(s) we want - // to merge. Break out the of loop. + var shouldMerge = false; + for (final rule in blockMergeRules) { + final ruleShouldMerge = rule.shouldMerge(blockTypeToMerge, node.getMetadataValue(NodeMetadata.blockType)); + if (ruleShouldMerge == true) { + // The rule says we definitely want to merge. + shouldMerge = true; + break; + } + if (ruleShouldMerge == false) { + // The rule says we definitely don't want to merge. + shouldMerge = false; + break; + } + } + if (!shouldMerge) { + // Our merge rules don't want us to merge this node. break; } @@ -589,22 +583,21 @@ const defaultBlockMergeRules = [ /// This is useful, for example, to place multiple lines of code within a /// single code block. abstract interface class DeltaBlockMergeRule { - /// Returns `true` if the block type should merge consecutive blocks, - /// `false` if it shouldn't, or `null` if this rule has no opinion - /// about the given [blockType]. - bool? shouldMerge(Attribution blockType); + /// Returns `true` if two consecutive blocks with the given types should merge, + /// `false` if they shouldn't, or `null` if this rule has no opinion about the merge. + bool? shouldMerge(Attribution block1, Attribution block2); } -/// A [DeltaBlockMergeRule] that chooses to merge two blocks whose block -/// types are `==`. +/// A [DeltaBlockMergeRule] that chooses to merge blocks whose type `==` +/// the given block type. class MergeBlock implements DeltaBlockMergeRule { const MergeBlock(this._blockType); final Attribution _blockType; @override - bool? shouldMerge(Attribution blockType) { - if (blockType == _blockType) { + bool? shouldMerge(Attribution block1, Attribution block2) { + if (block1 == _blockType && block2 == _blockType) { // Yes, try to merge them. return true; }