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

[Quill] - Make consecutive node merges configurable (Resolves #2510) #2511

Merged
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
8 changes: 7 additions & 1 deletion super_editor/clones/quill/lib/app.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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.
''');
}

Expand Down
5 changes: 4 additions & 1 deletion super_editor_quill/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Quill Delta, or vis-a-versa. `SuperEditor` internals are format agnostic.

## References
* Interactive Playground: https://v1.quilljs.com/playground/
130 changes: 113 additions & 17 deletions super_editor_quill/lib/src/parsing/parser.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,38 +22,75 @@ 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.
///
/// [blockMergeRules] explicitly tells the parser which consecutive
/// [DocumentNode]s should be merged together when not separated by an unstyled
/// newline in the given deltas.
///
/// 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<String, dynamic> deltaDocument, {
Editor? customEditor,
List<BlockDeltaFormat> blockFormats = defaultBlockFormats,
List<DeltaBlockMergeRule> blockMergeRules = defaultBlockMergeRules,
List<InlineDeltaFormat> inlineFormats = defaultInlineFormats,
List<InlineEmbedFormat> inlineEmbedFormats = const [],
List<BlockDeltaFormat> embedBlockFormats = defaultEmbedBockFormats,
}) {
return parseQuillDeltaOps(
deltaDocument["ops"],
customEditor: customEditor,
blockMergeRules: blockMergeRules,
blockFormats: blockFormats,
inlineFormats: inlineFormats,
inlineEmbedFormats: inlineEmbedFormats,
embedBlockFormats: embedBlockFormats,
);
}

/// 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
/// method is provided for convenience because in some situations only the
/// operations are exchanged, rather than the whole document object.
///
/// {@macro parse_deltas_custom_editor}
///
/// {@macro merge_consecutive_blocks}
MutableDocument parseQuillDeltaOps(
List<dynamic> deltaOps, {
Editor? customEditor,
List<BlockDeltaFormat> blockFormats = defaultBlockFormats,
List<DeltaBlockMergeRule> blockMergeRules = defaultBlockMergeRules,
List<InlineDeltaFormat> inlineFormats = defaultInlineFormats,
List<InlineEmbedFormat> inlineEmbedFormats = const [],
List<BlockDeltaFormat> embedBlockFormats = defaultEmbedBockFormats,
Expand Down Expand Up @@ -117,6 +154,7 @@ MutableDocument parseQuillDeltaOps(
delta.applyToDocument(
editor,
blockFormats: blockFormats,
blockMergeRules: blockMergeRules,
inlineFormats: inlineFormats,
inlineEmbedFormats: inlineEmbedFormats,
embedBlockFormats: embedBlockFormats,
Expand Down Expand Up @@ -179,6 +217,7 @@ extension OperationParser on Operation {
void applyToDocument(
Editor editor, {
required List<BlockDeltaFormat> blockFormats,
List<DeltaBlockMergeRule> blockMergeRules = defaultBlockMergeRules,
required List<InlineDeltaFormat> inlineFormats,
required List<InlineEmbedFormat> inlineEmbedFormats,
required List<BlockDeltaFormat> embedBlockFormats,
Expand All @@ -197,44 +236,65 @@ 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<MutableDocument>(Editor.documentKey);
if (document.nodeCount < 3) {
// Minimum of 3 nodes: code, code, newline.
// Minimum of 3 nodes: block, block, newline.
break;
}

var codeBlocks = <ParagraphNode>[];
// 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 blocksToMerge = <ParagraphNode>[];
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) {

var shouldMerge = false;
for (final rule in blockMergeRules) {
final ruleShouldMerge = rule.shouldMerge(blockTypeToMerge, node.getMetadataValue(NodeMetadata.blockType));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a small detail, we could cache node.getMetadataValue(NodeMetadata.blockType) in a variable outside of this for loop.

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

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:
Expand Down Expand Up @@ -457,7 +517,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,
Expand All @@ -476,7 +536,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,
Expand Down Expand Up @@ -510,3 +570,39 @@ enum DeltaOperationType {
retain,
delete,
}

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

Choose a reason for hiding this comment

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

Using a nullable bool looks a bit weird to me, because when reading the code that uses this, it isn't immediately obvious. I would suggest to use an enum instead.

}

/// 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 block1, Attribution block2) {
if (block1 == _blockType && block2 == _blockType) {
// Yes, try to merge them.
return true;
}

// This isn't our block type. We don't have an opinion.
return null;
}
}
Loading
Loading