Skip to content

Commit b24c838

Browse files
[SuperEditor] Fix crash due to conflicting attributions in SelectedTextColorStrategy (Resolves #2084) (#2086)
1 parent b70263a commit b24c838

File tree

4 files changed

+93
-11
lines changed

4 files changed

+93
-11
lines changed

attributed_text/lib/src/attributed_text.dart

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -171,20 +171,48 @@ class AttributedText {
171171
/// Adds the given [attribution] to all characters within the given
172172
/// [range], inclusive.
173173
///
174-
/// When [autoMerge] is `true`, the new attribution is merged with any
175-
/// preceding or following attribution whose [Attribution.canMergeWith] returns
176-
/// `true`.
174+
/// The effect of adding an attribution is straight forward when the text doesn't
175+
/// contain any other attributions with the same ID. However, there are various
176+
/// situations where the [attribution] can't necessarily co-exist with other
177+
/// attribution spans that already exist in the text.
178+
///
179+
/// Attribution overlaps can take one of two forms: mergeable or conflicting.
180+
///
181+
/// ## Mergeable Attribution Spans
182+
/// An example of a mergeable overlap is where two bold spans overlap each
183+
/// other. All bold attributions are interchangeable, so when two bold spans
184+
/// overlap, those spans can be merged together into a single span.
185+
///
186+
/// However, mergeable overlapping spans are not automatically merged. Instead,
187+
/// this decision is left to the user of this class. If you want [AttributedText] to
188+
/// merge overlapping mergeable spans, pass `true` for [autoMerge]. Otherwise,
189+
/// if [autoMerge] is `false`, an exception is thrown when two mergeable spans
190+
/// overlap each other.
191+
///
192+
///
193+
/// ## Conflicting Attribution Spans
194+
/// An example of a conflicting overlap is where a black text color overlaps a red
195+
/// text color. Text is either black, OR red, but never both. Therefore, the black
196+
/// attribution cannot co-exist with the red attribution. Something must be done
197+
/// to resolve this.
198+
///
199+
/// There are two possible ways to handle conflicting overlaps. The new attribution
200+
/// can overwrite the existing attribution where they overlap. Or, an exception can be
201+
/// thrown. To overwrite the existing attribution with the new attribution, pass `true`
202+
/// for [overwriteConflictingSpans]. Otherwise, if [overwriteConflictingSpans]
203+
/// is `false`, an exception is thrown.
177204
void addAttribution(
178205
Attribution attribution,
179206
SpanRange range, {
180207
bool autoMerge = true,
208+
bool overwriteConflictingSpans = false,
181209
}) {
182210
spans.addAttribution(
183211
newAttribution: attribution,
184212
start: range.start,
185213
end: range.end,
186214
autoMerge: autoMerge,
187-
overwriteConflictingSpans: false,
215+
overwriteConflictingSpans: overwriteConflictingSpans,
188216
);
189217
_notifyListeners();
190218
}

super_editor/lib/src/default_editor/layout_single_column/_styler_user_selection.dart

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -138,11 +138,15 @@ class SingleColumnLayoutSelectionStyler extends SingleColumnLayoutStylePhase {
138138
textSelection != null && _selectedTextColorStrategy != null && componentTextColor != null
139139
? (viewModel.text.copyText(0)
140140
..addAttribution(
141-
ColorAttribution(_selectedTextColorStrategy!(
142-
originalTextColor: componentTextColor,
143-
selectionHighlightColor: _selectionStyles.selectionColor,
144-
)),
145-
SpanRange(textSelection.start, textSelection.end - 1)))
141+
ColorAttribution(_selectedTextColorStrategy!(
142+
originalTextColor: componentTextColor,
143+
selectionHighlightColor: _selectionStyles.selectionColor,
144+
)),
145+
SpanRange(textSelection.start, textSelection.end - 1),
146+
// The selected range might already have a color attribution. We want to override it
147+
// with the selected text color.
148+
overwriteConflictingSpans: true,
149+
))
146150
: viewModel.text;
147151

148152
viewModel

super_editor/pubspec.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ dependencies:
4141
dependency_overrides:
4242
# # Override to local mono-repo path so devs can test this repo
4343
# # against changes that they're making to other mono-repo packages
44-
# attributed_text:
45-
# path: ../attributed_text
44+
attributed_text:
45+
path: ../attributed_text
4646
# super_text_layout:
4747
# path: ../super_text_layout
4848

super_editor/test/super_editor/supereditor_selection_test.dart

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,56 @@ void main() {
3838
expect(richText.getSpanForPosition(const TextPosition(offset: 6))!.style!.color, Colors.white);
3939
expect(richText.getSpanForPosition(const TextPosition(offset: 10))!.style!.color, Colors.white);
4040
});
41+
42+
testWidgetsOnAllPlatforms("overrides existing color attributions", (tester) async {
43+
final stylesheet = defaultStylesheet.copyWith(
44+
selectedTextColorStrategy: ({required Color originalTextColor, required Color selectionHighlightColor}) {
45+
return Colors.white;
46+
},
47+
);
48+
49+
// Pump an editor with green text throught the document.
50+
await tester //
51+
.createDocument()
52+
.withCustomContent(
53+
MutableDocument(
54+
nodes: [
55+
ParagraphNode(
56+
id: '1',
57+
text: AttributedText(
58+
'Lorem ipsum dolor',
59+
AttributedSpans(
60+
attributions: [
61+
SpanMarker(
62+
attribution: const ColorAttribution(Colors.green),
63+
offset: 0,
64+
markerType: SpanMarkerType.start),
65+
SpanMarker(
66+
attribution: const ColorAttribution(Colors.green),
67+
offset: 16,
68+
markerType: SpanMarkerType.end),
69+
],
70+
),
71+
),
72+
),
73+
],
74+
),
75+
)
76+
.useStylesheet(stylesheet)
77+
.pump();
78+
79+
// Double tap to select the word "Lorem".
80+
await tester.doubleTapInParagraph('1', 2);
81+
82+
// Ensure that the first word is white and the rest is green.
83+
final richText = SuperEditorInspector.findRichTextInParagraph('1');
84+
85+
expect(richText.getSpanForPosition(const TextPosition(offset: 0))!.style!.color, Colors.white);
86+
expect(richText.getSpanForPosition(const TextPosition(offset: 4))!.style!.color, Colors.white);
87+
88+
expect(richText.getSpanForPosition(const TextPosition(offset: 5))!.style!.color, Colors.green);
89+
expect(richText.getSpanForPosition(const TextPosition(offset: 16))!.style!.color, Colors.green);
90+
});
4191
});
4292

4393
testWidgetsOnArbitraryDesktop("calculates upstream document selection within a single node", (tester) async {

0 commit comments

Comments
 (0)