diff --git a/attributed_text/lib/src/attributed_text.dart b/attributed_text/lib/src/attributed_text.dart index f292fc858..8912128b5 100644 --- a/attributed_text/lib/src/attributed_text.dart +++ b/attributed_text/lib/src/attributed_text.dart @@ -451,6 +451,14 @@ class AttributedText { return spans.collapseSpans(contentLength: text.length); } + /// Returns a copy of this [AttributedText]. + AttributedText copy() { + return AttributedText( + text, + spans.copy(), + ); + } + @override bool operator ==(Object other) { return identical(this, other) || diff --git a/attributed_text/test/attributed_text_test.dart b/attributed_text/test/attributed_text_test.dart index 332a4232a..cf41d6f85 100644 --- a/attributed_text/test/attributed_text_test.dart +++ b/attributed_text/test/attributed_text_test.dart @@ -828,6 +828,77 @@ void main() { expect(spans[1].end, 10); }); }); + + group("copy >", () { + test("copies an AttributedText without any attributions", () { + final attributedText = AttributedText( + 'Sample Text', + ); + + expect(attributedText.copy(), AttributedText('Sample Text')); + }); + + test("copies an AttributedText with attributions", () { + final attributedText = AttributedText( + 'abcdefghij', + AttributedSpans( + attributions: [ + const SpanMarker( + attribution: ExpectedSpans.bold, + offset: 2, + markerType: SpanMarkerType.start, + ), + const SpanMarker( + attribution: ExpectedSpans.italics, + offset: 4, + markerType: SpanMarkerType.start, + ), + const SpanMarker( + attribution: ExpectedSpans.bold, + offset: 5, + markerType: SpanMarkerType.end, + ), + const SpanMarker( + attribution: ExpectedSpans.italics, + offset: 7, + markerType: SpanMarkerType.end, + ), + ], + ), + ); + + expect( + attributedText.copy(), + AttributedText( + 'abcdefghij', + AttributedSpans( + attributions: [ + const SpanMarker( + attribution: ExpectedSpans.bold, + offset: 2, + markerType: SpanMarkerType.start, + ), + const SpanMarker( + attribution: ExpectedSpans.italics, + offset: 4, + markerType: SpanMarkerType.start, + ), + const SpanMarker( + attribution: ExpectedSpans.bold, + offset: 5, + markerType: SpanMarkerType.end, + ), + const SpanMarker( + attribution: ExpectedSpans.italics, + offset: 7, + markerType: SpanMarkerType.end, + ), + ], + ), + ), + ); + }); + }); }); } diff --git a/super_editor/example/lib/demos/demo_markdown_serialization.dart b/super_editor/example/lib/demos/demo_markdown_serialization.dart index e504c3a43..ca2f6fcb3 100644 --- a/super_editor/example/lib/demos/demo_markdown_serialization.dart +++ b/super_editor/example/lib/demos/demo_markdown_serialization.dart @@ -137,11 +137,11 @@ MutableDocument _createInitialDocument() { AttributedSpans( attributions: [ SpanMarker( - attribution: LinkAttribution(url: Uri.https('example.org', '')), + attribution: LinkAttribution.fromUri(Uri.https('example.org', '')), offset: 30, markerType: SpanMarkerType.start), SpanMarker( - attribution: LinkAttribution(url: Uri.https('example.org', '')), + attribution: LinkAttribution.fromUri(Uri.https('example.org', '')), offset: 35, markerType: SpanMarkerType.end), ], diff --git a/super_editor/example/lib/demos/example_editor/_toolbar.dart b/super_editor/example/lib/demos/example_editor/_toolbar.dart index 4eae6b792..a00eec855 100644 --- a/super_editor/example/lib/demos/example_editor/_toolbar.dart +++ b/super_editor/example/lib/demos/example_editor/_toolbar.dart @@ -384,7 +384,7 @@ class _EditorToolbarState extends State { final trimmedRange = _trimTextRangeWhitespace(text, selectionRange); - final linkAttribution = LinkAttribution(url: Uri.parse(url)); + final linkAttribution = LinkAttribution.fromUri(Uri.parse(url)); widget.editor!.execute([ AddTextAttributionsRequest( @@ -664,35 +664,28 @@ class _EditorToolbarState extends State { child: Row( children: [ Expanded( - child: Focus( + child: SuperTextField( focusNode: _urlFocusNode, - parentNode: _popoverFocusNode, - // We use a SuperTextField instead of a TextField because TextField - // automatically re-parents its FocusNode, which causes #609. Flutter - // #106923 tracks the TextField issue. - child: SuperTextField( - focusNode: _urlFocusNode, - textController: _urlController, - minLines: 1, - maxLines: 1, - inputSource: TextInputSource.ime, - hintBehavior: HintBehavior.displayHintUntilTextEntered, - hintBuilder: (context) { - return const Text( - "enter a url...", - style: TextStyle( - color: Colors.grey, - fontSize: 16, - ), - ); - }, - textStyleBuilder: (_) { - return const TextStyle( - color: Colors.black, + textController: _urlController, + minLines: 1, + maxLines: 1, + inputSource: TextInputSource.ime, + hintBehavior: HintBehavior.displayHintUntilTextEntered, + hintBuilder: (context) { + return const Text( + "enter a url...", + style: TextStyle( + color: Colors.grey, fontSize: 16, - ); - }, - ), + ), + ); + }, + textStyleBuilder: (_) { + return const TextStyle( + color: Colors.black, + fontSize: 16, + ); + }, ), ), IconButton( diff --git a/super_editor/example/lib/demos/super_reader/example_document.dart b/super_editor/example/lib/demos/super_reader/example_document.dart index 8461bd9ec..123f8aaa0 100644 --- a/super_editor/example/lib/demos/super_reader/example_document.dart +++ b/super_editor/example/lib/demos/super_reader/example_document.dart @@ -98,11 +98,11 @@ Document createInitialDocument() { "Built by the Flutter Bounty Hunters", AttributedSpans(attributions: [ SpanMarker( - attribution: LinkAttribution(url: Uri.parse("https://flutterbountyhunters.com")), + attribution: LinkAttribution.fromUri(Uri.parse("https://flutterbountyhunters.com")), offset: 13, markerType: SpanMarkerType.start), SpanMarker( - attribution: LinkAttribution(url: Uri.parse("https://flutterbountyhunters.com")), + attribution: LinkAttribution.fromUri(Uri.parse("https://flutterbountyhunters.com")), offset: 34, markerType: SpanMarkerType.end), ]), diff --git a/super_editor/example_docs/lib/toolbar.dart b/super_editor/example_docs/lib/toolbar.dart index bb5ea003a..b4368b852 100644 --- a/super_editor/example_docs/lib/toolbar.dart +++ b/super_editor/example_docs/lib/toolbar.dart @@ -263,7 +263,7 @@ class _DocsEditorToolbarState extends State { final trimmedRange = _trimTextRangeWhitespace(text, selectionRange); - final linkAttribution = LinkAttribution(url: Uri.parse(url)); + final linkAttribution = LinkAttribution.fromUri(Uri.parse(url)); widget.editor.execute([ AddTextAttributionsRequest( @@ -1156,35 +1156,28 @@ class _DocsEditorToolbarState extends State { child: Row( children: [ Expanded( - child: Focus( + child: SuperTextField( focusNode: _urlFocusNode, - - // We use a SuperTextField instead of a TextField because TextField - // automatically re-parents its FocusNode, which causes #609. Flutter - // #106923 tracks the TextField issue. - child: SuperTextField( - focusNode: _urlFocusNode, - textController: _urlController, - minLines: 1, - maxLines: 1, - inputSource: TextInputSource.ime, - hintBehavior: HintBehavior.displayHintUntilTextEntered, - hintBuilder: (context) { - return const Text( - "enter a url...", - style: TextStyle( - color: Colors.grey, - fontSize: 16, - ), - ); - }, - textStyleBuilder: (_) { - return const TextStyle( - color: Colors.black, + textController: _urlController, + minLines: 1, + maxLines: 1, + inputSource: TextInputSource.ime, + hintBehavior: HintBehavior.displayHintUntilTextEntered, + hintBuilder: (context) { + return const Text( + "enter a url...", + style: TextStyle( + color: Colors.grey, fontSize: 16, - ); - }, - ), + ), + ); + }, + textStyleBuilder: (_) { + return const TextStyle( + color: Colors.black, + fontSize: 16, + ); + }, ), ), IconButton( diff --git a/super_editor/lib/src/default_editor/attributions.dart b/super_editor/lib/src/default_editor/attributions.dart index 6ba2a5ab3..7265c681c 100644 --- a/super_editor/lib/src/default_editor/attributions.dart +++ b/super_editor/lib/src/default_editor/attributions.dart @@ -151,14 +151,29 @@ class FontSizeAttribution implements Attribution { /// within [AttributedText]. This class doesn't have a special /// relationship with [AttributedText]. class LinkAttribution implements Attribution { - LinkAttribution({ - required this.url, - }); + factory LinkAttribution.fromUri(Uri uri) { + return LinkAttribution(uri.toString()); + } + + const LinkAttribution(this.url); @override String get id => 'link'; - final Uri url; + /// The URL associated with the attributed text, as a `String`. + final String url; + + /// Attempts to parse the [url] as a [Uri], and returns `true` if the [url] + /// is successfully parsed, or `false` if parsing fails, such as due to the [url] + /// including an invalid scheme, separator syntax, extra segments, etc. + bool get hasValidUri => Uri.tryParse(url) != null; + + /// The URL associated with the attributed text, as a `Uri`. + /// + /// Accessing the [uri] throws an exception if the [url] isn't valid. + /// To access a URL that might not be valid, consider accessing the [url], + /// instead. + Uri get uri => Uri.parse(url); @override bool canMergeWith(Attribution other) { diff --git a/super_editor/lib/src/default_editor/common_editor_operations.dart b/super_editor/lib/src/default_editor/common_editor_operations.dart index 47fc7a388..4f3efd4ca 100644 --- a/super_editor/lib/src/default_editor/common_editor_operations.dart +++ b/super_editor/lib/src/default_editor/common_editor_operations.dart @@ -2380,7 +2380,7 @@ class PasteEditorCommand implements EditCommand { if (link != null && link.hasScheme && link.hasAuthority) { // Valid url. Apply [LinkAttribution] to the url - final linkAttribution = LinkAttribution(url: link); + final linkAttribution = LinkAttribution.fromUri(link); final startOffset = wordBoundary.start; // -1 because TextPosition's offset indexes the character after the diff --git a/super_editor/lib/src/default_editor/default_document_editor_reactions.dart b/super_editor/lib/src/default_editor/default_document_editor_reactions.dart index a2a954f1a..51cb29247 100644 --- a/super_editor/lib/src/default_editor/default_document_editor_reactions.dart +++ b/super_editor/lib/src/default_editor/default_document_editor_reactions.dart @@ -637,7 +637,7 @@ class LinkifyReaction implements EditReaction { final uri = _parseLink(word); text.addAttribution( - LinkAttribution(url: uri), + LinkAttribution.fromUri(uri), SpanRange(wordStartOffset, endOffset - 1), ); } @@ -798,8 +798,8 @@ class LinkifyReaction implements EditReaction { // link attribution that reflects the edited URL text. We do that below. if (updatePolicy == LinkUpdatePolicy.update) { changedNodeText.addAttribution( - LinkAttribution( - url: _parseLink(changedNodeText.text.substring(rangeToUpdate.start, rangeToUpdate.end + 1)), + LinkAttribution.fromUri( + _parseLink(changedNodeText.text.substring(rangeToUpdate.start, rangeToUpdate.end + 1)), ), rangeToUpdate, ); diff --git a/super_editor/lib/src/default_editor/document_caret_overlay.dart b/super_editor/lib/src/default_editor/document_caret_overlay.dart index 29503ba77..59a128f1b 100644 --- a/super_editor/lib/src/default_editor/document_caret_overlay.dart +++ b/super_editor/lib/src/default_editor/document_caret_overlay.dart @@ -164,7 +164,8 @@ class CaretDocumentOverlayState extends DocumentLayoutLayerState= overlayBox.size.width) { diff --git a/super_editor/lib/src/default_editor/document_gestures_mouse.dart b/super_editor/lib/src/default_editor/document_gestures_mouse.dart index f762f3770..06c548606 100644 --- a/super_editor/lib/src/default_editor/document_gestures_mouse.dart +++ b/super_editor/lib/src/default_editor/document_gestures_mouse.dart @@ -577,13 +577,6 @@ class _DocumentMouseInteractorState extends State with } } - /// Beginning with Flutter 3.3.3, we are responsible for starting and - /// stopping scroll momentum. This method cancels any scroll momentum - /// in our scroll controller. - void _cancelScrollMomentum() { - widget.autoScroller.goIdle(); - } - void _updateDragSelection() { if (_dragEndGlobal == null) { // User isn't dragging. No need to update drag selection. @@ -729,7 +722,6 @@ Updating drag selection: } void _onMouseMove(PointerHoverEvent event) { - _cancelScrollMomentum(); _updateMouseCursor(event.position); _lastHoverOffset = event.position; } diff --git a/super_editor/lib/src/default_editor/document_gestures_touch_android.dart b/super_editor/lib/src/default_editor/document_gestures_touch_android.dart index 01953f337..54c27b746 100644 --- a/super_editor/lib/src/default_editor/document_gestures_touch_android.dart +++ b/super_editor/lib/src/default_editor/document_gestures_touch_android.dart @@ -1750,11 +1750,7 @@ class SuperEditorAndroidControlsOverlayManagerState extends State impl Widget build(BuildContext context) { return SuperEditorImeDebugVisuals( imeConnection: _imeConnection, - child: Actions( - actions: defaultTargetPlatform == TargetPlatform.macOS ? disabledMacIntents : {}, + child: IntentBlocker( + intents: CurrentPlatform.isApple ? appleBlockedIntents : nonAppleBlockedIntents, child: SuperEditorHardwareKeyHandler( focusNode: _focusNode, editContext: widget.editContext, diff --git a/super_editor/lib/src/default_editor/list_items.dart b/super_editor/lib/src/default_editor/list_items.dart index 1e7f26973..cba6422d7 100644 --- a/super_editor/lib/src/default_editor/list_items.dart +++ b/super_editor/lib/src/default_editor/list_items.dart @@ -112,10 +112,18 @@ class ListItemComponentBuilder implements ComponentBuilder { int? ordinalValue; if (node.type == ListItemType.ordered) { + // Counts the number of ordered list items above the current node with the same indentation level. Ordered + // list items with the same indentation level might be separated by ordered or unordered list items with + // different indentation levels. ordinalValue = 1; DocumentNode? nodeAbove = document.getNodeBefore(node); while (nodeAbove != null && nodeAbove is ListItemNode && nodeAbove.indent >= node.indent) { if (nodeAbove.indent == node.indent) { + if (nodeAbove.type != ListItemType.ordered) { + // We found an unordered list item with the same indentation level as the ordered list item. + // Other ordered list items aboce this one do not belong to the same list. + break; + } ordinalValue = ordinalValue! + 1; } nodeAbove = document.getNodeBefore(nodeAbove); @@ -322,11 +330,15 @@ class _UnorderedListItemComponentState extends State @override Widget build(BuildContext context) { - final textStyle = widget.styleBuilder({}); + // Usually, the font size is obtained via the stylesheet. But the attributions might + // also contain a FontSizeAttribution, which overrides the stylesheet. Use the attributions + // of the first character to determine the text style. + final attributions = widget.text.getAllAttributionsAt(0).toSet(); + final textStyle = widget.styleBuilder(attributions); + final indentSpace = widget.indentCalculator(textStyle, widget.indent); final textScaler = MediaQuery.textScalerOf(context); final lineHeight = textScaler.scale(textStyle.fontSize! * (textStyle.height ?? 1.25)); - const manualVerticalAdjustment = 3.0; return ProxyTextDocumentComponent( key: widget.componentKey, @@ -336,7 +348,6 @@ class _UnorderedListItemComponentState extends State children: [ Container( width: indentSpace, - margin: const EdgeInsets.only(top: manualVerticalAdjustment), decoration: BoxDecoration( border: widget.showDebugPaint ? Border.all(width: 1, color: Colors.grey) : null, ), @@ -368,16 +379,38 @@ class _UnorderedListItemComponentState extends State typedef UnorderedListItemDotBuilder = Widget Function(BuildContext, UnorderedListItemComponent); Widget _defaultUnorderedListItemDotBuilder(BuildContext context, UnorderedListItemComponent component) { + // Usually, the font size is obtained via the stylesheet. But the attributions might + // also contain a FontSizeAttribution, which overrides the stylesheet. Use the attributions + // of the first character to determine the text style. + final attributions = component.text.getAllAttributionsAt(0).toSet(); + final textStyle = component.styleBuilder(attributions); + return Align( alignment: Alignment.centerRight, - child: Container( - width: 4, - height: 4, - margin: const EdgeInsets.only(right: 10), - decoration: BoxDecoration( - shape: BoxShape.circle, - color: component.styleBuilder({}).color, + child: Text.rich( + TextSpan( + // Place a zero-width joiner before the bullet point to make it properly aligned. Without this, + // the bullet point is not vertically centered with the text, even when setting the textStyle + // on the whole rich text or WidgetSpan. + text: '\u200C', + style: textStyle, + children: [ + WidgetSpan( + alignment: PlaceholderAlignment.middle, + child: Container( + width: 4, + height: 4, + margin: const EdgeInsets.only(right: 10), + decoration: BoxDecoration( + shape: BoxShape.circle, + color: textStyle.color, + ), + ), + ), + ], ), + // Don't scale the dot. + textScaler: const TextScaler.linear(1.0), ), ); } @@ -440,7 +473,12 @@ class _OrderedListItemComponentState extends State { @override Widget build(BuildContext context) { - final textStyle = widget.styleBuilder({}); + // Usually, the font size is obtained via the stylesheet. But the attributions might + // also contain a FontSizeAttribution, which overrides the stylesheet. Use the attributions + // of the first character to determine the text style. + final attributions = widget.text.getAllAttributionsAt(0).toSet(); + final textStyle = widget.styleBuilder(attributions); + final indentSpace = widget.indentCalculator(textStyle, widget.indent); final textScaler = MediaQuery.textScalerOf(context); final lineHeight = textScaler.scale(textStyle.fontSize! * (textStyle.height ?? 1.0)); @@ -489,6 +527,12 @@ double _defaultIndentCalculator(TextStyle textStyle, int indent) { } Widget _defaultOrderedListItemNumeralBuilder(BuildContext context, OrderedListItemComponent component) { + // Usually, the font size is obtained via the stylesheet. But the attributions might + // also contain a FontSizeAttribution, which overrides the stylesheet. Use the attributions + // of the first character to determine the text style. + final attributions = component.text.getAllAttributionsAt(0).toSet(); + final textStyle = component.styleBuilder(attributions); + return OverflowBox( maxWidth: double.infinity, maxHeight: double.infinity, @@ -499,7 +543,7 @@ Widget _defaultOrderedListItemNumeralBuilder(BuildContext context, OrderedListIt child: Text( '${component.listIndex}.', textAlign: TextAlign.right, - style: component.styleBuilder({}).copyWith(), + style: textStyle, ), ), ), diff --git a/super_editor/lib/src/default_editor/super_editor.dart b/super_editor/lib/src/default_editor/super_editor.dart index 695bb8ff9..0a2b21a53 100644 --- a/super_editor/lib/src/default_editor/super_editor.dart +++ b/super_editor/lib/src/default_editor/super_editor.dart @@ -1578,7 +1578,7 @@ class SuperEditorLaunchLinkTapHandler extends ContentTapDelegate { final tappedAttributions = textNode.text.getAllAttributionsAt(nodePosition.offset); for (final tappedAttribution in tappedAttributions) { if (tappedAttribution is LinkAttribution) { - return tappedAttribution.url; + return tappedAttribution.uri; } } diff --git a/super_editor/lib/src/default_editor/text.dart b/super_editor/lib/src/default_editor/text.dart index 97a0fc3fe..735c3d5e7 100644 --- a/super_editor/lib/src/default_editor/text.dart +++ b/super_editor/lib/src/default_editor/text.dart @@ -1480,6 +1480,14 @@ class ToggleTextAttributionsCommand implements EditCommand { editorDocLog.info(' - selecting part of the first node: ${textNode.id}'); startOffset = (normalizedRange.start.nodePosition as TextPosition).offset; endOffset = max(textNode.text.length - 1, 0); + + if (startOffset >= textNode.text.length) { + // The range spans multiple nodes, starting at the end of the first node of the + // range. From the first node's perspective, this is equivalent to a collapsed + // selection at the end of the node. There's no text to toggle any attributions. + // Skip this node. + continue; + } } else if (textNode == nodes.last) { // Handle partial node selection in last node. editorDocLog.info(' - toggling part of the last node: ${textNode.id}'); @@ -1488,6 +1496,14 @@ class ToggleTextAttributionsCommand implements EditCommand { // -1 because TextPosition's offset indexes the character after the // selection, not the final character in the selection. endOffset = (normalizedRange.end.nodePosition as TextPosition).offset - 1; + + if (endOffset <= 0) { + // The range spans multiple nodes, ending at the beginning of the last node of the + // range. From the last node's perspective, this is equivalent to a collapsed + // selection at the beginning of the node. There's no text to toggle any attributions. + // Skip this node. + continue; + } } else { // Handle full node selection. editorDocLog.info(' - toggling full node: ${textNode.id}'); diff --git a/super_editor/lib/src/infrastructure/actions.dart b/super_editor/lib/src/infrastructure/actions.dart new file mode 100644 index 000000000..c2b085585 --- /dev/null +++ b/super_editor/lib/src/infrastructure/actions.dart @@ -0,0 +1,201 @@ +import 'package:flutter/widgets.dart'; + +/// Blocks a set of intents from launching [Action]s. +/// +/// An [IntentBlocker] blocks [Action]s from running by stopping the associated +/// [Intent]s from bubbling any further up the widget tree. +/// +/// Flutter includes default [Action]s at app-level that might not be desirable for +/// all applications. +/// +/// To prevent [Intent]s from bubbling up to the default [Action]s, locate any widget +/// below the [Actions] widget that handles the desired [Intent]s (typically [MaterialApp] +/// or [WidgetsApp]), and wrap it with an [IntentBlocker] widget, passing the types +/// of the [Intent]s that should be blocked into the [intents] parameter. +/// +/// For example, the following code blocks [ScrollIntent]s from bubbling up: +/// +/// IntentBlocker( +/// intents: { ScrollIntent }, +/// child: child, +/// ) +class IntentBlocker extends StatelessWidget { + const IntentBlocker({ + super.key, + required this.intents, + required this.child, + }); + + /// The types of intents that should be blocked. + final Set intents; + + /// The rest of the widget tree. + final Widget child; + + @override + Widget build(BuildContext context) { + final actions = >{}; + for (final intent in intents) { + actions[intent] = DoNothingAction(consumesKey: false); + } + + return Actions( + actions: { + ...actions, + // Flutter might dispatch Intents individually or as a group. We want + // to also block any desired Intents when they are inside a group. + PrioritizedIntents: _BlockIntentInsideGroupAction( + intents: intents, + ) + }, + child: child, + ); + } +} + +/// A set of [Intent]s, which Flutter dispatches by default on non-Apple +/// platforms (Android, Windows, Linux), that should have its associated +/// [Action]s blocked. +/// +/// {@template flutter_default_actions} +/// For example: The user presses the SPACE key on web. By default +/// Flutter emits an `ActivateIntent` and a `ScrollIntent`. But you +/// probably want to insert a " " in some text instead of activating +/// or scrolling. Those default Flutter intents must be blocked for +/// two reasons. First, you don't want to scroll down every time you +/// type a space. Second, the SPACE key event won't have a chance to +/// be handled by the OS IME if Flutter handles the key with an [Action]. +/// Therefore, you should preveng this set of [Intent]s from bubbling up, +/// to block those default Flutter behaviors, let the IME handle the key event, +/// and enjoy expected text input. +/// {@endtemplate} +/// +/// To prevent the default Flutter [Action]s, locate the specific widget +/// where you want to run non-default behaviors, such as inserting a space +/// instead of scrolling. Wrap that widget with an [IntentBlocker] widget, and +/// then pass this set of [Intent] types to block the default behaviors: +/// +/// IntentBlocker( +/// intents: nonAppleBlockedIntents, +/// child: SuperEditor(), +/// ) +/// +/// See [WidgetsApp.defaultShortcuts] for the list of keybindings that Flutter +/// adds by default. +final Set nonAppleBlockedIntents = { + ActivateIntent, + ScrollIntent, +}; + +/// A set of [Intent]s, which Flutter dispatches by default on Apple +/// platforms (macOS and iOS), that should have its associated +/// [Action]s blocked. +/// +/// {@macro flutter_default_actions} +/// +/// To prevent the default Flutter [Action]s, locate the specific widget +/// where you want to run non-default behaviors, such as inserting a space +/// instead of scrolling. Wrap that widget with an [IntentBlocker] widget, and +/// then pass this set of [Intent] types to block the default behaviors: +/// +/// IntentBlocker( +/// intents: appleBlockedIntents, +/// child: SuperEditor(), +/// ) +/// +/// See [WidgetsApp.defaultShortcuts] for the list of keybindings that Flutter +/// adds by default. +final Set appleBlockedIntents = { + // Generated by pressing LEFT/RIGHT ARROW. + ExtendSelectionByCharacterIntent, + // Generated by pressing UP/DOWN ARROW. + ExtendSelectionVerticallyToAdjacentLineIntent, + // Generated by pressing PAGE UP/DOWN. + ScrollIntent, + // Generated by pressing HOME/END. + ScrollToDocumentBoundaryIntent, + // Generated by pressing TAB. + NextFocusIntent, + // Generated by pressing SHIFT + TAB. + PreviousFocusIntent, + // Generated by pressing SPACE. + ActivateIntent, +}; + +/// An [Action], which blocks certain [Intent]s dispatched inside a group of [Intent]s +/// from launching other [Action]s. +/// +/// A [_BlockIntentInsideGroupAction] blocks other [Action]s from running by stopping +/// the associated [Intent]s from bubbling any further up the widget tree. +/// +/// To prevent [Intent]s inside of a group from bubbling up to the default [Action]s, locate any widget +/// below the [Actions] widget that handles the desired [Intent]s (typically [MaterialApp] +/// or [WidgetsApp]), and wrap it with an [Actions] widget. Then, associate [PrioritizedIntents] +/// with [_BlockIntentInsideGroupAction] pass the set of [Intent]s that should be blocked. +/// +/// For example, the following code blocks [ScrollIntent]s from bubbling up: +/// +/// Actions( +/// actions: { +/// PrioritizedIntents: _BlockIntentInsideGroupAction( +/// intents: { ScrollIntent }, +/// ) +/// }, +/// child: child, +/// ); +class _BlockIntentInsideGroupAction extends Action { + _BlockIntentInsideGroupAction({ + required this.intents, + }); + + final Set intents; + + @override + bool consumesKey(PrioritizedIntents intent) => false; + + @override + void invoke(PrioritizedIntents intent) {} + + @override + bool isEnabled(PrioritizedIntents intent, [BuildContext? context]) { + final FocusNode? focus = primaryFocus; + if (focus == null || focus.context == null) { + return false; + } + + for (final candidateIntent in intent.orderedIntents) { + if (_hasEnabledAction(candidateIntent, context)) { + // Flutter wants to run an Action for this intent. + if (intents.contains(candidateIntent.runtimeType)) { + // We want to prevent this intent from bubbling up. Return `true` to + // signal to Flutter that we want to handle it. + return true; + } + + // We don't care about the intent that is going to have its corresponding + // Action executed. Let it bubble up so Flutter will execute it. + return false; + } + } + + // We didn't find any intents with a corresponding enabled Action. Let the + // intent bubble up. + return false; + } + + bool _hasEnabledAction(Intent intent, BuildContext? context) { + final Action? candidateAction = Actions.maybeFind( + primaryFocus!.context!, + intent: intent, + ); + + if (candidateAction == null) { + // We didn't find an Action associated with the given intent. + return false; + } + + return (candidateAction is ContextAction) + ? candidateAction.isEnabled(intent, context) + : candidateAction.isEnabled(intent); + } +} diff --git a/super_editor/lib/src/infrastructure/content_layers.dart b/super_editor/lib/src/infrastructure/content_layers.dart index 240260eb4..d702aedf5 100644 --- a/super_editor/lib/src/infrastructure/content_layers.dart +++ b/super_editor/lib/src/infrastructure/content_layers.dart @@ -2,6 +2,7 @@ import 'package:flutter/foundation.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter/scheduler.dart'; import 'package:flutter/widgets.dart'; +import 'package:logging/logging.dart'; import 'package:super_editor/src/infrastructure/_logging.dart'; /// Widget that displays [content] above a number of [underlays], and beneath a number of @@ -208,31 +209,48 @@ class ContentLayersElement extends RenderObjectElement { contentLayersLog.finer("Checking underlays"); for (final underlay in _underlays) { - contentLayersLog.finer(" - Is underlay ($underlay) subtree dirty? ${_isSubtreeDirty(underlay)}"); + contentLayersLog.finer(() => " - Is underlay ($underlay) subtree dirty? ${_isSubtreeDirty(underlay)}"); hasDirtyElements = hasDirtyElements || _isSubtreeDirty(underlay); } contentLayersLog.finer("Checking overlays"); for (final overlay in _overlays) { - contentLayersLog.finer(" - Is overlay ($overlay) subtree dirty? ${_isSubtreeDirty(overlay)}"); + contentLayersLog.finer(() => " - Is overlay ($overlay) subtree dirty? ${_isSubtreeDirty(overlay)}"); hasDirtyElements = hasDirtyElements || _isSubtreeDirty(overlay); } return hasDirtyElements; } + static bool _isDirty = false; + bool _isSubtreeDirty(Element element) { - contentLayersLog.finest("Finding dirty children for: $element"); - if (element.dirty) { - contentLayersLog.finest("Found a dirty child: $element"); + _isDirty = false; + element.visitChildren(_isSubtreeDirtyVisitor); + return _isDirty; + } + +// This is intentionally static to prevent closure allocation during + // the traversal of the element tree. + static void _isSubtreeDirtyVisitor(Element element) { + // Can't use the () => message syntax because it allocates a closure. + assert(() { + if (contentLayersLog.isLoggable(Level.FINEST)) { + contentLayersLog.finest("Finding dirty children for: $element"); + } return true; + }()); + if (element.dirty) { + assert(() { + if (contentLayersLog.isLoggable(Level.FINEST)) { + contentLayersLog.finest("Found a dirty child: $element"); + } + return true; + }()); + _isDirty = true; + return; } - - bool isDirty = false; - element.visitChildren((childElement) { - isDirty = isDirty || _isSubtreeDirty(childElement); - }); - return isDirty; + element.visitChildren(_isSubtreeDirtyVisitor); } void _onContentBuildScheduled() { diff --git a/super_editor/lib/src/infrastructure/focus.dart b/super_editor/lib/src/infrastructure/focus.dart deleted file mode 100644 index 5cf0b8d61..000000000 --- a/super_editor/lib/src/infrastructure/focus.dart +++ /dev/null @@ -1,641 +0,0 @@ -import 'package:flutter/foundation.dart'; -import 'package:flutter/widgets.dart'; - -/// Widget that responds to keyboard events for a given [focusNode] without -/// necessarily re-parenting the [focusNode]. -/// -/// The [focusNode] is only re-parented if its parent is `null`. -/// -/// The traditional [Focus] widget provides an `onKey` property, but that widget -/// automatically re-parents the [FocusNode] based on the structure of the widget -/// tree. Re-parenting is a problem in some situations, e.g., a popover toolbar -/// that appears while editing a document. The toolbar and the document are on -/// different branches of the widget tree, but they need to share focus. That shared -/// focus is impossible when the [Focus] widget forces re-parenting. The -/// [NonReparentingFocus] widget provides an [onKey] property without re-parenting the -/// given [focusNode]. -class NonReparentingFocus extends StatefulWidget { - const NonReparentingFocus({ - Key? key, - required this.focusNode, - this.onKeyEvent, - required this.child, - }) : super(key: key); - - /// The [FocusNode] that sends key events to [onKey]. - final FocusNode focusNode; - - - /// The callback invoked whenever [focusNode] receives [KeyEvent] events. - final FocusOnKeyEventCallback? onKeyEvent; - - /// The child of this widget. - final Widget child; - - @override - State createState() => _NonReparentingFocusState(); -} - -class _NonReparentingFocusState extends State { - late FocusAttachment _keyboardFocusAttachment; - - @override - void initState() { - super.initState(); - _keyboardFocusAttachment = widget.focusNode.attach(context, onKeyEvent: _onKeyEvent); - } - - @override - void didChangeDependencies() { - super.didChangeDependencies(); - _reparentIfMissingParent(); - } - - @override - void didUpdateWidget(NonReparentingFocus oldWidget) { - super.didUpdateWidget(oldWidget); - - if (widget.focusNode != oldWidget.focusNode) { - _keyboardFocusAttachment.detach(); - _keyboardFocusAttachment = widget.focusNode.attach(context, onKeyEvent: _onKeyEvent); - _reparentIfMissingParent(); - } - } - - @override - void dispose() { - _keyboardFocusAttachment.detach(); - super.dispose(); - } - - void _reparentIfMissingParent() { - if (widget.focusNode.parent == null) { - _keyboardFocusAttachment.reparent(); - } - } - - KeyEventResult _onKeyEvent(FocusNode focusNode, KeyEvent event) { - return widget.onKeyEvent?.call(focusNode, event) ?? KeyEventResult.ignored; - } - - @override - Widget build(BuildContext context) { - _reparentIfMissingParent(); - - return widget.child; - } -} - -/// Copy of [Focus] that configures its [FocusNode] with the -/// given [parentNode], rather than automatically re-parenting based -/// on the widget tree structure. -/// -/// One reason to attach a [FocusNode] to a parent [FocusNode] that -/// isn't an ancestor in the widget tree is an editor with a popover -/// toolbar that contains a text field for entering a link URL. With -/// this widget, the editor's [FocusNode] can become the parent of the -/// toolbar's [FocusNode], which allows the editor to remain "focused" -/// while the user types a URL into the toolbar's text field, which has -/// "primary focus". -/// -/// The only changes made to [Focus] are the parts related to re-parenting. -/// As a result, there may be [Focus] behaviors that don't work. We don't -/// want to invest the time to re-write all the standard [Focus] tests. -/// Hopefully, Flutter #106923 will result in changes to [Focus] that make -/// [FocusWithCustomParent] superfluous, and we can remove it. -class FocusWithCustomParent extends StatefulWidget { - const FocusWithCustomParent({ - Key? key, - this.focusNode, - this.parentFocusNode, - this.autofocus = false, - this.onFocusChange, - FocusOnKeyEventCallback? onKeyEvent, - FocusOnKeyCallback? onKey, - bool? canRequestFocus, - bool? skipTraversal, - bool? descendantsAreFocusable, - bool? descendantsAreTraversable, - this.includeSemantics = true, - String? debugLabel, - required this.child, - }) : _onKeyEvent = onKeyEvent, - _onKey = onKey, - _canRequestFocus = canRequestFocus, - _skipTraversal = skipTraversal, - _descendantsAreFocusable = descendantsAreFocusable, - _descendantsAreTraversable = descendantsAreTraversable, - _debugLabel = debugLabel, - super(key: key); - - // Indicates whether the widget's focusNode attributes should have priority - // when then widget is updated. - bool get _usingExternalFocus => false; - - /// The child widget of this [FocusWithCustomParent]. - /// - /// {@macro flutter.widgets.ProxyWidget.child} - final Widget child; - - /// {@template flutter.widgets.Focus.focusNode} - /// An optional focus node to use as the focus node for this widget. - /// - /// If one is not supplied, then one will be automatically allocated, owned, - /// and managed by this widget. The widget will be focusable even if a - /// [focusNode] is not supplied. If supplied, the given `focusNode` will be - /// _hosted_ by this widget, but not owned. See [FocusNode] for more - /// information on what being hosted and/or owned implies. - /// - /// Supplying a focus node is sometimes useful if an ancestor to this widget - /// wants to control when this widget has the focus. The owner will be - /// responsible for calling [FocusNode.dispose] on the focus node when it is - /// done with it, but this widget will attach/detach and reparent the node - /// when needed. - /// {@endtemplate} - /// - /// A non-null [focusNode] must be supplied if using the - /// [Focus.withExternalFocusNode] constructor is used. - final FocusNode? focusNode; - - /// The [FocusNode] that's installed as the parent of [focusNode], regardless - /// of the location of [parentFocusNode] in the widget tree. - final FocusNode? parentFocusNode; - - /// {@template flutter.widgets.Focus.autofocus} - /// True if this widget will be selected as the initial focus when no other - /// node in its scope is currently focused. - /// - /// Ideally, there is only one widget with autofocus set in each [FocusScope]. - /// If there is more than one widget with autofocus set, then the first one - /// added to the tree will get focus. - /// - /// Must not be null. Defaults to false. - /// {@endtemplate} - final bool autofocus; - - /// Handler called when the focus changes. - /// - /// Called with true if this widget's node gains focus, and false if it loses - /// focus. - final ValueChanged? onFocusChange; - - /// A handler for keys that are pressed when this object or one of its - /// children has focus. - /// - /// Key events are first given to the [FocusNode] that has primary focus, and - /// if its [onKeyEvent] method returns [KeyEventResult.ignored], then they are - /// given to each ancestor node up the focus hierarchy in turn. If an event - /// reaches the root of the hierarchy, it is discarded. - /// - /// This is not the way to get text input in the manner of a text field: it - /// leaves out support for input method editors, and doesn't support soft - /// keyboards in general. For text input, consider [TextField], - /// [EditableText], or [CupertinoTextField] instead, which do support these - /// things. - FocusOnKeyEventCallback? get onKeyEvent => _onKeyEvent ?? focusNode?.onKeyEvent; - final FocusOnKeyEventCallback? _onKeyEvent; - - /// A handler for keys that are pressed when this object or one of its - /// children has focus. - /// - /// This is a legacy API based on [RawKeyEvent] and will be deprecated in the - /// future. Prefer [onKeyEvent] instead. - /// - /// Key events are first given to the [FocusNode] that has primary focus, and - /// if its [onKey] method return false, then they are given to each ancestor - /// node up the focus hierarchy in turn. If an event reaches the root of the - /// hierarchy, it is discarded. - /// - /// This is not the way to get text input in the manner of a text field: it - /// leaves out support for input method editors, and doesn't support soft - /// keyboards in general. For text input, consider [TextField], - /// [EditableText], or [CupertinoTextField] instead, which do support these - /// things. - FocusOnKeyCallback? get onKey => _onKey ?? focusNode?.onKey; - final FocusOnKeyCallback? _onKey; - - /// {@template flutter.widgets.Focus.canRequestFocus} - /// If true, this widget may request the primary focus. - /// - /// Defaults to true. Set to false if you want the [FocusNode] this widget - /// manages to do nothing when [FocusNode.requestFocus] is called on it. Does - /// not affect the children of this node, and [FocusNode.hasFocus] can still - /// return true if this node is the ancestor of the primary focus. - /// - /// This is different than [FocusWithCustomParent.skipTraversal] because [FocusWithCustomParent.skipTraversal] - /// still allows the widget to be focused, just not traversed to. - /// - /// Setting [FocusNode.canRequestFocus] to false implies that the widget will - /// also be skipped for traversal purposes. - /// - /// See also: - /// - /// * [FocusTraversalGroup], a widget that sets the traversal policy for its - /// descendants. - /// * [FocusTraversalPolicy], a class that can be extended to describe a - /// traversal policy. - /// {@endtemplate} - bool get canRequestFocus => _canRequestFocus ?? focusNode?.canRequestFocus ?? true; - final bool? _canRequestFocus; - - /// Sets the [FocusNode.skipTraversal] flag on the focus node so that it won't - /// be visited by the [FocusTraversalPolicy]. - /// - /// This is sometimes useful if a [FocusWithCustomParent] widget should receive key events as - /// part of the focus chain, but shouldn't be accessible via focus traversal. - /// - /// This is different from [FocusNode.canRequestFocus] because it only implies - /// that the widget can't be reached via traversal, not that it can't be - /// focused. It may still be focused explicitly. - bool get skipTraversal => _skipTraversal ?? focusNode?.skipTraversal ?? false; - final bool? _skipTraversal; - - /// {@template flutter.widgets.Focus.descendantsAreFocusable} - /// If false, will make this widget's descendants unfocusable. - /// - /// Defaults to true. Does not affect focusability of this node (just its - /// descendants): for that, use [FocusNode.canRequestFocus]. - /// - /// If any descendants are focused when this is set to false, they will be - /// unfocused. When `descendantsAreFocusable` is set to true again, they will - /// not be refocused, although they will be able to accept focus again. - /// - /// Does not affect the value of [FocusNode.canRequestFocus] on the - /// descendants. - /// - /// If a descendant node loses focus when this value is changed, the focus - /// will move to the scope enclosing this node. - /// - /// See also: - /// - /// * [ExcludeFocus], a widget that uses this property to conditionally - /// exclude focus for a subtree. - /// * [descendantsAreTraversable], which makes this widget's descendants - /// untraversable. - /// * [ExcludeFocusTraversal], a widget that conditionally excludes focus - /// traversal for a subtree. - /// * [FocusTraversalGroup], a widget used to group together and configure the - /// focus traversal policy for a widget subtree that has a - /// `descendantsAreFocusable` parameter to conditionally block focus for a - /// subtree. - /// {@endtemplate} - bool get descendantsAreFocusable => _descendantsAreFocusable ?? focusNode?.descendantsAreFocusable ?? true; - final bool? _descendantsAreFocusable; - - /// {@template flutter.widgets.Focus.descendantsAreTraversable} - /// If false, will make this widget's descendants untraversable. - /// - /// Defaults to true. Does not affect traversablility of this node (just its - /// descendants): for that, use [FocusNode.skipTraversal]. - /// - /// Does not affect the value of [FocusNode.skipTraversal] on the - /// descendants. Does not affect focusability of the descendants. - /// - /// See also: - /// - /// * [ExcludeFocusTraversal], a widget that uses this property to - /// conditionally exclude focus traversal for a subtree. - /// * [descendantsAreFocusable], which makes this widget's descendants - /// unfocusable. - /// * [ExcludeFocus], a widget that conditionally excludes focus for a subtree. - /// * [FocusTraversalGroup], a widget used to group together and configure the - /// focus traversal policy for a widget subtree that has a - /// `descendantsAreFocusable` parameter to conditionally block focus for a - /// subtree. - /// {@endtemplate} - bool get descendantsAreTraversable => _descendantsAreTraversable ?? focusNode?.descendantsAreTraversable ?? true; - final bool? _descendantsAreTraversable; - - /// {@template flutter.widgets.Focus.includeSemantics} - /// Include semantics information in this widget. - /// - /// If true, this widget will include a [Semantics] node that indicates the - /// [SemanticsProperties.focusable] and [SemanticsProperties.focused] - /// properties. - /// - /// It is not typical to set this to false, as that can affect the semantics - /// information available to accessibility systems. - /// - /// Must not be null, defaults to true. - /// {@endtemplate} - final bool includeSemantics; - - /// A debug label for this widget. - /// - /// Not used for anything except to be printed in the diagnostic output from - /// [toString] or [toStringDeep]. - /// - /// To get a string with the entire tree, call [debugDescribeFocusTree]. To - /// print it to the console call [debugDumpFocusTree]. - /// - /// Defaults to null. - String? get debugLabel => _debugLabel ?? focusNode?.debugLabel; - final String? _debugLabel; - - /// Returns the [focusNode] of the [FocusWithCustomParent] that most tightly encloses the - /// given [BuildContext]. - /// - /// If no [FocusWithCustomParent] node is found before reaching the nearest [FocusScope] - /// widget, or there is no [FocusWithCustomParent] widget in scope, then this method will - /// throw an exception. - /// - /// The `context` and `scopeOk` arguments must not be null. - /// - /// Calling this function creates a dependency that will rebuild the given - /// context when the focus changes. - /// - /// See also: - /// - /// * [maybeOf], which is similar to this function, but will return null - /// instead of throwing if it doesn't find a [Focus] node. - static FocusNode of(BuildContext context, {bool scopeOk = false}) { - final _FocusMarker? marker = context.dependOnInheritedWidgetOfExactType<_FocusMarker>(); - final FocusNode? node = marker?.notifier; - assert(() { - if (node == null) { - throw FlutterError( - 'Focus.of() was called with a context that does not contain a Focus widget.\n' - 'No Focus widget ancestor could be found starting from the context that was passed to ' - 'Focus.of(). This can happen because you are using a widget that looks for a Focus ' - 'ancestor, and do not have a Focus widget descendant in the nearest FocusScope.\n' - 'The context used was:\n' - ' $context', - ); - } - return true; - }()); - assert(() { - if (!scopeOk && node is FocusScopeNode) { - throw FlutterError( - 'Focus.of() was called with a context that does not contain a Focus between the given ' - 'context and the nearest FocusScope widget.\n' - 'No Focus ancestor could be found starting from the context that was passed to ' - 'Focus.of() to the point where it found the nearest FocusScope widget. This can happen ' - 'because you are using a widget that looks for a Focus ancestor, and do not have a ' - 'Focus widget ancestor in the current FocusScope.\n' - 'The context used was:\n' - ' $context', - ); - } - return true; - }()); - return node!; - } - - /// Returns the [focusNode] of the [FocusWithCustomParent] that most tightly encloses the - /// given [BuildContext]. - /// - /// If no [FocusWithCustomParent] node is found before reaching the nearest [FocusScope] - /// widget, or there is no [FocusWithCustomParent] widget in scope, then this method will - /// return null. - /// - /// The `context` and `scopeOk` arguments must not be null. - /// - /// Calling this function creates a dependency that will rebuild the given - /// context when the focus changes. - /// - /// See also: - /// - /// * [of], which is similar to this function, but will throw an exception if - /// it doesn't find a [Focus] node instead of returning null. - static FocusNode? maybeOf(BuildContext context, {bool scopeOk = false}) { - final _FocusMarker? marker = context.dependOnInheritedWidgetOfExactType<_FocusMarker>(); - final FocusNode? node = marker?.notifier; - if (node == null) { - return null; - } - if (!scopeOk && node is FocusScopeNode) { - return null; - } - return node; - } - - /// Returns true if the nearest enclosing [FocusWithCustomParent] widget's node is focused. - /// - /// A convenience method to allow build methods to write: - /// `Focus.isAt(context)` to get whether or not the nearest [FocusWithCustomParent] above them - /// in the widget hierarchy currently has the input focus. - /// - /// Returns false if no [FocusWithCustomParent] widget is found before reaching the nearest - /// [FocusScope], or if the root of the focus tree is reached without finding - /// a [FocusWithCustomParent] widget. - /// - /// Calling this function creates a dependency that will rebuild the given - /// context when the focus changes. - static bool isAt(BuildContext context) => FocusWithCustomParent.maybeOf(context)?.hasFocus ?? false; - - @override - void debugFillProperties(DiagnosticPropertiesBuilder properties) { - super.debugFillProperties(properties); - properties.add(StringProperty('debugLabel', debugLabel, defaultValue: null)); - properties.add(FlagProperty('autofocus', value: autofocus, ifTrue: 'AUTOFOCUS', defaultValue: false)); - properties - .add(FlagProperty('canRequestFocus', value: canRequestFocus, ifFalse: 'NOT FOCUSABLE', defaultValue: false)); - properties.add(FlagProperty('descendantsAreFocusable', - value: descendantsAreFocusable, ifFalse: 'DESCENDANTS UNFOCUSABLE', defaultValue: true)); - properties.add(FlagProperty('descendantsAreTraversable', - value: descendantsAreTraversable, ifFalse: 'DESCENDANTS UNTRAVERSABLE', defaultValue: true)); - properties.add(DiagnosticsProperty('focusNode', focusNode, defaultValue: null)); - } - - @override - State createState() => _FocusWithCustomParentState(); -} - -class _FocusWithCustomParentState extends State { - FocusNode? _internalNode; - FocusNode get focusNode => widget.focusNode ?? _internalNode!; - late bool _hadPrimaryFocus; - late bool _couldRequestFocus; - late bool _descendantsWereFocusable; - late bool _descendantsWereTraversable; - bool _didAutofocus = false; - late FocusAttachment _focusAttachment; - - @override - void initState() { - super.initState(); - _initNode(); - } - - void _initNode() { - if (widget.focusNode == null) { - // Only create a new node if the widget doesn't have one. - // This calls a function instead of just allocating in place because - // _createNode is overridden in _FocusScopeState. - _internalNode ??= _createNode(); - } - focusNode.descendantsAreFocusable = widget.descendantsAreFocusable; - focusNode.descendantsAreTraversable = widget.descendantsAreTraversable; - focusNode.skipTraversal = widget.skipTraversal; - - if (widget._canRequestFocus != null) { - focusNode.canRequestFocus = widget._canRequestFocus!; - } - _couldRequestFocus = focusNode.canRequestFocus; - _descendantsWereFocusable = focusNode.descendantsAreFocusable; - _descendantsWereTraversable = focusNode.descendantsAreTraversable; - _hadPrimaryFocus = focusNode.hasPrimaryFocus; - _focusAttachment = focusNode.attach(context, onKeyEvent: widget.onKeyEvent, onKey: widget.onKey) - ..reparent(parent: widget.parentFocusNode); - - // Add listener even if the _internalNode existed before, since it should - // not be listening now if we're re-using a previous one because it should - // have already removed its listener. - focusNode.addListener(_handleFocusChanged); - } - - FocusNode _createNode() { - return FocusNode( - debugLabel: widget.debugLabel, - canRequestFocus: widget.canRequestFocus, - descendantsAreFocusable: widget.descendantsAreFocusable, - descendantsAreTraversable: widget.descendantsAreTraversable, - skipTraversal: widget.skipTraversal, - ); - } - - @override - void didChangeDependencies() { - super.didChangeDependencies(); - _focusAttachment.reparent(parent: widget.parentFocusNode); - _handleAutofocus(); - } - - @override - void didUpdateWidget(FocusWithCustomParent oldWidget) { - super.didUpdateWidget(oldWidget); - assert(() { - // Only update the debug label in debug builds. - if (oldWidget.focusNode == widget.focusNode && - !widget._usingExternalFocus && - oldWidget.debugLabel != widget.debugLabel) { - focusNode.debugLabel = widget.debugLabel; - } - return true; - }()); - - if (oldWidget.focusNode == widget.focusNode) { - if (!widget._usingExternalFocus) { - if (widget.onKey != focusNode.onKey) { - focusNode.onKey = widget.onKey; - } - if (widget.onKeyEvent != focusNode.onKeyEvent) { - focusNode.onKeyEvent = widget.onKeyEvent; - } - - focusNode.skipTraversal = widget.skipTraversal; - - if (widget._canRequestFocus != null) { - focusNode.canRequestFocus = widget._canRequestFocus!; - } - focusNode.descendantsAreFocusable = widget.descendantsAreFocusable; - focusNode.descendantsAreTraversable = widget.descendantsAreTraversable; - } - } else { - _focusAttachment.detach(); - oldWidget.focusNode?.removeListener(_handleFocusChanged); - _initNode(); - } - - if (widget.parentFocusNode != oldWidget.parentFocusNode) { - _focusAttachment.reparent(parent: widget.parentFocusNode); - } - - if (oldWidget.autofocus != widget.autofocus) { - _handleAutofocus(); - } - } - - @override - void deactivate() { - super.deactivate(); - // The focus node's location in the tree is no longer valid here. But - // we can't unfocus or remove the node from the tree because if the widget - // is moved to a different part of the tree (via global key) it should - // retain its focus state. That's why we temporarily park it on the root - // focus node (via reparent) until it either gets moved to a different part - // of the tree (via didChangeDependencies) or until it is disposed. - _focusAttachment.reparent(); - _didAutofocus = false; - } - - @override - void dispose() { - // Regardless of the node owner, we need to remove it from the tree and stop - // listening to it. - focusNode.removeListener(_handleFocusChanged); - _focusAttachment.detach(); - - // Don't manage the lifetime of external nodes given to the widget, just the - // internal node. - _internalNode?.dispose(); - super.dispose(); - } - - void _handleAutofocus() { - if (!_didAutofocus && widget.autofocus) { - FocusScope.of(context).autofocus(focusNode); - _didAutofocus = true; - } - } - - void _handleFocusChanged() { - final bool hasPrimaryFocus = focusNode.hasPrimaryFocus; - final bool canRequestFocus = focusNode.canRequestFocus; - final bool descendantsAreFocusable = focusNode.descendantsAreFocusable; - final bool descendantsAreTraversable = focusNode.descendantsAreTraversable; - widget.onFocusChange?.call(focusNode.hasFocus); - // Check the cached states that matter here, and call setState if they have - // changed. - if (_hadPrimaryFocus != hasPrimaryFocus) { - setState(() { - _hadPrimaryFocus = hasPrimaryFocus; - }); - } - if (_couldRequestFocus != canRequestFocus) { - setState(() { - _couldRequestFocus = canRequestFocus; - }); - } - if (_descendantsWereFocusable != descendantsAreFocusable) { - setState(() { - _descendantsWereFocusable = descendantsAreFocusable; - }); - } - if (_descendantsWereTraversable != descendantsAreTraversable) { - setState(() { - _descendantsWereTraversable = descendantsAreTraversable; - }); - } - } - - @override - Widget build(BuildContext context) { - if (widget.parentFocusNode == null) { - _focusAttachment.reparent(); - } - - Widget child = widget.child; - if (widget.includeSemantics) { - child = Semantics( - focusable: _couldRequestFocus, - focused: _hadPrimaryFocus, - child: widget.child, - ); - } - return _FocusMarker( - node: focusNode, - child: child, - ); - } -} - -// The InheritedWidget marker for Focus and FocusScope. -class _FocusMarker extends InheritedNotifier { - const _FocusMarker({ - Key? key, - required FocusNode node, - required Widget child, - }) : super(key: key, notifier: node, child: child); -} diff --git a/super_editor/lib/src/infrastructure/platforms/android/android_document_controls.dart b/super_editor/lib/src/infrastructure/platforms/android/android_document_controls.dart index b0bc0ae78..2c451d41e 100644 --- a/super_editor/lib/src/infrastructure/platforms/android/android_document_controls.dart +++ b/super_editor/lib/src/infrastructure/platforms/android/android_document_controls.dart @@ -313,8 +313,26 @@ class AndroidControlsDocumentLayerState } if (selection.isCollapsed && !_controlsController!.shouldShowExpandedHandles.value) { + Rect caretRect = documentLayout.getEdgeForPosition(selection.extent)!; + + // Default caret width used by the Android caret. + const caretWidth = 2; + + final layerBox = context.findRenderObject() as RenderBox?; + if (layerBox != null && layerBox.hasSize && caretRect.left + caretWidth >= layerBox.size.width) { + // Ajust the caret position to make it entirely visible because it's currently placed + // partially or entirely outside of the layers' bounds. This can happen for downstream selections + // of block components that take all the available width. + caretRect = Rect.fromLTWH( + layerBox.size.width - caretWidth, + caretRect.top, + caretRect.width, + caretRect.height, + ); + } + return DocumentSelectionLayout( - caret: documentLayout.getRectForPosition(selection.extent)!, + caret: caretRect, ); } else { return DocumentSelectionLayout( diff --git a/super_editor/lib/src/infrastructure/platforms/ios/ios_document_controls.dart b/super_editor/lib/src/infrastructure/platforms/ios/ios_document_controls.dart index d574ba233..99e3a5957 100644 --- a/super_editor/lib/src/infrastructure/platforms/ios/ios_document_controls.dart +++ b/super_editor/lib/src/infrastructure/platforms/ios/ios_document_controls.dart @@ -673,8 +673,26 @@ class IosControlsDocumentLayerState extends DocumentLayoutLayerState= layerBox.size.width) { + // Ajust the caret position to make it entirely visible because it's currently placed + // partially or entirely outside of the layers' bounds. This can happen for downstream selections + // of block components that take all the available width. + caretRect = Rect.fromLTWH( + layerBox.size.width - caretWidth, + caretRect.top, + caretRect.width, + caretRect.height, + ); + } + return DocumentSelectionLayout( - caret: documentLayout.getRectForPosition(selection.extent)!, + caret: caretRect, ); } else { return DocumentSelectionLayout( diff --git a/super_editor/lib/src/infrastructure/platforms/mac/mac_ime.dart b/super_editor/lib/src/infrastructure/platforms/mac/mac_ime.dart index 49923e0a8..5b7e81433 100644 --- a/super_editor/lib/src/infrastructure/platforms/mac/mac_ime.dart +++ b/super_editor/lib/src/infrastructure/platforms/mac/mac_ime.dart @@ -1,5 +1,3 @@ -import 'package:flutter/widgets.dart'; - /// MacOS selector names that are sent to [TextInputClient.performSelector]. /// /// These selectors express the user intent and are generated by shortcuts. For example, @@ -63,91 +61,3 @@ class MacOsSelectors { static const String insertBacktab = 'insertBacktab:'; static const String insertNewLine = 'insertNewline:'; } - -/// Stop propagating intents that prevent [TextInputClient.performSelector] from being called. -/// -/// These intents have default handlers installed by Flutter -/// that prevents the key events from bubbling up to the IME. -/// -/// Use this object in an [Actions] widget. -final Map> disabledMacIntents = { - // Generated by pressing LEFT/RIGHT ARROW. - ExtendSelectionByCharacterIntent: DoNothingAction(consumesKey: false), - - // Generated by pressing UP/DOWN ARROW. - ExtendSelectionVerticallyToAdjacentLineIntent: DoNothingAction(consumesKey: false), - - // Generated by pressing PAGE UP/DOWN. - ScrollIntent: DoNothingAction(consumesKey: false), - - // Generated by pressing HOME/END. - ScrollToDocumentBoundaryIntent: DoNothingAction(consumesKey: false), - - // Generated by pressing TAB. - NextFocusIntent: DoNothingAction(consumesKey: false), - - // Generated by pressing SHIFT + TAB. - PreviousFocusIntent: DoNothingAction(consumesKey: false), - - // Generated by pressing SPACE. - // PrioritizedIntents might contain intents that we don't want to prevent - // from bubbling up. - // So, we need to look into its inner intents. - PrioritizedIntents: _PreventPrioritizedIntentsFromBubblingUp( - intentFilter: (e) => e is ActivateIntent, - ), -}; - -/// Prevents a [PrioritizedIntents] from bubbling up if [intentFilter] returns -/// `true` for at least one of its `orderedIntents`. -/// -/// Based on [PrioritizedAction]. -class _PreventPrioritizedIntentsFromBubblingUp extends Action { - _PreventPrioritizedIntentsFromBubblingUp({ - required this.intentFilter, - }); - - /// Whether the [intent] should be prevented from bubbling up. - final bool Function(Intent intent) intentFilter; - - @override - bool consumesKey(Intent intent) => false; - - @override - void invoke(Intent intent) {} - - @override - bool isEnabled(PrioritizedIntents intent, [BuildContext? context]) { - final FocusNode? focus = primaryFocus; - if (focus == null || focus.context == null) { - return false; - } - - for (final Intent candidateIntent in intent.orderedIntents) { - final Action? candidateAction = Actions.maybeFind( - focus.context!, - intent: candidateIntent, - ); - if (candidateAction != null && _isActionEnabled(candidateAction, candidateIntent, context)) { - // The corresponding Action for the Intent is enabled. - // This is the Action that Flutter will execute. - if (intentFilter(candidateIntent)) { - return true; - } - - // We don't care about the Intent that is going to have its corresponding Action executed. - // Don't block it. - return false; - } - } - - return false; - } - - bool _isActionEnabled(Action action, Intent intent, BuildContext? context) { - if (action is ContextAction) { - return action.isEnabled(intent, context); - } - return action.isEnabled(intent); - } -} diff --git a/super_editor/lib/src/infrastructure/popovers.dart b/super_editor/lib/src/infrastructure/popovers.dart index 0d5976341..b5aecf380 100644 --- a/super_editor/lib/src/infrastructure/popovers.dart +++ b/super_editor/lib/src/infrastructure/popovers.dart @@ -1,6 +1,5 @@ import 'package:flutter/material.dart'; -import 'package:super_editor/src/infrastructure/focus.dart'; -import 'package:super_editor/src/infrastructure/platforms/mac/mac_ime.dart'; +import 'package:super_editor/src/infrastructure/actions.dart'; /// A popover that shares focus with a `SuperEditor`. /// @@ -51,11 +50,11 @@ class SuperEditorPopover extends StatelessWidget { @override Widget build(BuildContext context) { - return Actions( - actions: disabledMacIntents, - child: FocusWithCustomParent( + return IntentBlocker( + intents: appleBlockedIntents, + child: Focus( focusNode: popoverFocusNode, - parentFocusNode: editorFocusNode, + parentNode: editorFocusNode, onKeyEvent: onKeyEvent, child: child, ), diff --git a/super_editor/lib/src/super_reader/read_only_document_mouse_interactor.dart b/super_editor/lib/src/super_reader/read_only_document_mouse_interactor.dart index 61e7477f9..16589a921 100644 --- a/super_editor/lib/src/super_reader/read_only_document_mouse_interactor.dart +++ b/super_editor/lib/src/super_reader/read_only_document_mouse_interactor.dart @@ -185,7 +185,6 @@ class _ReadOnlyDocumentMouseInteractorState extends State bool get _isMultiline => (widget.minLines ?? 1) != 1 || widget.maxLines != 1; void _updateSelectionAndImeConnectionOnFocusChange() { - if (_focusNode.hasFocus) { - if (!_textEditingController.isAttachedToIme) { - _log.info('Attaching TextInputClient to TextInput'); + // The focus change callback might be invoked in the build phase, usually when used inside + // an OverlayPortal. If that's the case, defer the setState call until the end of the frame. + WidgetsBinding.instance.runAsSoonAsPossible(() { + if (!mounted) { + return; + } + + if (_focusNode.hasFocus) { + if (!_textEditingController.isAttachedToIme) { + _log.info('Attaching TextInputClient to TextInput'); + setState(() { + if (!_textEditingController.selection.isValid) { + _textEditingController.selection = TextSelection.collapsed(offset: _textEditingController.text.length); + } + + if (widget.imeConfiguration != null) { + _textEditingController.attachToImeWithConfig(widget.imeConfiguration!); + } else { + _textEditingController.attachToIme( + textInputAction: widget.textInputAction ?? TextInputAction.done, + textInputType: _isMultiline ? TextInputType.multiline : TextInputType.text, + ); + } + + _autoScrollToKeepTextFieldVisible(); + _showEditingControlsOverlay(); + }); + } + } else { + _log.info('Lost focus. Detaching TextInputClient from TextInput.'); setState(() { - if (!_textEditingController.selection.isValid) { - _textEditingController.selection = TextSelection.collapsed(offset: _textEditingController.text.length); - } - - if (widget.imeConfiguration != null) { - _textEditingController.attachToImeWithConfig(widget.imeConfiguration!); - } else { - _textEditingController.attachToIme( - textInputAction: widget.textInputAction ?? TextInputAction.done, - textInputType: _isMultiline ? TextInputType.multiline : TextInputType.text, - ); - } - - _autoScrollToKeepTextFieldVisible(); - _showEditingControlsOverlay(); + _textEditingController.detachFromIme(); + _textEditingController.selection = const TextSelection.collapsed(offset: -1); + _textEditingController.composingRegion = TextRange.empty; + _removeEditingOverlayControls(); }); } - } else { - _log.info('Lost focus. Detaching TextInputClient from TextInput.'); - setState(() { - _textEditingController.detachFromIme(); - _textEditingController.selection = const TextSelection.collapsed(offset: -1); - _textEditingController.composingRegion = TextRange.empty; - _removeEditingOverlayControls(); - }); - } + }); } void _onTextOrSelectionChange() { @@ -540,7 +547,7 @@ class SuperAndroidTextFieldState extends State Widget _buildTextField() { return TapRegion( groupId: widget.tapRegionGroupId, - child: NonReparentingFocus( + child: Focus( key: _textFieldKey, focusNode: _focusNode, onKeyEvent: _onKeyEventPressed, diff --git a/super_editor/lib/src/super_textfield/desktop/desktop_textfield.dart b/super_editor/lib/src/super_textfield/desktop/desktop_textfield.dart index fc2a7f73b..8f5456099 100644 --- a/super_editor/lib/src/super_textfield/desktop/desktop_textfield.dart +++ b/super_editor/lib/src/super_textfield/desktop/desktop_textfield.dart @@ -4,17 +4,16 @@ import 'dart:ui' as ui; import 'package:flutter/foundation.dart'; import 'package:flutter/gestures.dart'; import 'package:flutter/material.dart' hide SelectableText; -import 'package:flutter/rendering.dart'; import 'package:flutter/scheduler.dart'; import 'package:flutter/services.dart'; import 'package:super_editor/src/core/document_layout.dart'; import 'package:super_editor/src/infrastructure/_logging.dart'; +import 'package:super_editor/src/infrastructure/actions.dart'; import 'package:super_editor/src/infrastructure/attributed_text_styles.dart'; import 'package:super_editor/src/infrastructure/flutter/build_context.dart'; import 'package:super_editor/src/infrastructure/flutter/flutter_scheduler.dart'; import 'package:super_editor/src/infrastructure/flutter/material_scrollbar.dart'; import 'package:super_editor/src/infrastructure/flutter/text_input_configuration.dart'; -import 'package:super_editor/src/infrastructure/focus.dart'; import 'package:super_editor/src/infrastructure/ime_input_owner.dart'; import 'package:super_editor/src/infrastructure/keyboard.dart'; import 'package:super_editor/src/infrastructure/multi_listenable_builder.dart'; @@ -460,8 +459,8 @@ class SuperDesktopTextFieldState extends State implements required bool isMultiline, required Widget child, }) { - return Actions( - actions: defaultTargetPlatform == TargetPlatform.macOS ? disabledMacIntents : {}, + return IntentBlocker( + intents: CurrentPlatform.isApple ? appleBlockedIntents : nonAppleBlockedIntents, child: SuperTextFieldKeyboardInteractor( focusNode: _focusNode, textFieldContext: _textFieldContext, @@ -1125,7 +1124,7 @@ class _SuperTextFieldKeyboardInteractorState extends State DeltaTextInputClient get imeClient => _textEditingController; void _updateSelectionAndImeConnectionOnFocusChange() { - if (_focusNode.hasFocus) { - if (!_textEditingController.isAttachedToIme) { - _log.info('Attaching TextInputClient to TextInput'); + // The focus change callback might be invoked in the build phase, usually when used inside + // an OverlayPortal. If that's the case, defer the setState call until the end of the frame. + WidgetsBinding.instance.runAsSoonAsPossible(() { + if (!mounted) { + return; + } + + if (_focusNode.hasFocus) { + if (!_textEditingController.isAttachedToIme) { + _log.info('Attaching TextInputClient to TextInput'); + setState(() { + if (!_textEditingController.selection.isValid) { + _textEditingController.selection = TextSelection.collapsed(offset: _textEditingController.text.length); + } + + if (widget.imeConfiguration != null) { + _textEditingController.attachToImeWithConfig(widget.imeConfiguration!); + } else { + _textEditingController.attachToIme( + textInputAction: widget.textInputAction ?? TextInputAction.done, + textInputType: _isMultiline ? TextInputType.multiline : TextInputType.text, + ); + } + + _autoScrollToKeepTextFieldVisible(); + _showHandles(); + }); + } + } else { + _log.info('Lost focus. Detaching TextInputClient from TextInput.'); setState(() { - if (!_textEditingController.selection.isValid) { - _textEditingController.selection = TextSelection.collapsed(offset: _textEditingController.text.length); - } - - if (widget.imeConfiguration != null) { - _textEditingController.attachToImeWithConfig(widget.imeConfiguration!); - } else { - _textEditingController.attachToIme( - textInputAction: widget.textInputAction ?? TextInputAction.done, - textInputType: _isMultiline ? TextInputType.multiline : TextInputType.text, - ); - } - - _autoScrollToKeepTextFieldVisible(); - _showHandles(); + _textEditingController.detachFromIme(); + _textEditingController.selection = const TextSelection.collapsed(offset: -1); + _textEditingController.composingRegion = TextRange.empty; + _removeEditingOverlayControls(); }); } - } else { - _log.info('Lost focus. Detaching TextInputClient from TextInput.'); - setState(() { - _textEditingController.detachFromIme(); - _textEditingController.selection = const TextSelection.collapsed(offset: -1); - _textEditingController.composingRegion = TextRange.empty; - _removeEditingOverlayControls(); - }); - } + }); } void _onTextOrSelectionChange() { @@ -538,7 +545,7 @@ class SuperIOSTextFieldState extends State Widget _buildTextField() { return TapRegion( groupId: widget.tapRegionGroupId, - child: NonReparentingFocus( + child: Focus( key: _textFieldKey, focusNode: _focusNode, child: CompositedTransformTarget( diff --git a/super_editor/lib/src/test/super_editor_test/supereditor_inspector.dart b/super_editor/lib/src/test/super_editor_test/supereditor_inspector.dart index e4cb148f5..623aa75e7 100644 --- a/super_editor/lib/src/test/super_editor_test/supereditor_inspector.dart +++ b/super_editor/lib/src/test/super_editor_test/supereditor_inspector.dart @@ -217,6 +217,24 @@ class SuperEditorInspector { return findRichTextInParagraph(nodeId, superEditorFinder).style; } + /// Finds the ordered list item with the given [nodeId] and returns its ordinal value. + /// + /// List items ordinals start from 1. + /// + /// {@macro supereditor_finder} + static int findListItemOrdinal(String nodeId, [Finder? superEditorFinder]) { + final listItem = find + .ancestor( + of: find.byWidget(SuperEditorInspector.findWidgetForComponent(nodeId)), + matching: find.byType(OrderedListItemComponent), + ) + .evaluate() + .single + .widget as OrderedListItemComponent; + + return listItem.listIndex; + } + /// Calculates the delta between the center of the character at [textOffset1] and and the /// center of the character at [textOffset2] within the node with the given [nodeId]. /// diff --git a/super_editor/lib/super_editor.dart b/super_editor/lib/super_editor.dart index 6ee77646f..ca741524e 100644 --- a/super_editor/lib/super_editor.dart +++ b/super_editor/lib/super_editor.dart @@ -64,7 +64,6 @@ export 'src/infrastructure/content_layers.dart'; export 'src/infrastructure/documents/document_layers.dart'; export 'src/infrastructure/documents/document_scroller.dart'; export 'src/infrastructure/documents/selection_leader_document_layer.dart'; -export 'src/infrastructure/focus.dart'; export 'src/infrastructure/ime_input_owner.dart'; export 'src/infrastructure/keyboard.dart'; export 'src/infrastructure/multi_tap_gesture.dart'; @@ -88,6 +87,7 @@ export 'src/infrastructure/text_input.dart'; export 'src/infrastructure/viewport_size_reporting.dart'; export 'src/infrastructure/popovers.dart'; export 'src/infrastructure/selectable_list.dart'; +export 'src/infrastructure/actions.dart'; // Super Reader export 'src/super_reader/read_only_document_android_touch_interactor.dart'; diff --git a/super_editor/pubspec.yaml b/super_editor/pubspec.yaml index 22fade5ad..ab841e570 100644 --- a/super_editor/pubspec.yaml +++ b/super_editor/pubspec.yaml @@ -24,7 +24,7 @@ dependencies: attributed_text: ^0.3.0 characters: ^1.2.0 collection: ^1.15.0 - follow_the_leader: ^0.0.4+7 + follow_the_leader: ^0.0.4+8 http: ">=0.13.1 <2.0.0" linkify: ^5.0.0 logging: ^1.0.1 diff --git a/super_editor/test/super_editor/components/list_items_test.dart b/super_editor/test/super_editor/components/list_items_test.dart index 697450968..2bd5760f0 100644 --- a/super_editor/test/super_editor/components/list_items_test.dart +++ b/super_editor/test/super_editor/components/list_items_test.dart @@ -512,6 +512,77 @@ void main() { expect(secondOrderedItem.listIndex, 2); }); + testWidgetsOnArbitraryDesktop('keeps sequence for items split by ordered list items with higher indentation', + (tester) async { + final context = await tester // + .createDocument() + .fromMarkdown(""" + 1. list item 1 + 2. list item 2 + 1. list item 2.1 + 2. list item 2.2 + 3. list item 3 + 1. list item 3.1 +""") // + .pump(); + + expect(context.document.nodes.length, 6); + + // Ensure the nodes have the correct type. + for (int i = 0; i < 6; i++) { + expect(context.document.nodes[i], isA()); + expect((context.document.nodes[i] as ListItemNode).type, ListItemType.ordered); + } + + // Ensure the sequence was kept. + expect(SuperEditorInspector.findListItemOrdinal(context.document.nodes[0].id), 1); + expect(SuperEditorInspector.findListItemOrdinal(context.document.nodes[1].id), 2); + expect(SuperEditorInspector.findListItemOrdinal(context.document.nodes[2].id), 1); + expect(SuperEditorInspector.findListItemOrdinal(context.document.nodes[3].id), 2); + expect(SuperEditorInspector.findListItemOrdinal(context.document.nodes[4].id), 3); + expect(SuperEditorInspector.findListItemOrdinal(context.document.nodes[5].id), 1); + }); + + testWidgetsOnArbitraryDesktop('restarts item order when separated by an unordered item', (tester) async { + final context = await tester // + .createDocument() + .fromMarkdown(""" +1. First ordered item +2. Second ordered item +- First unordered item +- Second unordered item +1. First ordered item +2. Second ordered item""") // + .pump(); + + expect(context.document.nodes.length, 6); + + // Ensure the nodes have the correct type. + expect(context.document.nodes[0], isA()); + expect((context.document.nodes[0] as ListItemNode).type, ListItemType.ordered); + + expect(context.document.nodes[1], isA()); + expect((context.document.nodes[1] as ListItemNode).type, ListItemType.ordered); + + expect(context.document.nodes[2], isA()); + expect((context.document.nodes[2] as ListItemNode).type, ListItemType.unordered); + + expect(context.document.nodes[3], isA()); + expect((context.document.nodes[3] as ListItemNode).type, ListItemType.unordered); + + expect(context.document.nodes[4], isA()); + expect((context.document.nodes[4] as ListItemNode).type, ListItemType.ordered); + + expect(context.document.nodes[5], isA()); + expect((context.document.nodes[5] as ListItemNode).type, ListItemType.ordered); + + // Ensure the sequence restarted after the unordered items. + expect(SuperEditorInspector.findListItemOrdinal(context.document.nodes[0].id), 1); + expect(SuperEditorInspector.findListItemOrdinal(context.document.nodes[1].id), 2); + expect(SuperEditorInspector.findListItemOrdinal(context.document.nodes[4].id), 1); + expect(SuperEditorInspector.findListItemOrdinal(context.document.nodes[5].id), 2); + }); + testWidgetsOnArbitraryDesktop('does not keep sequence for items split by paragraphs', (tester) async { final context = await tester // .createDocument() diff --git a/super_editor/test/super_editor/infrastructure/common_editor_operations_test.dart b/super_editor/test/super_editor/infrastructure/common_editor_operations_test.dart index a20ec3db1..461056dc4 100644 --- a/super_editor/test/super_editor/infrastructure/common_editor_operations_test.dart +++ b/super_editor/test/super_editor/infrastructure/common_editor_operations_test.dart @@ -198,12 +198,12 @@ MutableDocument _singleParagraphWithLinkDoc() { AttributedSpans( attributions: [ SpanMarker( - attribution: LinkAttribution(url: Uri.parse('https://google.com')), + attribution: LinkAttribution.fromUri(Uri.parse('https://google.com')), offset: 0, markerType: SpanMarkerType.start, ), SpanMarker( - attribution: LinkAttribution(url: Uri.parse('https://google.com')), + attribution: LinkAttribution.fromUri(Uri.parse('https://google.com')), offset: 17, markerType: SpanMarkerType.end, ), diff --git a/super_editor/test/super_editor/supereditor_attributions_test.dart b/super_editor/test/super_editor/supereditor_attributions_test.dart index 6b139edfe..dc5c60a5c 100644 --- a/super_editor/test/super_editor/supereditor_attributions_test.dart +++ b/super_editor/test/super_editor/supereditor_attributions_test.dart @@ -584,6 +584,104 @@ void main() { ); }); + testWidgetsOnAllPlatforms( + "toggles an attribution when selection spans multiple nodes and starts at the end of the first selected node", + (tester) async { + // Test situations where the selection starts after the last character of the first selected node. See #1948 + // for more information. + + final context = await tester // + .createDocument() + .fromMarkdown("First node\n\nSecond node") + .pump(); + + final editor = context.editor; + final document = context.document; + + final firstNode = document.nodes[0] as ParagraphNode; + final secondNode = document.nodes[1] as ParagraphNode; + + // Apply the bold attribution, starting after the last character of the first node. + editor.toggleAttributionsForDocumentSelection( + DocumentSelection( + base: firstNode.endDocumentPosition, + extent: secondNode.endDocumentPosition, + ), + {boldAttribution}, + ); + + // Ensure bold attribution is applied only to the second node. Since the selection starts at the + // end of the first node, there's no text there to apply the attribution to. + expect( + document, + equalsMarkdown( + "First node\n\n**Second node**", + ), + ); + + // Remove the bold attribution, starting after the last character of the first node. + editor.toggleAttributionsForDocumentSelection( + DocumentSelection( + base: firstNode.endDocumentPosition, + extent: secondNode.endDocumentPosition, + ), + {boldAttribution}, + ); + + // Ensure bold attribution was removed. + expect(secondNode.text.spans.markers.isEmpty, true); + }); + + testWidgetsOnAllPlatforms( + "toggles an attribution when selection spans multiple nodes and ends at the beginning of the last selected node", + (tester) async { + // Test situations where the selection ends before the first character of the last selected node. See #1948 + // for more information. + + final context = await tester // + .createDocument() + .fromMarkdown("First node\n\nSecond node") + .pump(); + + final editor = context.editor; + final document = context.document; + + final firstNode = document.nodes[0] as ParagraphNode; + final secondNode = document.nodes[1] as ParagraphNode; + + // Apply the bold attribution, with a selection that start at the beginning of the first node and ends + // before the first character of the second node. + editor.toggleAttributionsForDocumentSelection( + DocumentSelection( + base: firstNode.beginningDocumentPosition, + extent: secondNode.beginningDocumentPosition, + ), + {boldAttribution}, + ); + + // Ensure bold attribution is applied only to the first node. Since the selection ends before the first + // character of the second node, there's no text there to apply the attribution to. + expect( + document, + equalsMarkdown( + "**First node**\n\nSecond node", + ), + ); + + // Remove the bold attribution, with a selection that start at the beginning of the first node and ends + // before the first character of the second node. + editor.toggleAttributionsForDocumentSelection( + DocumentSelection( + base: firstNode.beginningDocumentPosition, + extent: secondNode.beginningDocumentPosition, + ), + {boldAttribution}, + ); + + // Ensure bold attribution was removed. + expect(secondNode.text.spans.markers.isEmpty, true); + }); + testWidgetsOnAllPlatforms( "toggles an attribution across nodes with the attribution applied throughout and partially within first and second node respectively", (tester) async { diff --git a/super_editor/test/super_editor/supereditor_input_ime_test.dart b/super_editor/test/super_editor/supereditor_input_ime_test.dart index 05028afe9..53b19e0c3 100644 --- a/super_editor/test/super_editor/supereditor_input_ime_test.dart +++ b/super_editor/test/super_editor/supereditor_input_ime_test.dart @@ -959,6 +959,27 @@ Paragraph two }, variant: inputSourceVariant); }); + testWidgetsOnWebDesktop('inside a CustomScrollView > inserts space instead of scrolling with SPACEBAR', + (tester) async { + final testContext = await tester // + .createDocument() + .withSingleEmptyParagraph() + .insideCustomScrollView() + .withInputSource(TextInputSource.ime) + .pump(); + + final nodeId = testContext.document.nodes.first.id; + + // Place the caret at the beginning of the paragraph. + await tester.placeCaretInParagraph(nodeId, 0); + + // Press space to insert a space character. + await _typeSpaceAdaptive(tester); + + // Ensure the space character was inserted. + expect(SuperEditorInspector.findTextInComponent(nodeId).text, ' '); + }); + testWidgetsOnWebDesktop('deletes a character with backspace', (tester) async { final testContext = await tester // .createDocument() @@ -1467,12 +1488,12 @@ MutableDocument _singleParagraphWithLinkDoc() { AttributedSpans( attributions: [ SpanMarker( - attribution: LinkAttribution(url: Uri.parse('https://google.com')), + attribution: LinkAttribution.fromUri(Uri.parse('https://google.com')), offset: 0, markerType: SpanMarkerType.start, ), SpanMarker( - attribution: LinkAttribution(url: Uri.parse('https://google.com')), + attribution: LinkAttribution.fromUri(Uri.parse('https://google.com')), offset: 17, markerType: SpanMarkerType.end, ), @@ -1494,3 +1515,20 @@ class _TestImeOverrides extends DeltaTextInputClientDecorator { performActionCallback(action); } } + +/// Simulates pressing the SPACE key. +/// +/// First, this method simulates pressing the SPACE key on a physical keyboard. If that key event goes unhandled +/// then this method generates an insertion delta of " ". +/// +// TODO: extract this to the flutter_test_robots package. +Future _typeSpaceAdaptive(WidgetTester tester) async { + final handled = await tester.sendKeyEvent(LogicalKeyboardKey.space); + + if (handled) { + await tester.pumpAndSettle(); + return; + } + + await tester.typeImeText(' '); +} diff --git a/super_editor/test/super_editor/supereditor_popover_focus_test.dart b/super_editor/test/super_editor/supereditor_popover_focus_test.dart index 82b13afdc..c15595bc8 100644 --- a/super_editor/test/super_editor/supereditor_popover_focus_test.dart +++ b/super_editor/test/super_editor/supereditor_popover_focus_test.dart @@ -127,7 +127,7 @@ Future _hidePopover(WidgetTester tester) async { await tester.pump(); } -class _Popover extends StatelessWidget { +class _Popover extends StatefulWidget { const _Popover({ Key? key, required this.editorFocusNode, @@ -137,16 +137,29 @@ class _Popover extends StatelessWidget { final FocusNode editorFocusNode; final FocusNode textFieldFocusNode; + @override + State<_Popover> createState() => _PopoverState(); +} + +class _PopoverState extends State<_Popover> { + final _popoverFocusNode = FocusNode(); + + @override + void dispose() { + _popoverFocusNode.dispose(); + super.dispose(); + } + @override Widget build(BuildContext context) { return Center( child: ConstrainedBox( constraints: const BoxConstraints(minWidth: 300, minHeight: 56), - child: FocusWithCustomParent( - parentFocusNode: editorFocusNode, - focusNode: textFieldFocusNode, + child: Focus( + focusNode: _popoverFocusNode, + parentNode: widget.editorFocusNode, child: SuperTextField( - focusNode: textFieldFocusNode, + focusNode: widget.textFieldFocusNode, lineHeight: 20, ), ), diff --git a/super_editor/test/super_editor/supereditor_scrolling_test.dart b/super_editor/test/super_editor/supereditor_scrolling_test.dart index 23997d346..cc1081c6a 100644 --- a/super_editor/test/super_editor/supereditor_scrolling_test.dart +++ b/super_editor/test/super_editor/supereditor_scrolling_test.dart @@ -557,6 +557,37 @@ void main() { expect(SuperEditorInspector.findDocumentSelection(), isNull); }); + testWidgetsOnArbitraryDesktop("does not stop momentum on mouse move", (tester) async { + final scrollController = ScrollController(); + + // Pump an editor with a small size to make it scrollable. + await tester // + .createDocument() // + .withLongDoc() // + .withScrollController(scrollController) // + .withEditorSize(const Size(300, 300)) + .pump(); + + // Fling scroll with the trackpad to generate momentum. + await tester.trackpadFling( + find.byType(SuperEditor), + const Offset(0.0, -300), + 300.0, + ); + + final scrollOffsetInMiddleOfMomentum = scrollController.offset; + + // Move the mouse around. + final gesture = await tester.createGesture(); + await gesture.moveTo(tester.getTopLeft(find.byType(SuperEditor))); + + // Let any momentum run. + await tester.pumpAndSettle(); + + // Ensure that the momentum didn't stop due to mouse movement. + expect(scrollOffsetInMiddleOfMomentum, lessThan(scrollController.offset)); + }); + testWidgetsOnAndroid("doesn't overscroll when dragging down", (tester) async { final scrollController = ScrollController(); diff --git a/super_editor/test/super_editor/supereditor_selection_test.dart b/super_editor/test/super_editor/supereditor_selection_test.dart index dcab2939f..bc5e0bb35 100644 --- a/super_editor/test/super_editor/supereditor_selection_test.dart +++ b/super_editor/test/super_editor/supereditor_selection_test.dart @@ -619,6 +619,7 @@ Second Paragraph // result. final textFieldFocus = FocusNode(); + final subtreeFocus = FocusNode(); final editorFocus = FocusNode(); await tester .createDocument() @@ -630,9 +631,9 @@ Second Paragraph home: Scaffold( body: Column( children: [ - FocusWithCustomParent( - focusNode: textFieldFocus, - parentFocusNode: editorFocus, + Focus( + focusNode: subtreeFocus, + parentNode: editorFocus, child: SuperTextField( focusNode: textFieldFocus, // We put the SuperTextField in keyboard mode so that the SuperTextField @@ -686,6 +687,7 @@ Second Paragraph // result. final textFieldFocus = FocusNode(); + final subtreeFocus = FocusNode(); final editorFocus = FocusNode(); await tester .createDocument() @@ -700,9 +702,9 @@ Second Paragraph home: Scaffold( body: Column( children: [ - FocusWithCustomParent( - focusNode: textFieldFocus, - parentFocusNode: editorFocus, + Focus( + focusNode: subtreeFocus, + parentNode: editorFocus, child: SuperTextField( focusNode: textFieldFocus, // We put the SuperTextField in keyboard mode so that the SuperTextField @@ -752,6 +754,7 @@ Second Paragraph testWidgetsOnAllPlatforms("retains selection when user types in sub-focus text field", (tester) async { final textFieldFocus = FocusNode(); + final subTreeFocusNode = FocusNode(); final textFieldController = ImeAttributedTextEditingController(); final editorFocus = FocusNode(); const initialEditorSelection = DocumentSelection( @@ -772,9 +775,9 @@ Second Paragraph home: Scaffold( body: Column( children: [ - FocusWithCustomParent( - focusNode: textFieldFocus, - parentFocusNode: editorFocus, + Focus( + focusNode: subTreeFocusNode, + parentNode: editorFocus, child: SuperTextField( focusNode: textFieldFocus, textController: textFieldController, diff --git a/super_editor/test/super_editor/supereditor_test_tools.dart b/super_editor/test/super_editor/supereditor_test_tools.dart index 9e9545512..e626bb311 100644 --- a/super_editor/test/super_editor/supereditor_test_tools.dart +++ b/super_editor/test/super_editor/supereditor_test_tools.dart @@ -9,6 +9,7 @@ import 'package:super_editor/super_editor_test.dart'; import 'package:super_editor_markdown/super_editor_markdown.dart'; import 'package:text_table/text_table.dart'; +import '../test_tools_user_input.dart'; import 'test_documents.dart'; /// Extensions on [WidgetTester] that configure and pump [SuperEditor] @@ -409,6 +410,13 @@ class TestSuperEditorConfigurator { } return MaterialApp( theme: _config.appTheme, + // By default, Flutter chooses the shortcuts based on the platform. For "native" platforms, + // the defaults already work correctly, because we set `debugDefaultTargetPlatformOverride` to force + // the desired platform. However, for web Flutter checks for `kIsWeb`, which we can't control. + // + // Use our own version of the shortcuts, so we can set `debugIsWebOverride` to `true` to force + // Flutter to pick the web shortcuts. + shortcuts: defaultFlutterShortcuts, home: Scaffold( body: superEditor, ), diff --git a/super_editor/test/super_editor/test_documents.dart b/super_editor/test/super_editor/test_documents.dart index 4c7cd6bb9..75878332e 100644 --- a/super_editor/test/super_editor/test_documents.dart +++ b/super_editor/test/super_editor/test_documents.dart @@ -57,14 +57,16 @@ MutableDocument singleParagraphWithLinkDoc() => MutableDocument( "This paragraph includes a link that the user can tap", AttributedSpans( attributions: [ - SpanMarker( - attribution: LinkAttribution(url: Uri.parse("https://fake.url")), - offset: 26, - markerType: SpanMarkerType.start), - SpanMarker( - attribution: LinkAttribution(url: Uri.parse("https://fake.url")), - offset: 30, - markerType: SpanMarkerType.end), + const SpanMarker( + attribution: LinkAttribution("https://fake.url"), + offset: 26, + markerType: SpanMarkerType.start, + ), + const SpanMarker( + attribution: LinkAttribution("https://fake.url"), + offset: 30, + markerType: SpanMarkerType.end, + ), ], ), ), diff --git a/super_editor/test/super_editor/text_entry/attributions_test.dart b/super_editor/test/super_editor/text_entry/attributions_test.dart index 3b813c07e..0e42369e6 100644 --- a/super_editor/test/super_editor/text_entry/attributions_test.dart +++ b/super_editor/test/super_editor/text_entry/attributions_test.dart @@ -10,7 +10,7 @@ void main() { // Add link across "one two" text.addAttribution( - LinkAttribution(url: Uri.parse('https://flutter.dev')), + const LinkAttribution('https://flutter.dev'), const SpanRange(0, 6), ); @@ -18,7 +18,7 @@ void main() { // an exception expect(() { text.addAttribution( - LinkAttribution(url: Uri.parse('https://pub.dev')), + const LinkAttribution('https://pub.dev'), const SpanRange(4, 12), ); }, throwsA(isA())); @@ -27,7 +27,7 @@ void main() { test('identical link attributions can overlap', () { final text = AttributedText('one two three'); - final linkAttribution = LinkAttribution(url: Uri.parse('https://flutter.dev')); + const linkAttribution = LinkAttribution('https://flutter.dev'); // Add link across "one two" text.addAttribution( @@ -36,7 +36,7 @@ void main() { ); text.addAttribution( - LinkAttribution(url: Uri.parse('https://flutter.dev')), + const LinkAttribution('https://flutter.dev'), const SpanRange(4, 12), ); diff --git a/super_editor/test/super_editor/text_entry/links_test.dart b/super_editor/test/super_editor/text_entry/links_test.dart index f322faa82..51a4aff47 100644 --- a/super_editor/test/super_editor/text_entry/links_test.dart +++ b/super_editor/test/super_editor/text_entry/links_test.dart @@ -45,7 +45,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.com")), + LinkAttribution.fromUri(Uri.parse("https://www.google.com")), }, range: SpanRange(0, text.length - 2), ), @@ -88,7 +88,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.com")), + LinkAttribution.fromUri(Uri.parse("https://www.google.com")), }, range: SpanRange(0, text.length - 1), ), @@ -138,7 +138,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.com")), + LinkAttribution.fromUri(Uri.parse("https://www.google.com")), }, range: SpanRange(12, text.length - 1), ), @@ -189,7 +189,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.com")), + LinkAttribution.fromUri(Uri.parse("https://www.google.com")), }, range: SpanRange(0, text.length - 1), ), @@ -242,7 +242,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.com")), + LinkAttribution.fromUri(Uri.parse("https://www.google.com")), }, range: SpanRange(12, text.length - 1), ), @@ -293,7 +293,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.com")), + LinkAttribution.fromUri(Uri.parse("https://www.google.com")), }, range: SpanRange(0, text.length - 1), ), @@ -347,7 +347,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.com")), + LinkAttribution.fromUri(Uri.parse("https://www.google.com")), }, range: SpanRange(12, text.length - 1), ), @@ -397,7 +397,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.com")), + LinkAttribution.fromUri(Uri.parse("https://www.google.com")), }, range: SpanRange(5, text.length - 1), ), @@ -447,7 +447,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.com")), + LinkAttribution.fromUri(Uri.parse("https://www.google.com")), }, range: SpanRange(12, text.length - 1), ), @@ -500,7 +500,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.com")), + LinkAttribution.fromUri(Uri.parse("https://www.google.com")), }, range: SpanRange(5, text.length - 1), ), @@ -553,7 +553,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.com")), + LinkAttribution.fromUri(Uri.parse("https://www.google.com")), }, range: SpanRange(12, text.length - 1), ), @@ -606,7 +606,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.com")), + LinkAttribution.fromUri(Uri.parse("https://www.google.com")), }, range: SpanRange(5, text.length - 1), ), @@ -660,7 +660,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.com")), + LinkAttribution.fromUri(Uri.parse("https://www.google.com")), }, range: SpanRange(12, text.length - 1), ), @@ -726,7 +726,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.com")), + LinkAttribution.fromUri(Uri.parse("https://www.google.com")), }, range: SpanRange(15, text.length - 1), ), @@ -792,7 +792,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.com")), + LinkAttribution.fromUri(Uri.parse("https://www.google.com")), }, range: SpanRange(12, text.length - 1), ), @@ -861,7 +861,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.com")), + LinkAttribution.fromUri(Uri.parse("https://www.google.com")), }, range: SpanRange(15, text.length - 1), ), @@ -930,7 +930,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.com")), + LinkAttribution.fromUri(Uri.parse("https://www.google.com")), }, range: SpanRange(12, text.length - 1), ), @@ -999,7 +999,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.com")), + LinkAttribution.fromUri(Uri.parse("https://www.google.com")), }, range: SpanRange(15, text.length - 1), ), @@ -1068,7 +1068,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.com")), + LinkAttribution.fromUri(Uri.parse("https://www.google.com")), }, range: SpanRange(12, text.length - 1), ), @@ -1102,7 +1102,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.com")), + LinkAttribution.fromUri(Uri.parse("https://www.google.com")), }, range: const SpanRange(0, 21), ), @@ -1112,7 +1112,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://flutter.dev")), + LinkAttribution.fromUri(Uri.parse("https://flutter.dev")), }, range: const SpanRange(27, 45), ), @@ -1155,7 +1155,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://google.com")), + LinkAttribution.fromUri(Uri.parse("https://google.com")), }, range: SpanRange(0, text.length - 2), ), @@ -1187,7 +1187,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.com")), + LinkAttribution.fromUri(Uri.parse("https://www.google.com")), }, range: SpanRange(0, text.length - 2), ), @@ -1359,7 +1359,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("www.google.com")), + LinkAttribution.fromUri(Uri.parse("www.google.com")), }, range: SpanRange(0, text.length - 1), ), @@ -1389,7 +1389,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.googoooole.com")), + LinkAttribution.fromUri(Uri.parse("https://www.googoooole.com")), }, range: SpanRange(0, text.length - 1), ), @@ -1447,7 +1447,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("www.google.com")), + LinkAttribution.fromUri(Uri.parse("www.google.com")), }, range: SpanRange(0, text.length - 1), ), @@ -1480,7 +1480,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://google.com")), + LinkAttribution.fromUri(Uri.parse("https://google.com")), }, range: SpanRange(0, text.length - 1), ), @@ -1565,7 +1565,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("www.google.com")), + LinkAttribution.fromUri(Uri.parse("www.google.com")), }, range: SpanRange(0, text.length - 1), ), @@ -1603,7 +1603,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.duckduckgo.com")), + LinkAttribution.fromUri(Uri.parse("https://www.duckduckgo.com")), }, range: SpanRange(0, text.length - 1), ), @@ -1661,7 +1661,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("www.google.com")), + LinkAttribution.fromUri(Uri.parse("www.google.com")), }, range: SpanRange(0, text.length - 1), ), @@ -1692,7 +1692,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.google.c")), + LinkAttribution.fromUri(Uri.parse("https://www.google.c")), }, range: SpanRange(0, text.length - 1), ), @@ -1745,7 +1745,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("www.google.com")), + LinkAttribution.fromUri(Uri.parse("www.google.com")), }, range: SpanRange(0, text.length - 1), ), @@ -1775,7 +1775,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("https://www.duckduckgo.com")), + LinkAttribution.fromUri(Uri.parse("https://www.duckduckgo.com")), }, range: SpanRange(0, text.length - 1), ), @@ -1832,7 +1832,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("www.google.com")), + LinkAttribution.fromUri(Uri.parse("www.google.com")), }, range: const SpanRange(0, 12), ), @@ -1841,7 +1841,7 @@ void main() { expect( text.hasAttributionsThroughout( attributions: { - LinkAttribution(url: Uri.parse("www.google.com")), + LinkAttribution.fromUri(Uri.parse("www.google.com")), }, range: SpanRange(13, text.length - 1), ), diff --git a/super_editor/test/super_editor/text_entry/super_editor_content_conversion_test.dart b/super_editor/test/super_editor/text_entry/super_editor_content_conversion_test.dart index 92e5403e3..4fc55af91 100644 --- a/super_editor/test/super_editor/text_entry/super_editor_content_conversion_test.dart +++ b/super_editor/test/super_editor/text_entry/super_editor_content_conversion_test.dart @@ -218,13 +218,13 @@ MutableDocument _singleParagraphWithLinkDoc() { "https://google.com", AttributedSpans( attributions: [ - SpanMarker( - attribution: LinkAttribution(url: Uri.parse('https://google.com')), + const SpanMarker( + attribution: LinkAttribution('https://google.com'), offset: 0, markerType: SpanMarkerType.start, ), - SpanMarker( - attribution: LinkAttribution(url: Uri.parse('https://google.com')), + const SpanMarker( + attribution: LinkAttribution('https://google.com'), offset: 17, markerType: SpanMarkerType.end, ), diff --git a/super_editor/test/super_reader/super_reader_scrolling_test.dart b/super_editor/test/super_reader/super_reader_scrolling_test.dart index fc142e3be..1ae9e2482 100644 --- a/super_editor/test/super_reader/super_reader_scrolling_test.dart +++ b/super_editor/test/super_reader/super_reader_scrolling_test.dart @@ -343,6 +343,37 @@ void main() { expect(scrollController.offset, scrollController.position.maxScrollExtent); }); + testWidgetsOnArbitraryDesktop("does not stop momentum on mouse move", (tester) async { + final scrollController = ScrollController(); + + // Pump a reader with a small size to make it scrollable. + await tester // + .createDocument() // + .withCustomContent(longTextDoc()) // + .withScrollController(scrollController) // + .withEditorSize(const Size(300, 300)) + .pump(); + + // Fling scroll with the trackpad to generate momentum. + await tester.trackpadFling( + find.byType(SuperReader), + const Offset(0.0, -300), + 300.0, + ); + + final scrollOffsetInMiddleOfMomentum = scrollController.offset; + + // Move the mouse around. + final gesture = await tester.createGesture(); + await gesture.moveTo(tester.getTopLeft(find.byType(SuperReader))); + + // Let any momentum run. + await tester.pumpAndSettle(); + + // Ensure that the momentum didn't stop due to mouse movement. + expect(scrollOffsetInMiddleOfMomentum, lessThan(scrollController.offset)); + }); + group("when all content fits in the viewport", () { testWidgetsOnDesktop( "trackpad doesn't scroll content", diff --git a/super_editor/test/test_tools_user_input.dart b/super_editor/test/test_tools_user_input.dart index 418f39076..c416d01b5 100644 --- a/super_editor/test/test_tools_user_input.dart +++ b/super_editor/test/test_tools_user_input.dart @@ -1,5 +1,8 @@ +import 'package:flutter/foundation.dart'; import 'package:flutter/services.dart'; +import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:super_editor/src/infrastructure/platforms/platform.dart'; import 'package:super_editor/super_editor.dart'; final inputSourceVariant = ValueVariant({ @@ -45,3 +48,215 @@ class ImeConnectionWithUpdateCount extends TextInputConnectionDecorator { _contentUpdateCount += 1; } } + +/// An [AutomatedTestWidgetsFlutterBinding] with a fake [HardwareKeyboard], which can be used +/// to simulate keys being pressed while new keys are pressed, e.g., `CMD` being pressed when +/// the user then presses `C` to copy or `V` to paste. +/// +/// When this binding is instantiated, it replaces the standard Flutter test binding. Once this +/// happens, this binding cannot be removed within the given test file. The binding won't be +/// reset until the next test file runs. Therefore, testers must ensure that the presence of this +/// binding throughout a test file won't impact other tests. +/// +/// To help prevent issues with this binding being used in unrelated tests in the same file, +/// a concept of "activation" is included. When this binding is `activate()`d, fakes are +/// be used. When this binding is `deactivate()`d, the regular superclass behaviors are +/// used. A test can use these behaviors as follows: +/// +/// ```dart +/// void main() { +/// final fakeServicesBinding = FakeServicesBinding(); +/// +/// testWidgets('my regular test', (tester) async { +/// // This test doesn't care about the binding. +/// }); +/// +/// testWidgets('my fake binding test', (tester) async { +/// // This test wants to use the fake binding. Activate it. +/// fakeServicesBinding.activate(); +/// +/// // Ensure we deactivate the fake services binding after this test. +/// addTearDown(() => fakeServicesBinding.deactivate()); +/// +/// // Use the binding +/// fakeServicesBinding.fakeKeyboard.isMetaPressed = true; +/// }); +/// } +/// ``` +class FakeServicesBinding extends AutomatedTestWidgetsFlutterBinding { + @override + void initInstances() { + fakeKeyboard = FakeHardwareKeyboard(); + super.initInstances(); + } + + late final FakeHardwareKeyboard fakeKeyboard; + + void activate() => _isActive = true; + bool _isActive = false; + void deactivate() => _isActive = false; + + @override + HardwareKeyboard get keyboard => _isActive ? fakeKeyboard : super.keyboard; +} + +/// A fake [HardwareKeyboard], which can be used to simulate keys being pressed while new +/// keys are pressed, e.g., `CMD` being pressed when the user then presses `C` to copy +/// or `V` to paste. +class FakeHardwareKeyboard extends HardwareKeyboard { + FakeHardwareKeyboard({ + this.isAltPressed = false, + this.isControlPressed = false, + this.isMetaPressed = false, + this.isShiftPressed = false, + }); + + @override + bool isMetaPressed; + @override + bool isControlPressed; + @override + bool isAltPressed; + @override + bool isShiftPressed; + + @override + bool isLogicalKeyPressed(LogicalKeyboardKey key) { + return switch (key) { + LogicalKeyboardKey.shift || LogicalKeyboardKey.shiftLeft || LogicalKeyboardKey.shiftRight => isShiftPressed, + LogicalKeyboardKey.alt || LogicalKeyboardKey.altLeft || LogicalKeyboardKey.altRight => isAltPressed, + LogicalKeyboardKey.control || + LogicalKeyboardKey.controlLeft || + LogicalKeyboardKey.controlRight => + isControlPressed, + LogicalKeyboardKey.meta || LogicalKeyboardKey.metaLeft || LogicalKeyboardKey.metaRight => isMetaPressed, + _ => super.isLogicalKeyPressed(key) + }; + } +} + +/// Generates the default shortcut key bindings based on the [defaultTargetPlatform]. +/// +/// Copied from [WidgetsApp.defaultShortcuts] to make it possible to force the usage of web shortcuts. +Map get defaultFlutterShortcuts { + if (CurrentPlatform.isWeb) { + return defaultWebShortcuts; + } + + switch (defaultTargetPlatform) { + case TargetPlatform.android: + case TargetPlatform.fuchsia: + case TargetPlatform.linux: + case TargetPlatform.windows: + return defaultNonAppleShortcuts; + case TargetPlatform.iOS: + case TargetPlatform.macOS: + return defaultAppleShortcuts; + } +} + +/// Default shortcuts for Windows, Linux, Android and Fuchsia. +/// +/// Copied from [WidgetsApp._defaultShortcuts] because Flutter doesn't expose it and +/// we need to provide these shortcuts during tests in order to be able to simulate +/// web platforms during tests. +/// +/// This map must be kept up to date with [WidgetsApp._defaultShortcuts]. +const Map defaultNonAppleShortcuts = { + // Activation + SingleActivator(LogicalKeyboardKey.enter): ActivateIntent(), + SingleActivator(LogicalKeyboardKey.numpadEnter): ActivateIntent(), + SingleActivator(LogicalKeyboardKey.space): ActivateIntent(), + SingleActivator(LogicalKeyboardKey.gameButtonA): ActivateIntent(), + SingleActivator(LogicalKeyboardKey.select): ActivateIntent(), + + // Dismissal + SingleActivator(LogicalKeyboardKey.escape): DismissIntent(), + + // Keyboard traversal. + SingleActivator(LogicalKeyboardKey.tab): NextFocusIntent(), + SingleActivator(LogicalKeyboardKey.tab, shift: true): PreviousFocusIntent(), + SingleActivator(LogicalKeyboardKey.arrowLeft): DirectionalFocusIntent(TraversalDirection.left), + SingleActivator(LogicalKeyboardKey.arrowRight): DirectionalFocusIntent(TraversalDirection.right), + SingleActivator(LogicalKeyboardKey.arrowDown): DirectionalFocusIntent(TraversalDirection.down), + SingleActivator(LogicalKeyboardKey.arrowUp): DirectionalFocusIntent(TraversalDirection.up), + + // Scrolling + SingleActivator(LogicalKeyboardKey.arrowUp, control: true): ScrollIntent(direction: AxisDirection.up), + SingleActivator(LogicalKeyboardKey.arrowDown, control: true): ScrollIntent(direction: AxisDirection.down), + SingleActivator(LogicalKeyboardKey.arrowLeft, control: true): ScrollIntent(direction: AxisDirection.left), + SingleActivator(LogicalKeyboardKey.arrowRight, control: true): ScrollIntent(direction: AxisDirection.right), + SingleActivator(LogicalKeyboardKey.pageUp): ScrollIntent(direction: AxisDirection.up, type: ScrollIncrementType.page), + SingleActivator(LogicalKeyboardKey.pageDown): + ScrollIntent(direction: AxisDirection.down, type: ScrollIncrementType.page), +}; + +/// Default shortcuts for the Apple platforms. +/// +/// Copied from [WidgetsApp._defaultAppleOsShortcuts] because Flutter doesn't expose it and +/// we need to provide these shortcuts during tests in order to be able to simulate +/// web platforms during tests. +/// +/// This map must be kept up to date with [WidgetsApp._defaultAppleOsShortcuts]. +const Map defaultAppleShortcuts = { + // Activation + SingleActivator(LogicalKeyboardKey.enter): ActivateIntent(), + SingleActivator(LogicalKeyboardKey.numpadEnter): ActivateIntent(), + SingleActivator(LogicalKeyboardKey.space): ActivateIntent(), + + // Dismissal + SingleActivator(LogicalKeyboardKey.escape): DismissIntent(), + + // Keyboard traversal + SingleActivator(LogicalKeyboardKey.tab): NextFocusIntent(), + SingleActivator(LogicalKeyboardKey.tab, shift: true): PreviousFocusIntent(), + SingleActivator(LogicalKeyboardKey.arrowLeft): DirectionalFocusIntent(TraversalDirection.left), + SingleActivator(LogicalKeyboardKey.arrowRight): DirectionalFocusIntent(TraversalDirection.right), + SingleActivator(LogicalKeyboardKey.arrowDown): DirectionalFocusIntent(TraversalDirection.down), + SingleActivator(LogicalKeyboardKey.arrowUp): DirectionalFocusIntent(TraversalDirection.up), + + // Scrolling + SingleActivator(LogicalKeyboardKey.arrowUp, meta: true): ScrollIntent(direction: AxisDirection.up), + SingleActivator(LogicalKeyboardKey.arrowDown, meta: true): ScrollIntent(direction: AxisDirection.down), + SingleActivator(LogicalKeyboardKey.arrowLeft, meta: true): ScrollIntent(direction: AxisDirection.left), + SingleActivator(LogicalKeyboardKey.arrowRight, meta: true): ScrollIntent(direction: AxisDirection.right), + SingleActivator(LogicalKeyboardKey.pageUp): ScrollIntent(direction: AxisDirection.up, type: ScrollIncrementType.page), + SingleActivator(LogicalKeyboardKey.pageDown): + ScrollIntent(direction: AxisDirection.down, type: ScrollIncrementType.page), +}; + +/// Default shortcuts for web. +/// +/// Copied from [WidgetsApp._defaultWebShortcuts] because Flutter doesn't expose it and +/// we need to provide these shortcuts during tests in order to be able to simulate +/// web platforms during tests. +/// +/// This map must be kept up to date with [WidgetsApp._defaultWebShortcuts]. +const Map defaultWebShortcuts = { + // Activation + SingleActivator(LogicalKeyboardKey.space): PrioritizedIntents( + orderedIntents: [ + ActivateIntent(), + ScrollIntent(direction: AxisDirection.down, type: ScrollIncrementType.page), + ], + ), + // On the web, enter activates buttons, but not other controls. + SingleActivator(LogicalKeyboardKey.enter): ButtonActivateIntent(), + SingleActivator(LogicalKeyboardKey.numpadEnter): ButtonActivateIntent(), + + // Dismissal + SingleActivator(LogicalKeyboardKey.escape): DismissIntent(), + + // Keyboard traversal. + SingleActivator(LogicalKeyboardKey.tab): NextFocusIntent(), + SingleActivator(LogicalKeyboardKey.tab, shift: true): PreviousFocusIntent(), + + // Scrolling + SingleActivator(LogicalKeyboardKey.arrowUp): ScrollIntent(direction: AxisDirection.up), + SingleActivator(LogicalKeyboardKey.arrowDown): ScrollIntent(direction: AxisDirection.down), + SingleActivator(LogicalKeyboardKey.arrowLeft): ScrollIntent(direction: AxisDirection.left), + SingleActivator(LogicalKeyboardKey.arrowRight): ScrollIntent(direction: AxisDirection.right), + SingleActivator(LogicalKeyboardKey.pageUp): ScrollIntent(direction: AxisDirection.up, type: ScrollIncrementType.page), + SingleActivator(LogicalKeyboardKey.pageDown): + ScrollIntent(direction: AxisDirection.down, type: ScrollIncrementType.page), +}; diff --git a/super_editor/test_goldens/editor/components/goldens/super_editor_list_item_ordered_aligns_dot_with_text_with_font_sizes.png b/super_editor/test_goldens/editor/components/goldens/super_editor_list_item_ordered_aligns_dot_with_text_with_font_sizes.png new file mode 100644 index 000000000..17f20732c Binary files /dev/null and b/super_editor/test_goldens/editor/components/goldens/super_editor_list_item_ordered_aligns_dot_with_text_with_font_sizes.png differ diff --git a/super_editor/test_goldens/editor/components/goldens/super_editor_list_item_ordered_aligns_dot_with_text_with_font_sizes_and_line_multiplier.png b/super_editor/test_goldens/editor/components/goldens/super_editor_list_item_ordered_aligns_dot_with_text_with_font_sizes_and_line_multiplier.png new file mode 100644 index 000000000..ee63c850c Binary files /dev/null and b/super_editor/test_goldens/editor/components/goldens/super_editor_list_item_ordered_aligns_dot_with_text_with_font_sizes_and_line_multiplier.png differ diff --git a/super_editor/test_goldens/editor/components/goldens/super_editor_list_item_unordered_aligns_dot_with_text_with_font_sizes.png b/super_editor/test_goldens/editor/components/goldens/super_editor_list_item_unordered_aligns_dot_with_text_with_font_sizes.png new file mode 100644 index 000000000..5ee5bd687 Binary files /dev/null and b/super_editor/test_goldens/editor/components/goldens/super_editor_list_item_unordered_aligns_dot_with_text_with_font_sizes.png differ diff --git a/super_editor/test_goldens/editor/components/goldens/super_editor_list_item_unordered_aligns_dot_with_text_with_font_sizes_and_line_multiplier.png b/super_editor/test_goldens/editor/components/goldens/super_editor_list_item_unordered_aligns_dot_with_text_with_font_sizes_and_line_multiplier.png new file mode 100644 index 000000000..53da55311 Binary files /dev/null and b/super_editor/test_goldens/editor/components/goldens/super_editor_list_item_unordered_aligns_dot_with_text_with_font_sizes_and_line_multiplier.png differ diff --git a/super_editor/test_goldens/editor/components/list_items_test.dart b/super_editor/test_goldens/editor/components/list_items_test.dart new file mode 100644 index 000000000..586db3e89 --- /dev/null +++ b/super_editor/test_goldens/editor/components/list_items_test.dart @@ -0,0 +1,158 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:golden_toolkit/golden_toolkit.dart'; +import 'package:super_editor/super_editor.dart'; + +import '../../../test/super_editor/supereditor_test_tools.dart'; +import '../../test_tools_goldens.dart'; + +Future main() async { + await loadAppFonts(); + + group('SuperEditor > list items', () { + group('unordered', () { + testGoldensOnMac('aligns the dot vertically with the text', (tester) async { + await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [ + _createListItemNode(text: 'Font size of 8', fontSize: 8), + _createListItemNode(text: 'Font size of 10', fontSize: 10), + _createListItemNode(text: 'Font size of 12', fontSize: 12), + _createListItemNode(text: 'Font size of 14', fontSize: 14), + _createListItemNode(text: 'Font size of 16', fontSize: 16), + _createListItemNode(text: 'Font size of 18', fontSize: 18), + _createListItemNode(text: 'Font size of 24', fontSize: 24), + _createListItemNode(text: 'Font size of 40', fontSize: 40), + ], + ), + ) + .useStylesheet(_createStylesheet()) + .pump(); + + await screenMatchesGolden(tester, 'super_editor_list_item_unordered_aligns_dot_with_text_with_font_sizes'); + }); + + testGoldensOnMac('aligns the dot vertically with the text with a line multiplier', (tester) async { + await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [ + _createListItemNode(text: 'Font size of 8', fontSize: 8), + _createListItemNode(text: 'Font size of 10', fontSize: 10), + _createListItemNode(text: 'Font size of 12', fontSize: 12), + _createListItemNode(text: 'Font size of 14', fontSize: 14), + _createListItemNode(text: 'Font size of 16', fontSize: 16), + _createListItemNode(text: 'Font size of 18', fontSize: 18), + _createListItemNode(text: 'Font size of 24', fontSize: 24), + _createListItemNode(text: 'Font size of 40', fontSize: 40), + ], + ), + ) + .useStylesheet(_createStylesheet(lineHeightMultiplier: 3.0)) + .pump(); + + await screenMatchesGolden( + tester, 'super_editor_list_item_unordered_aligns_dot_with_text_with_font_sizes_and_line_multiplier'); + }); + }); + + group('ordered', () { + testGoldensOnMac('aligns the dot vertically with the text', (tester) async { + await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [ + _createListItemNode(text: 'Font size of 8', fontSize: 8, listItemType: ListItemType.ordered), + _createListItemNode(text: 'Font size of 10', fontSize: 10, listItemType: ListItemType.ordered), + _createListItemNode(text: 'Font size of 12', fontSize: 12, listItemType: ListItemType.ordered), + _createListItemNode(text: 'Font size of 14', fontSize: 14, listItemType: ListItemType.ordered), + _createListItemNode(text: 'Font size of 16', fontSize: 16, listItemType: ListItemType.ordered), + _createListItemNode(text: 'Font size of 18', fontSize: 18, listItemType: ListItemType.ordered), + _createListItemNode(text: 'Font size of 24', fontSize: 24, listItemType: ListItemType.ordered), + _createListItemNode(text: 'Font size of 40', fontSize: 40, listItemType: ListItemType.ordered), + ], + ), + ) + .useStylesheet(_createStylesheet()) + .pump(); + + await screenMatchesGolden(tester, 'super_editor_list_item_ordered_aligns_dot_with_text_with_font_sizes'); + }); + + testGoldensOnMac('aligns the dot vertically with the text with a line multiplier', (tester) async { + await tester // + .createDocument() + .withCustomContent( + MutableDocument( + nodes: [ + _createListItemNode(text: 'Font size of 8', fontSize: 8, listItemType: ListItemType.ordered), + _createListItemNode(text: 'Font size of 10', fontSize: 10, listItemType: ListItemType.ordered), + _createListItemNode(text: 'Font size of 12', fontSize: 12, listItemType: ListItemType.ordered), + _createListItemNode(text: 'Font size of 14', fontSize: 14, listItemType: ListItemType.ordered), + _createListItemNode(text: 'Font size of 16', fontSize: 16, listItemType: ListItemType.ordered), + _createListItemNode(text: 'Font size of 18', fontSize: 18, listItemType: ListItemType.ordered), + _createListItemNode(text: 'Font size of 24', fontSize: 24, listItemType: ListItemType.ordered), + _createListItemNode(text: 'Font size of 40', fontSize: 40, listItemType: ListItemType.ordered), + ], + ), + ) + .useStylesheet(_createStylesheet(lineHeightMultiplier: 3.0)) + .pump(); + + await screenMatchesGolden( + tester, 'super_editor_list_item_ordered_aligns_dot_with_text_with_font_sizes_and_line_multiplier'); + }); + }); + }); +} + +ListItemNode _createListItemNode({ + required String text, + required double fontSize, + ListItemType listItemType = ListItemType.unordered, +}) { + return ListItemNode( + id: Editor.createNodeId(), + itemType: listItemType, + text: AttributedText( + text, + AttributedSpans(attributions: [ + SpanMarker( + attribution: FontSizeAttribution(fontSize), + offset: 0, + markerType: SpanMarkerType.start, + ), + SpanMarker( + attribution: FontSizeAttribution(fontSize), + offset: text.length - 1, + markerType: SpanMarkerType.end, + ), + ]), + ), + ); +} + +Stylesheet _createStylesheet({ + double lineHeightMultiplier = 1.0, +}) { + return defaultStylesheet.copyWith( + addRulesAfter: [ + StyleRule( + BlockSelector.all, + (doc, docNode) { + return { + Styles.textStyle: TextStyle( + fontFamily: 'Roboto', + height: lineHeightMultiplier, + leadingDistribution: TextLeadingDistribution.even, + ), + }; + }, + ), + ], + ); +} diff --git a/super_editor/test_goldens/editor/goldens/super-editor-image-caret-downstream-android.png b/super_editor/test_goldens/editor/goldens/super-editor-image-caret-downstream-android.png new file mode 100644 index 000000000..c7bbd5540 Binary files /dev/null and b/super_editor/test_goldens/editor/goldens/super-editor-image-caret-downstream-android.png differ diff --git a/super_editor/test_goldens/editor/goldens/super-editor-image-caret-downstream.png b/super_editor/test_goldens/editor/goldens/super-editor-image-caret-downstream-ios.png similarity index 100% rename from super_editor/test_goldens/editor/goldens/super-editor-image-caret-downstream.png rename to super_editor/test_goldens/editor/goldens/super-editor-image-caret-downstream-ios.png diff --git a/super_editor/test_goldens/editor/goldens/super-editor-image-caret-upstream.png b/super_editor/test_goldens/editor/goldens/super-editor-image-caret-downstream-mac.png similarity index 100% rename from super_editor/test_goldens/editor/goldens/super-editor-image-caret-upstream.png rename to super_editor/test_goldens/editor/goldens/super-editor-image-caret-downstream-mac.png diff --git a/super_editor/test_goldens/editor/goldens/super-editor-image-caret-upstream-android.png b/super_editor/test_goldens/editor/goldens/super-editor-image-caret-upstream-android.png new file mode 100644 index 000000000..55a1c0de8 Binary files /dev/null and b/super_editor/test_goldens/editor/goldens/super-editor-image-caret-upstream-android.png differ diff --git a/super_editor/test_goldens/editor/goldens/super-editor-image-caret-upstream-ios.png b/super_editor/test_goldens/editor/goldens/super-editor-image-caret-upstream-ios.png new file mode 100644 index 000000000..ba75df7e0 Binary files /dev/null and b/super_editor/test_goldens/editor/goldens/super-editor-image-caret-upstream-ios.png differ diff --git a/super_editor/test_goldens/editor/goldens/super-editor-image-caret-upstream-mac.png b/super_editor/test_goldens/editor/goldens/super-editor-image-caret-upstream-mac.png new file mode 100644 index 000000000..0d2d71b9f Binary files /dev/null and b/super_editor/test_goldens/editor/goldens/super-editor-image-caret-upstream-mac.png differ diff --git a/super_editor/test_goldens/editor/supereditor_caret_test.dart b/super_editor/test_goldens/editor/supereditor_caret_test.dart index 7893c6794..165ad5c29 100644 --- a/super_editor/test_goldens/editor/supereditor_caret_test.dart +++ b/super_editor/test_goldens/editor/supereditor_caret_test.dart @@ -18,9 +18,40 @@ void main() { ); await tester.pump(); - await screenMatchesGolden(tester, 'super-editor-image-caret-downstream'); + await screenMatchesGolden(tester, 'super-editor-image-caret-downstream-mac'); }); + testGoldensOniOS('shows caret at right side of an image', (tester) async { + await _pumpCaretTestApp(tester); + + // Tap close to the right edge of the editor to place the caret + // downstream on the image. + await tester.tapAt( + tester.getTopRight(find.byType(SuperEditor)) + const Offset(-20, 20), + ); + await tester.pump(); + + await screenMatchesGolden(tester, 'super-editor-image-caret-downstream-ios'); + }); + + testGoldensOnAndroid( + 'shows caret at right side of an image', + (tester) async { + await _pumpCaretTestApp(tester); + + // Tap close to the right edge of the editor to place the caret + // downstream on the image. + await tester.tapAt( + tester.getTopRight(find.byType(SuperEditor)) + const Offset(-20, 20), + ); + await tester.pumpAndSettle(); + + await screenMatchesGolden(tester, 'super-editor-image-caret-downstream-android'); + }, + // TODO: find out why this test fails on CI only. + skip: true, + ); + testGoldensOnMac('shows caret at left side of an image', (tester) async { await _pumpCaretTestApp(tester); @@ -31,8 +62,39 @@ void main() { ); await tester.pump(); - await screenMatchesGolden(tester, 'super-editor-image-caret-upstream'); + await screenMatchesGolden(tester, 'super-editor-image-caret-upstream-mac'); }); + + testGoldensOniOS('shows caret at left side of an image', (tester) async { + await _pumpCaretTestApp(tester); + + // Tap close to the left edge of the editor to place the caret upstream + // on the image. + await tester.tapAt( + tester.getTopLeft(find.byType(SuperEditor)) + const Offset(20, 20), + ); + await tester.pump(); + + await screenMatchesGolden(tester, 'super-editor-image-caret-upstream-ios'); + }); + + testGoldensOnAndroid( + 'shows caret at left side of an image', + (tester) async { + await _pumpCaretTestApp(tester); + + // Tap close to the left edge of the editor to place the caret upstream + // on the image. + await tester.tapAt( + tester.getTopLeft(find.byType(SuperEditor)) + const Offset(20, 20), + ); + await tester.pump(); + + await screenMatchesGolden(tester, 'super-editor-image-caret-upstream-android'); + }, + // TODO: find out why this test fails on CI only. + skip: true, + ); }); } diff --git a/super_editor_markdown/lib/src/markdown_to_document_parsing.dart b/super_editor_markdown/lib/src/markdown_to_document_parsing.dart index 33a78cd5f..d128acd62 100644 --- a/super_editor_markdown/lib/src/markdown_to_document_parsing.dart +++ b/super_editor_markdown/lib/src/markdown_to_document_parsing.dart @@ -495,7 +495,7 @@ class _InlineMarkdownToDocument implements md.NodeVisitor { ); } else if (element.tag == 'a') { styledText.addAttribution( - LinkAttribution(url: Uri.parse(element.attributes['href']!)), + LinkAttribution.fromUri(Uri.parse(element.attributes['href']!)), SpanRange(0, styledText.text.length - 1), ); } diff --git a/super_editor_markdown/test/custom_parsers/callout_block.dart b/super_editor_markdown/test/custom_parsers/callout_block.dart index aab7c1cfa..59d4fe30b 100644 --- a/super_editor_markdown/test/custom_parsers/callout_block.dart +++ b/super_editor_markdown/test/custom_parsers/callout_block.dart @@ -175,7 +175,7 @@ class _InlineMarkdownToDocument implements md.NodeVisitor { ); } else if (element.tag == 'a') { styledText.addAttribution( - LinkAttribution(url: Uri.parse(element.attributes['href']!)), + LinkAttribution.fromUri(Uri.parse(element.attributes['href']!)), SpanRange(0, styledText.text.length - 1), ); } diff --git a/super_editor_markdown/test/super_editor_markdown_test.dart b/super_editor_markdown/test/super_editor_markdown_test.dart index 45f18c220..5484dd0f8 100644 --- a/super_editor_markdown/test/super_editor_markdown_test.dart +++ b/super_editor_markdown/test/super_editor_markdown_test.dart @@ -982,7 +982,7 @@ This is some code expect(styledText.getAllAttributionsAt(8).contains(boldAttribution), true); expect(styledText.getAllAttributionsAt(13).containsAll([boldAttribution, italicsAttribution]), true); expect(styledText.getAllAttributionsAt(19).isEmpty, true); - expect(styledText.getAllAttributionsAt(40).single, LinkAttribution(url: Uri.https('example.org', ''))); + expect(styledText.getAllAttributionsAt(40).single, LinkAttribution.fromUri(Uri.https('example.org', ''))); }); test('paragraph with special HTML symbols keeps the symbols by default', () { @@ -1027,9 +1027,8 @@ This is some code expect(styledText.getAllAttributionsAt(8).contains(boldAttribution), true); expect(styledText.getAllAttributionsAt(13).containsAll([boldAttribution, italicsAttribution]), true); expect( - styledText - .getAllAttributionsAt(20) - .containsAll([boldAttribution, italicsAttribution, LinkAttribution(url: Uri.https('example.org', ''))]), + styledText.getAllAttributionsAt(20).containsAll( + [boldAttribution, italicsAttribution, LinkAttribution.fromUri(Uri.https('example.org', ''))]), true); expect(styledText.getAllAttributionsAt(25).containsAll([boldAttribution, italicsAttribution]), true); }); @@ -1051,7 +1050,7 @@ This is some code expect( styledText .getAllAttributionsAt(13) - .containsAll([boldAttribution, LinkAttribution(url: Uri.https('example.org', ''))]), + .containsAll([boldAttribution, LinkAttribution.fromUri(Uri.https('example.org', ''))]), true); }); @@ -1069,7 +1068,7 @@ This is some code expect(styledText.text, 'This **is a** link test'); expect(styledText.getAllAttributionsAt(9).isEmpty, true); - expect(styledText.getAllAttributionsAt(12).single, LinkAttribution(url: Uri.https('example.org', ''))); + expect(styledText.getAllAttributionsAt(12).single, LinkAttribution.fromUri(Uri.https('example.org', ''))); }); test('empty link', () { @@ -1085,7 +1084,7 @@ This is some code final styledText = paragraph.text; expect(styledText.text, 'This is a link test'); - expect(styledText.getAllAttributionsAt(12).single, LinkAttribution(url: Uri.parse(''))); + expect(styledText.getAllAttributionsAt(12).single, LinkAttribution.fromUri(Uri.parse(''))); }); test('unordered list', () { diff --git a/super_text_layout/lib/src/caret_layer.dart b/super_text_layout/lib/src/caret_layer.dart index d4ee506b9..10d532e20 100644 --- a/super_text_layout/lib/src/caret_layer.dart +++ b/super_text_layout/lib/src/caret_layer.dart @@ -92,7 +92,9 @@ class TextLayoutCaretState extends State with TickerProviderSta bool get isCaretPresent => widget.position != null && widget.position!.offset >= 0; @visibleForTesting - Offset? get caretOffset => isCaretPresent ? widget.textLayout.getOffsetForCaret(widget.position!) : null; + Offset? get caretOffset => isCaretPresent + ? widget.textLayout.getOffsetForCaret(widget.position!).translate(-widget.style.width / 2, 0.0) + : null; @visibleForTesting double? get caretHeight => isCaretPresent