From a4da30e67b848548c9e3125086371dad5a2076e7 Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Wed, 13 Sep 2023 16:06:49 -0300 Subject: [PATCH 1/9] [SuperEditor] Fix crash in release mode (Resolves #1424) (#1425) --- .../src/infrastructure/content_layers.dart | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/super_editor/lib/src/infrastructure/content_layers.dart b/super_editor/lib/src/infrastructure/content_layers.dart index ccbcc7cace..eb37c7fba7 100644 --- a/super_editor/lib/src/infrastructure/content_layers.dart +++ b/super_editor/lib/src/infrastructure/content_layers.dart @@ -1,3 +1,4 @@ +import 'package:flutter/foundation.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter/scheduler.dart'; import 'package:flutter/widgets.dart'; @@ -423,6 +424,17 @@ class RenderContentLayers extends RenderBox { RenderBox? _content; final _overlays = []; + /// Whether this render object's layout information or its content + /// layout information is dirty. + /// + /// This is set to `true` when `markNeedsLayout` is called and it's + /// set to `false` after laying out the content. + bool get contentNeedsLayout => _contentNeedsLayout; + bool _contentNeedsLayout = true; + + /// Whether we are at the middle of a [performLayout] call. + bool _runningLayout = false; + @override void attach(PipelineOwner owner) { contentLayersLog.info("Attaching RenderContentLayers to owner: $owner"); @@ -446,6 +458,19 @@ class RenderContentLayers extends RenderBox { }); } + @override + void markNeedsLayout() { + super.markNeedsLayout(); + + if (_runningLayout) { + // We are already in a layout phase. + // When we call ContentLayerElement.buildLayers, markNeedsLayout is called again. + // We don't to mark the content as dirty, because otherwise the layers will never build. + return; + } + _contentNeedsLayout = true; + } + @override List debugDescribeChildren() { final childDiagnostics = []; @@ -572,9 +597,12 @@ class RenderContentLayers extends RenderBox { contentLayersLog.info("Laying out ContentLayers"); if (_content == null) { size = Size.zero; + _contentNeedsLayout = false; return; } + _runningLayout = true; + // Always layout the content first, so that layers can inspect the content layout. contentLayersLog.fine("Laying out content - $_content"); _content!.layout(constraints, parentUsesSize: true); @@ -583,6 +611,8 @@ class RenderContentLayers extends RenderBox { // The size of the layers, and the our size, is exactly the same as the content. size = _content!.size; + _contentNeedsLayout = false; + // Build the underlay and overlays during the layout phase so that they can inspect an // up-to-date content layout. // @@ -606,6 +636,8 @@ class RenderContentLayers extends RenderBox { contentLayersLog.fine("Laying out overlay: $overlay"); overlay.layout(layerConstraints); } + + _runningLayout = false; contentLayersLog.finer("Done laying out layers"); } @@ -841,7 +873,7 @@ abstract class ContentLayerState Date: Fri, 15 Sep 2023 14:48:16 -0300 Subject: [PATCH 2/9] [SuperEditor][SuperReader] Remove scroll listener when disposing (Resolves #1117) (#1448) --- .../document_gestures_touch_android.dart | 4 +- .../document_gestures_touch_ios.dart | 13 +- ...nly_document_android_touch_interactor.dart | 4 +- ...ad_only_document_ios_touch_interactor.dart | 13 +- .../supereditor_switching_test.dart | 130 ++++++++++++++++++ 5 files changed, 152 insertions(+), 12 deletions(-) create mode 100644 super_editor/test/super_editor/supereditor_switching_test.dart 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 dd54026fcc..f13082f8ce 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 @@ -249,9 +249,7 @@ class _AndroidDocumentTouchInteractorState extends State _removeEditingOverlayControls(); _teardownScrollController(); + _activeScrollPosition?.removeListener(_onScrollChange); _handleAutoScrolling.dispose(); @@ -895,9 +896,13 @@ class _IOSDocumentTouchInteractorState extends State if (scrollPosition is ScrollPositionWithSingleContext) { (scrollPosition as ScrollPositionWithSingleContext).goBallistic(-details.velocity.pixelsPerSecond.dy); - // We add the scroll change listener again, because going ballistic - // seems to switch out the scroll position. - scrollPosition.addListener(_onScrollChange); + if (_activeScrollPosition != scrollPosition) { + // We add the scroll change listener again, because going ballistic + // seems to switch out the scroll position. + _activeScrollPosition?.removeListener(_onScrollChange); + _activeScrollPosition = scrollPosition; + scrollPosition.addListener(_onScrollChange); + } } } else { // The user was dragging a handle. Stop any auto-scrolling that may have started. @@ -1260,6 +1265,8 @@ class _IOSDocumentTouchInteractorState extends State scheduleBuildAfterBuild(); } else { if (scrollPosition != _activeScrollPosition) { + _activeScrollPosition?.removeListener(_onScrollChange); + _activeScrollPosition = scrollPosition; _activeScrollPosition?.addListener(_onScrollChange); } diff --git a/super_editor/lib/src/super_reader/read_only_document_android_touch_interactor.dart b/super_editor/lib/src/super_reader/read_only_document_android_touch_interactor.dart index 39bdc6ae81..769341d45a 100644 --- a/super_editor/lib/src/super_reader/read_only_document_android_touch_interactor.dart +++ b/super_editor/lib/src/super_reader/read_only_document_android_touch_interactor.dart @@ -230,9 +230,7 @@ class _ReadOnlyAndroidDocumentTouchInteractorState extends State(true); + final document = longDoc(); + final scrollController = ScrollController(); + + await tester.pumpWidget( + MaterialApp( + home: _EditorReaderSwitchDemo( + document: document, + isEditable: isEditable, + scrollController: scrollController, + ), + ), + ); + + // Select the first word and scroll the viewport an arbitrary amount of pixels. + await SuperEditorRobot(tester).doubleTapInParagraph('1', 0); + scrollController.jumpTo(100.0); + await tester.pump(); + + // Switch the SuperEditor with a SuperReader. + isEditable.value = false; + await tester.pump(); + + // Select the first word and scroll the viewport an arbitrary amount of pixels. + await SuperReaderRobot(tester).doubleTapInParagraph('1', 0); + scrollController.jumpTo(150.0); + await tester.pump(); + + // Switch back to the editor. + isEditable.value = true; + await tester.pump(); + + // Select the first word and scroll the viewport an arbitrary amount of pixels. + await SuperEditorRobot(tester).doubleTapInParagraph('1', 0); + scrollController.jumpTo(200.0); + await tester.pump(); + + // Reaching this point means we switched between SuperEditor and SuperReader without any crashes. + }); + }); +} + +/// A Scaffold which switches between a [SuperEditor] and a [SuperEditor] depending +/// on the value of [isEditable]. +class _EditorReaderSwitchDemo extends StatefulWidget { + const _EditorReaderSwitchDemo({ + Key? key, + required this.isEditable, + required this.document, + required this.scrollController, + }) : super(key: key); + + final MutableDocument document; + + /// When `true` a [SuperEditor] is displayed. Otherwise, display a [SuperReader]. + final ValueListenable isEditable; + + /// Scroll controller of the viewport. + final ScrollController scrollController; + + @override + State<_EditorReaderSwitchDemo> createState() => _EditorReaderSwitchDemoState(); +} + +class _EditorReaderSwitchDemoState extends State<_EditorReaderSwitchDemo> { + late Editor _docEditor; + late MutableDocumentComposer _composer; + + @override + void initState() { + _composer = MutableDocumentComposer(); + _docEditor = createDefaultDocumentEditor( + document: widget.document, + composer: _composer, + ); + + super.initState(); + } + + @override + Widget build(BuildContext context) { + return Scaffold( + body: CustomScrollView( + controller: widget.scrollController, + slivers: [ + const SliverAppBar(), + SliverToBoxAdapter( + child: ListenableBuilder( + listenable: widget.isEditable, + builder: (context, _) { + return _buildEditorOrReader(); + }, + ), + ) + ], + ), + ); + } + + Widget _buildEditorOrReader() { + if (widget.isEditable.value) { + return SuperEditor( + document: widget.document, + composer: _composer, + editor: _docEditor, + ); + } else { + return SuperReader( + document: widget.document, + ); + } + } +} From ecdb0baa7799939f335ffcdafff33335fd0c1a56 Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Fri, 15 Sep 2023 14:48:43 -0300 Subject: [PATCH 3/9] [SuperEditor][SuperReader] Remove scroll listener when disposing (Resolves #1117) (#1448) From 0ab5f33d8a97f59bd9dc6fd843e05ba2b4454f88 Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Fri, 15 Sep 2023 14:50:15 -0300 Subject: [PATCH 4/9] [SuperEditor][iOS] Fix floating cursor crash with expanded selections (Resolves #1174) (#1450) --- .../platforms/ios/ios_document_controls.dart | 1 + .../supereditor_floating_cursor_test.dart | 45 +++++++++++++++++++ 2 files changed, 46 insertions(+) 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 d734256726..35aa51b25c 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 @@ -217,6 +217,7 @@ class _IosDocumentTouchEditingControlsState extends State _onFloatingCursorChange()); + return; } if (_floatingCursorOffset.value == null) { diff --git a/super_editor/test/super_editor/supereditor_floating_cursor_test.dart b/super_editor/test/super_editor/supereditor_floating_cursor_test.dart index cf05038d2c..2154dab379 100644 --- a/super_editor/test/super_editor/supereditor_floating_cursor_test.dart +++ b/super_editor/test/super_editor/supereditor_floating_cursor_test.dart @@ -1,6 +1,9 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test_runners/flutter_test_runners.dart'; +import 'package:super_editor/src/core/document.dart'; +import 'package:super_editor/src/core/document_selection.dart'; +import 'package:super_editor/src/default_editor/text.dart'; import 'package:super_editor/src/infrastructure/blinking_caret.dart'; import 'package:super_editor/src/test/super_editor_test/supereditor_inspector.dart'; import 'package:super_editor/src/test/super_editor_test/supereditor_robot.dart'; @@ -135,6 +138,48 @@ void main() { // M3 theme's primary color's value, Color(0xff6750a4). expect(caret.color, Theme.of(tester.firstElement(_caretFinder())).primaryColor); }); + + testWidgetsOnIos('collapses an expanded selection', (tester) async { + final testContext = await tester // + .createDocument() + .fromMarkdown('This is a paragraph') + .pump(); + + final nodeId = testContext.document.nodes.first.id; + + // Double tap to select the word "This" + await tester.doubleTapInParagraph(nodeId, 0); + + // Ensure the word is selected. + expect( + SuperEditorInspector.findDocumentSelection(), + DocumentSelection( + base: DocumentPosition( + nodeId: nodeId, + nodePosition: const TextNodePosition(offset: 0), + ), + extent: DocumentPosition( + nodeId: nodeId, + nodePosition: const TextNodePosition(offset: 4), + ), + ), + ); + + // Show the floating cursor. + await tester.startFloatingCursorGesture(); + await tester.pump(); + + // Ensure the selection collapsed. + expect( + SuperEditorInspector.findDocumentSelection(), + DocumentSelection.collapsed( + position: DocumentPosition( + nodeId: nodeId, + nodePosition: const TextNodePosition(offset: 4, affinity: TextAffinity.upstream), + ), + ), + ); + }); }); }); } From d8ddb1a578fcce975560c82aec08bc820a58ce9b Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Fri, 15 Sep 2023 15:06:48 -0300 Subject: [PATCH 5/9] [SuperTextField][Mac] Allow selector handlers configuration (Resolves #1439) (#1444) --- .../desktop/desktop_textfield.dart | 9 +++- .../src/super_textfield/super_textfield.dart | 8 ++++ .../super_textfield_ime_test.dart | 41 +++++++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) 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 b5a2690878..5821066161 100644 --- a/super_editor/lib/src/super_textfield/desktop/desktop_textfield.dart +++ b/super_editor/lib/src/super_textfield/desktop/desktop_textfield.dart @@ -64,6 +64,7 @@ class SuperDesktopTextField extends StatefulWidget { this.inputSource = TextInputSource.keyboard, this.textInputAction, this.imeConfiguration, + this.selectorHandlers, List? keyboardHandlers, }) : keyboardHandlers = keyboardHandlers ?? (inputSource == TextInputSource.keyboard @@ -124,6 +125,12 @@ class SuperDesktopTextField extends StatefulWidget { /// that input text based on individual character key presses. final List keyboardHandlers; + /// Handlers for all Mac OS "selectors" reported by the IME. + /// + /// The IME reports selectors as unique `String`s, therefore selector handlers are + /// defined as a mapping from selector names to handler functions. + final Map? selectorHandlers; + /// The type of action associated with ENTER key. /// /// This property is ignored when an [imeConfiguration] is provided. @@ -405,7 +412,7 @@ class SuperDesktopTextFieldState extends State implements focusNode: _focusNode, textController: _controller, isMultiline: isMultiline, - selectorHandlers: defaultTextFieldSelectorHandlers, + selectorHandlers: widget.selectorHandlers ?? defaultTextFieldSelectorHandlers, textInputAction: widget.textInputAction, imeConfiguration: widget.imeConfiguration, child: child, diff --git a/super_editor/lib/src/super_textfield/super_textfield.dart b/super_editor/lib/src/super_textfield/super_textfield.dart index 9c2b4cd3a3..78c2555718 100644 --- a/super_editor/lib/src/super_textfield/super_textfield.dart +++ b/super_editor/lib/src/super_textfield/super_textfield.dart @@ -69,6 +69,7 @@ class SuperTextField extends StatefulWidget { this.lineHeight, this.inputSource, this.keyboardHandlers, + this.selectorHandlers, this.padding, this.textInputAction, this.imeConfiguration, @@ -173,6 +174,12 @@ class SuperTextField extends StatefulWidget { /// Only used on desktop. final List? keyboardHandlers; + /// Handlers for all Mac OS "selectors" reported by the IME. + /// + /// The IME reports selectors as unique `String`s, therefore selector handlers are + /// defined as a mapping from selector names to handler functions. + final Map? selectorHandlers; + /// Padding placed around the text content of this text field, but within the /// scrollable viewport. final EdgeInsets? padding; @@ -327,6 +334,7 @@ class SuperTextFieldState extends State implements ImeInputOwner minLines: widget.minLines, maxLines: widget.maxLines, keyboardHandlers: widget.keyboardHandlers, + selectorHandlers: widget.selectorHandlers, padding: widget.padding ?? EdgeInsets.zero, inputSource: _inputSource, textInputAction: _textInputAction, diff --git a/super_editor/test/super_textfield/super_textfield_ime_test.dart b/super_editor/test/super_textfield/super_textfield_ime_test.dart index a7778085ec..19e53081cb 100644 --- a/super_editor/test/super_textfield/super_textfield_ime_test.dart +++ b/super_editor/test/super_textfield/super_textfield_ime_test.dart @@ -3,6 +3,7 @@ import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test_robots/flutter_test_robots.dart'; import 'package:flutter_test_runners/flutter_test_runners.dart'; +import 'package:super_editor/src/infrastructure/platforms/mac/mac_ime.dart'; import 'package:super_editor/super_editor.dart'; import 'package:super_editor/super_editor_test.dart'; @@ -589,6 +590,46 @@ void main() { expect(enableDeltaModel, true); expect(keyboardAppearance, 'Brightness.dark'); }); + + testWidgetsOnMac('allows apps to handle selectors in their own way', (tester) async { + bool customHandlerCalled = false; + + final controller = AttributedTextEditingController( + text: AttributedText('Selectors test'), + ); + + await tester.pumpWidget( + _buildScaffold( + child: SuperTextField( + textController: controller, + inputSource: TextInputSource.ime, + selectorHandlers: { + MacOsSelectors.moveRight: ({ + required AttributedTextEditingController controller, + required textLayout, + }) { + customHandlerCalled = true; + } + }, + ), + ), + ); + + // Place the caret at the beginning of the text field. + await tester.placeCaretInSuperTextField(0); + + // Press right arrow key to trigger the MacOsSelectors.moveRight selector. + await tester.pressRightArrow(); + + // Ensure the custom handler was called. + expect(customHandlerCalled, isTrue); + + // Ensure that the textfield didn't execute the default handler for the MacOsSelectors.moveRight selector. + expect( + SuperTextFieldInspector.findSelection(), + const TextSelection.collapsed(offset: 0), + ); + }); }); group('SuperTextField on some bad Android software keyboards', () { From 29b71b05bc2b56d715b04ba01356caf393623eca Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Fri, 15 Sep 2023 21:25:41 -0700 Subject: [PATCH 6/9] Tap to open multiple links in the same paragraph (Resolves #1426) (#1442) --- attributed_text/lib/src/attributed_spans.dart | 2 +- .../supereditor_gestures_test.dart | 50 ++++++++++++++ .../super_editor/text_entry/links_test.dart | 65 ++++++++++++++----- 3 files changed, 98 insertions(+), 19 deletions(-) diff --git a/attributed_text/lib/src/attributed_spans.dart b/attributed_text/lib/src/attributed_spans.dart index 3b6545b03a..92e7864dba 100644 --- a/attributed_text/lib/src/attributed_spans.dart +++ b/attributed_text/lib/src/attributed_spans.dart @@ -265,7 +265,7 @@ class AttributedSpans { return _markers // .reversed // search from the end so its the nearest start marker .where((marker) { - return attribution == null || (marker.attribution.id == attribution.id); + return attribution == null || (marker.attribution == attribution); }).firstWhereOrNull((marker) => marker.isStart && marker.offset <= offset); } diff --git a/super_editor/test/super_editor/supereditor_gestures_test.dart b/super_editor/test/super_editor/supereditor_gestures_test.dart index 81469cb83a..1f1baa5dbf 100644 --- a/super_editor/test/super_editor/supereditor_gestures_test.dart +++ b/super_editor/test/super_editor/supereditor_gestures_test.dart @@ -537,6 +537,56 @@ spans multiple lines.''', expect(testUrlLauncher.urlLaunchLog.length, 1); expect(testUrlLauncher.urlLaunchLog.first.toString(), "https://fake.url"); }); + + testWidgetsOnAllPlatforms("launches different URLs on tap", (tester) async { + // Setup test version of UrlLauncher to log URL launches. + final testUrlLauncher = TestUrlLauncher(); + UrlLauncher.instance = testUrlLauncher; + addTearDown(() => UrlLauncher.instance = null); + + // Pump the UI. + final context = await tester // + .createDocument() + .fromMarkdown("[Google](https://google.com) and [Flutter](https://flutter.dev)") + .autoFocus(true) + .pump(); + + // Activate interaction mode. + if (defaultTargetPlatform == TargetPlatform.android || defaultTargetPlatform == TargetPlatform.iOS) { + // On mobile, there's no hardware keyboard to easily activate + // interaction mode. In practice, app developers will decide + // when/how to activate interaction mode on mobile. Rather than + // add buttons in our test just for this purpose, we'll explicitly + // activate interaction mode. + context.findEditContext().editor.execute([ + const ChangeInteractionModeRequest(isInteractionModeDesired: true), + ]); + } else if (defaultTargetPlatform == TargetPlatform.macOS) { + // Press CMD to activate interaction mode on Mac. + await tester.sendKeyDownEvent(LogicalKeyboardKey.meta); + } else { + // Press CTRL to activate interaction mode on Windows and Linux. + await tester.sendKeyDownEvent(LogicalKeyboardKey.control); + } + + // Ensure that interaction mode is "on". + expect(context.findEditContext().composer.isInInteractionMode.value, isTrue); + + // Tap on the first link. + final textNode = context.document.nodes.first; + await tester.tapInParagraph(textNode.id, 3); + + // Ensure that we tried to launch the first URL. + expect(testUrlLauncher.urlLaunchLog.length, 1); + expect(testUrlLauncher.urlLaunchLog.first.toString(), "https://google.com"); + + // Tap on the second link. + await tester.tapInParagraph(textNode.id, 14); + + // Ensure that we tried to launch the second URL. + expect(testUrlLauncher.urlLaunchLog.length, 2); + expect(testUrlLauncher.urlLaunchLog.last.toString(), "https://flutter.dev"); + }); }); group("when inactive", () { 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 c0fc78bf41..9adecb4559 100644 --- a/super_editor/test/super_editor/text_entry/links_test.dart +++ b/super_editor/test/super_editor/text_entry/links_test.dart @@ -15,17 +15,14 @@ void main() { .withInputSource(TextInputSource.ime) .pump(); - final doc = SuperEditorInspector.findDocument()!; - // Place the caret at the beginning of the empty document. - await tester.placeCaretInParagraph(doc.nodes.first.id, 0); + await tester.placeCaretInParagraph("1", 0); // Type a URL. It shouldn't linkify until we add a space. await tester.typeImeText("https://www.google.com"); // Ensure it's not linkified yet. - final nodeId = doc.nodes.first.id; - var text = SuperEditorInspector.findTextInParagraph(nodeId); + var text = SuperEditorInspector.findTextInParagraph("1"); expect(text.text, "https://www.google.com"); expect( @@ -40,7 +37,7 @@ void main() { await tester.typeImeText(" "); // Ensure it's linkified. - text = SuperEditorInspector.findTextInParagraph(nodeId); + text = SuperEditorInspector.findTextInParagraph("1"); expect(text.text, "https://www.google.com "); expect( @@ -54,24 +51,59 @@ void main() { ); }); - testWidgetsOnAllPlatforms('recognizes a URL without www and converts it to a link', (tester) async { + testWidgetsOnAllPlatforms('recognizes a second URL when typing and converts it to a link', (tester) async { await tester // .createDocument() .withSingleEmptyParagraph() .withInputSource(TextInputSource.ime) .pump(); - final doc = SuperEditorInspector.findDocument()!; + // Place the caret at the beginning of the empty document. + await tester.placeCaretInParagraph("1", 0); + + // Type text with two URLs + await tester.typeImeText("https://www.google.com and https://flutter.dev "); + + // Ensure both URLs are linkified with the correct URLs. + final text = SuperEditorInspector.findTextInParagraph("1"); + + expect(text.text, "https://www.google.com and https://flutter.dev "); + expect( + text.hasAttributionsThroughout( + attributions: { + LinkAttribution(url: Uri.parse("https://www.google.com")), + }, + range: const SpanRange(0, 21), + ), + isTrue, + ); + + expect( + text.hasAttributionsThroughout( + attributions: { + LinkAttribution(url: Uri.parse("https://flutter.dev")), + }, + range: const SpanRange(27, 45), + ), + isTrue, + ); + }); + + testWidgetsOnAllPlatforms('recognizes a URL without www and converts it to a link', (tester) async { + await tester // + .createDocument() + .withSingleEmptyParagraph() + .withInputSource(TextInputSource.ime) + .pump(); // Place the caret at the beginning of the empty document. - await tester.placeCaretInParagraph(doc.nodes.first.id, 0); + await tester.placeCaretInParagraph("1", 0); // Type a URL without the www. It shouldn't linkify until we add a space. await tester.typeImeText("google.com"); // Ensure it's not linkified yet. - final nodeId = doc.nodes.first.id; - var text = SuperEditorInspector.findTextInParagraph(nodeId); + var text = SuperEditorInspector.findTextInParagraph("1"); expect(text.text, "google.com"); expect( @@ -86,7 +118,7 @@ void main() { await tester.typeImeText(" "); // Ensure it's linkified. - text = SuperEditorInspector.findTextInParagraph(nodeId); + text = SuperEditorInspector.findTextInParagraph("1"); expect(text.text, "google.com "); expect( @@ -107,10 +139,8 @@ void main() { .withInputSource(TextInputSource.ime) .pump(); - final doc = SuperEditorInspector.findDocument()!; - // Place the caret at the beginning of the empty document. - await tester.placeCaretInParagraph(doc.nodes.first.id, 0); + await tester.placeCaretInParagraph("1", 0); // Type a URL. It shouldn't linkify until we add a space. await tester.typeImeText("www.google.com"); @@ -119,9 +149,8 @@ void main() { await tester.typeImeText(" "); // Ensure it's linkified with a URL schema. - final nodeId = doc.nodes.first.id; - var text = SuperEditorInspector.findTextInParagraph(nodeId); - text = SuperEditorInspector.findTextInParagraph(nodeId); + var text = SuperEditorInspector.findTextInParagraph("1"); + text = SuperEditorInspector.findTextInParagraph("1"); expect(text.text, "www.google.com "); expect( From aee7bf5efc13281523d6e147733de8536d99621f Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Wed, 20 Sep 2023 21:17:44 -0300 Subject: [PATCH 7/9] [SuperEditor][SuperTextField][Web] Fix IME panels positioning (Resolves #1356) (#1418) --- .../document_keyboard_actions.dart | 18 +++ .../supereditor_ime_interactor.dart | 149 ++++++++++++++++-- .../lib/src/default_editor/tasks.dart | 25 +-- super_editor/lib/src/default_editor/text.dart | 22 +++ .../desktop/desktop_textfield.dart | 114 ++++++++++++-- 5 files changed, 295 insertions(+), 33 deletions(-) diff --git a/super_editor/lib/src/default_editor/document_hardware_keyboard/document_keyboard_actions.dart b/super_editor/lib/src/default_editor/document_hardware_keyboard/document_keyboard_actions.dart index 5e748acc20..18951fc760 100644 --- a/super_editor/lib/src/default_editor/document_hardware_keyboard/document_keyboard_actions.dart +++ b/super_editor/lib/src/default_editor/document_hardware_keyboard/document_keyboard_actions.dart @@ -517,6 +517,15 @@ ExecutionInstruction moveUpAndDownWithArrowKeys({ return ExecutionInstruction.continueExecution; } + if (isWeb && (editContext.hasPrimaryFocus.value) && (editContext.composer.composingRegion.value != null)) { + // We are composing a character on web. It's possible that a native element is being displayed, + // like an emoji picker or a character selection panel. + // We need to let the OS handle the key so the user can navigate + // on the list of possible characters. + // TODO: update this after https://github.com/flutter/flutter/issues/134268 is resolved. + return ExecutionInstruction.blocked; + } + if (defaultTargetPlatform == TargetPlatform.windows && keyEvent.isAltPressed) { return ExecutionInstruction.continueExecution; } @@ -569,6 +578,15 @@ ExecutionInstruction moveLeftAndRightWithArrowKeys({ return ExecutionInstruction.continueExecution; } + if (isWeb && (editContext.hasPrimaryFocus.value) && (editContext.composer.composingRegion.value != null)) { + // We are composing a character on web. It's possible that a native element is being displayed, + // like an emoji picker or a character selection panel. + // We need to let the OS handle the key so the user can navigate + // on the list of possible characters. + // TODO: update this after https://github.com/flutter/flutter/issues/134268 is resolved. + return ExecutionInstruction.blocked; + } + if (defaultTargetPlatform == TargetPlatform.windows && keyEvent.isAltPressed) { return ExecutionInstruction.continueExecution; } diff --git a/super_editor/lib/src/default_editor/document_ime/supereditor_ime_interactor.dart b/super_editor/lib/src/default_editor/document_ime/supereditor_ime_interactor.dart index 86b3b381c8..213c54a8a6 100644 --- a/super_editor/lib/src/default_editor/document_ime/supereditor_ime_interactor.dart +++ b/super_editor/lib/src/default_editor/document_ime/supereditor_ime_interactor.dart @@ -6,6 +6,7 @@ import 'package:flutter/services.dart'; import 'package:super_editor/src/core/document_layout.dart'; import 'package:super_editor/src/core/edit_context.dart'; import 'package:super_editor/src/default_editor/debug_visualization.dart'; +import 'package:super_editor/src/default_editor/text.dart'; import 'package:super_editor/src/infrastructure/_logging.dart'; import 'package:super_editor/src/infrastructure/flutter/flutter_pipeline.dart'; import 'package:super_editor/src/infrastructure/ime_input_owner.dart'; @@ -232,11 +233,13 @@ class SuperEditorImeInteractorState extends State impl if (_imeConnection.value == null) { _documentImeConnection.value = null; widget.imeOverrides?.client = null; - } else { - _configureImeClientDecorators(); - _documentImeConnection.value = _documentImeClient; - _reportVisualInformationToIme(); + return; } + + _configureImeClientDecorators(); + _documentImeConnection.value = _documentImeClient; + + _reportVisualInformationToIme(); } void _configureImeClientDecorators() { @@ -248,7 +251,7 @@ class SuperEditorImeInteractorState extends State impl _imeClient.client = widget.imeOverrides ?? _documentImeClient; } - /// Report our size, transform to the root node coordinates, and caret rect to the IME. + /// Report the global size and transform of the editor and the caret rect to the IME. /// /// This is needed to display the OS emoji & symbols panel at the editor selected position. /// @@ -258,13 +261,9 @@ class SuperEditorImeInteractorState extends State impl return; } - final renderBox = context.findRenderObject() as RenderBox; - _imeConnection.value!.setEditableSizeAndTransform(renderBox.size, renderBox.getTransformTo(null)); - - final caretRect = _computeCaretRectInViewportSpace(); - if (caretRect != null) { - _imeConnection.value!.setCaretRect(caretRect); - } + _reportSizeAndTransformToIme(); + _reportCaretRectToIme(); + _reportTextStyleToIme(); // There are some operations that might affect our transform, size and the caret rect, // but we can't react to them. @@ -274,6 +273,103 @@ class SuperEditorImeInteractorState extends State impl onNextFrame((_) => _reportVisualInformationToIme()); } + /// Report the global size and transform of the editor to the IME. + /// + /// This is needed to display the OS emoji & symbols panel at the editor selected position. + void _reportSizeAndTransformToIme() { + late Size size; + late Matrix4 transform; + + if (isWeb) { + // On web, we can't set the caret rect. + // To display the IME panels at the correct position, + // instead of reporting the whole editor size and transform, + // we report only the information about the selected node. + final sizeAndTransform = _computeSizeAndTransformOfSelectNode(); + if (sizeAndTransform == null) { + return; + } + + (size, transform) = sizeAndTransform; + } else { + final renderBox = context.findRenderObject() as RenderBox; + + size = renderBox.size; + transform = renderBox.getTransformTo(null); + } + + _imeConnection.value!.setEditableSizeAndTransform(size, transform); + } + + void _reportCaretRectToIme() { + if (isWeb) { + // On web, setting the caret rect isn't supported. + // To position the IME popovers, we report the size, transform and style + // of the selected component and let the browser position the popovers. + return; + } + + final caretRect = _computeCaretRectInViewportSpace(); + if (caretRect != null) { + _imeConnection.value!.setCaretRect(caretRect); + } + } + + /// Report our text style to the IME. + /// + /// This is used on web to set the text style of the hidden native input, + /// to try to match the text size on the browser with our text size. + /// + /// As our content can have multiple styles, the sizes won't be 100% in sync. + /// + /// TODO: update this after https://github.com/flutter/flutter/issues/134265 is resolved. + void _reportTextStyleToIme() { + if (!isWeb) { + // If we are not on the web, we can position the caret rect without the need + // to send the text styles to the IME. + return; + } + + final selection = widget.editContext.composer.selection; + if (selection == null) { + return; + } + + final nodePosition = selection.extent.nodePosition; + if (nodePosition is! TextNodePosition) { + // The selected component doesn't contain text. + return; + } + + final docLayout = widget.editContext.documentLayout; + + DocumentComponent? selectedComponent = docLayout.getComponentByNodeId(selection.extent.nodeId); + if (selectedComponent is ProxyDocumentComponent) { + // The selected componente is a proxy. + // If this component displays text, the text component is bounded to childDocumentComponentKey. + selectedComponent = selectedComponent.childDocumentComponentKey.currentState as DocumentComponent?; + } + + if (selectedComponent == null) { + editorImeLog.warning('A selection exists but no component for node ${selection.extent.nodeId} was found'); + return; + } + + if (selectedComponent is! TextComponentState) { + // The selected component isn't a text component. We can't query its style. + return; + } + + final style = selectedComponent.getTextStyleAt(nodePosition.offset); + _imeConnection.value!.setStyle( + fontFamily: style.fontFamily, + fontSize: style.fontSize, + fontWeight: style.fontWeight, + textDirection: selectedComponent.textDirection ?? TextDirection.ltr, + textAlign: selectedComponent.textAlign ?? TextAlign.left, + ); + } + /// Compute the caret rect in the editor's content space. /// /// Returns `null` if we don't have a selection or if we can't get the caret rect @@ -303,6 +399,35 @@ class SuperEditorImeInteractorState extends State impl return caretOffset & rectInDocLayoutSpace.size; } + /// Compute the size and transform of the selected node's visual component + /// to the global coordinates. + /// + /// Returns `null` if the we don't have a selection, or if we can't find + /// a component for the selected node. + (Size size, Matrix4 transform)? _computeSizeAndTransformOfSelectNode() { + final selection = widget.editContext.composer.selection; + if (selection == null) { + return null; + } + + final documentLayout = widget.editContext.documentLayout; + + DocumentComponent? selectedComponent = documentLayout.getComponentByNodeId(selection.extent.nodeId); + if (selectedComponent is ProxyDocumentComponent) { + // The selected componente is a proxy. + // If this component displays text, the text component is bounded to childDocumentComponentKey. + selectedComponent = selectedComponent.childDocumentComponentKey.currentState as DocumentComponent; + } + + if (selectedComponent == null) { + editorImeLog.warning('A selection exists but no component for node ${selection.extent.nodeId} was found'); + return null; + } + + final renderBox = selectedComponent.context.findRenderObject() as RenderBox; + return (renderBox.size, renderBox.getTransformTo(null)); + } + void _onPerformSelector(String selectorName) { final handler = widget.selectorHandlers[selectorName]; if (handler == null) { diff --git a/super_editor/lib/src/default_editor/tasks.dart b/super_editor/lib/src/default_editor/tasks.dart index e0183ff8de..df6478bf22 100644 --- a/super_editor/lib/src/default_editor/tasks.dart +++ b/super_editor/lib/src/default_editor/tasks.dart @@ -230,6 +230,19 @@ class _TaskComponentState extends State with ProxyDocumentCompone @override TextComposable get childTextComposable => childDocumentComponentKey.currentState as TextComposable; + /// Computes the [TextStyle] for this task's inner [TextComponent]. + TextStyle _computeStyles(Set attributions) { + // Show a strikethrough across the entire task if it's complete. + final style = widget.viewModel.textStyleBuilder(attributions); + return widget.viewModel.isComplete + ? style.copyWith( + decoration: style.decoration == null + ? TextDecoration.lineThrough + : TextDecoration.combine([TextDecoration.lineThrough, style.decoration!]), + ) + : style; + } + @override Widget build(BuildContext context) { return Row( @@ -248,17 +261,7 @@ class _TaskComponentState extends State with ProxyDocumentCompone child: TextComponent( key: _textKey, text: widget.viewModel.text, - textStyleBuilder: (attributions) { - // Show a strikethrough across the entire task if it's complete. - final style = widget.viewModel.textStyleBuilder(attributions); - return widget.viewModel.isComplete - ? style.copyWith( - decoration: style.decoration == null - ? TextDecoration.lineThrough - : TextDecoration.combine([TextDecoration.lineThrough, style.decoration!]), - ) - : style; - }, + textStyleBuilder: _computeStyles, textSelection: widget.viewModel.selection, selectionColor: widget.viewModel.selectionColor, highlightWhenEmpty: widget.viewModel.highlightWhenEmpty, diff --git a/super_editor/lib/src/default_editor/text.dart b/super_editor/lib/src/default_editor/text.dart index 26cc129908..cefdfe8c54 100644 --- a/super_editor/lib/src/default_editor/text.dart +++ b/super_editor/lib/src/default_editor/text.dart @@ -834,6 +834,28 @@ class TextComponentState extends State with DocumentComponent imp ); } + /// Return the [TextStyle] for the character at [offset]. + /// + /// If the caret sits at the beginning of the text, the style + /// of the first character is returned. + /// + /// If the caret sits at the end of the text, the style + /// of the last character is returned. + /// + /// If the text is empty, the style computed by the widget's `textStyleBuilder` + /// without any attributions is returned. + TextStyle getTextStyleAt(int offset) { + final attributions = widget.text.getAllAttributionsAt(offset < widget.text.length // + ? offset + : widget.text.length - 1); + + return _textStyleWithBlockType(attributions); + } + + TextAlign? get textAlign => widget.textAlign; + + TextDirection? get textDirection => widget.textDirection; + @override Widget build(BuildContext context) { editorLayoutLog.finer('Building a TextComponent with key: ${widget.key}'); 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 5821066161..c4d75f0a1c 100644 --- a/super_editor/lib/src/super_textfield/desktop/desktop_textfield.dart +++ b/super_editor/lib/src/super_textfield/desktop/desktop_textfield.dart @@ -415,6 +415,9 @@ class SuperDesktopTextFieldState extends State implements selectorHandlers: widget.selectorHandlers ?? defaultTextFieldSelectorHandlers, textInputAction: widget.textInputAction, imeConfiguration: widget.imeConfiguration, + textStyleBuilder: widget.textStyleBuilder, + textAlign: widget.textAlign, + textDirection: Directionality.of(context), child: child, ) : child, @@ -1002,6 +1005,9 @@ class SuperTextFieldImeInteractor extends StatefulWidget { required this.selectorHandlers, this.textInputAction, this.imeConfiguration, + required this.textStyleBuilder, + this.textAlign, + this.textDirection, required this.child, }) : super(key: key); @@ -1030,6 +1036,22 @@ class SuperTextFieldImeInteractor extends StatefulWidget { /// Preferences for how the platform IME should look and behave during editing. final TextInputConfiguration? imeConfiguration; + /// Text style factory that creates styles for the content in + /// [textController] based on the attributions in that content. + /// + /// On web, we can't set the position of IME popovers (e.g, emoji picker, + /// character selection panel) ourselves. Because of that, we need + /// to report to the IME what is our text style, so the browser can position + /// the popovers based on text metrics computed for the given style. + /// + /// This should be the same [AttributionStyleBuilder] used to + /// render the text. + final AttributionStyleBuilder textStyleBuilder; + + final TextAlign? textAlign; + + final TextDirection? textDirection; + /// The rest of the subtree for this text field. final Widget child; @@ -1043,7 +1065,7 @@ class _SuperTextFieldImeInteractorState extends State _reportVisualInformationToIme()); } + /// Report the global size and transform of the text field to the IME. + /// + /// This is needed to display the OS emoji & symbols panel at the selected position. + void _reportSizeAndTransformToIme() { + final renderBox = widget.textKey.currentContext?.findRenderObject() as RenderBox?; + if (renderBox == null) { + return; + } + + widget.textController.inputConnectionNotifier.value! + .setEditableSizeAndTransform(renderBox.size, renderBox.getTransformTo(null)); + } + + void _reportCaretRectToIme() { + if (isWeb) { + // On web, setting the caret rect isn't supported. + // To position the IME popovers, we report our size, transform and text style + // and let the browser position the popovers. + return; + } + + final caretRect = _computeCaretRectInContentSpace(); + if (caretRect != null) { + widget.textController.inputConnectionNotifier.value!.setCaretRect(caretRect); + } + } + + /// Report our text style to the IME. + /// + /// This is used on web to set the text style of the hidden native input, + /// to try to match the text size on the browser with our text size. + /// + /// As our content can have multiple styles, the sizes won't be 100% in sync. + void _reportTextStyleToIme() { + late TextStyle textStyle; + + final selection = widget.textController.selection; + if (!selection.isValid) { + return; + } + + // We have a selection, compute the style based on the attributions present + // at the selection extent. + final text = widget.textController.text; + final attributions = text.getAllAttributionsAt(selection.extentOffset); + textStyle = widget.textStyleBuilder(attributions); + + widget.textController.inputConnectionNotifier.value!.setStyle( + fontFamily: textStyle.fontFamily, + fontSize: textStyle.fontSize, + fontWeight: textStyle.fontWeight, + textDirection: widget.textDirection ?? TextDirection.ltr, + textAlign: widget.textAlign ?? TextAlign.left, + ); + } + Rect? _computeCaretRectInContentSpace() { final text = widget.textKey.currentState; if (text == null) { @@ -1791,6 +1875,16 @@ class DefaultSuperTextFieldKeyboardHandlers { if (!arrowKeys.contains(keyEvent.logicalKey)) { return TextFieldKeyboardHandlerResult.notHandled; } + + if (isWeb && (controller.composingRegion.isValid)) { + // We are composing a character on web. It's possible that a native element is being displayed, + // like an emoji picker or a character selection panel. + // We need to let the OS handle the key so the user can navigate + // on the list of possible characters. + // TODO: update this after https://github.com/flutter/flutter/issues/134268 is resolved. + return TextFieldKeyboardHandlerResult.blocked; + } + if (controller.selection.extentOffset == -1) { // The result is reported as "handled" because an arrow // key was pressed, but we return early because there is From 6789b770ea17f1a22948ca124059218cda2075d3 Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Wed, 20 Sep 2023 22:01:33 -0300 Subject: [PATCH 8/9] [SuperEditor][Web] Fix entering spaces (Resolves #1446) (#1455) --- .../infrastructure/platforms/mac/mac_ime.dart | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) 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 5f58e99472..b1a2f2858a 100644 --- a/super_editor/lib/src/infrastructure/platforms/mac/mac_ime.dart +++ b/super_editor/lib/src/infrastructure/platforms/mac/mac_ime.dart @@ -82,4 +82,66 @@ final Map> disabledMacIntents = { // Generated by pressing HOME/END. ScrollToDocumentBoundaryIntent: 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); + } +} From f471c2435083181a893e1d7278c7c6c27ca5dd69 Mon Sep 17 00:00:00 2001 From: Angelo Silvestre Date: Wed, 20 Sep 2023 23:03:19 -0300 Subject: [PATCH 9/9] [ExampleApp] Fix link popover (Resolves #1440) (#1443) --- .../lib/demos/example_editor/_toolbar.dart | 42 ++++++++---- .../lib/demos/in_the_lab/popover_list.dart | 12 ++-- super_editor/lib/src/core/edit_context.dart | 4 -- .../document_keyboard_actions.dart | 15 +---- .../lib/src/default_editor/super_editor.dart | 1 - .../lib/src/infrastructure/popovers.dart | 64 +++++++++++++++++++ super_editor/lib/super_editor.dart | 1 + .../super_editor/text_entry/text_test.dart | 1 - 8 files changed, 102 insertions(+), 38 deletions(-) create mode 100644 super_editor/lib/src/infrastructure/popovers.dart diff --git a/super_editor/example/lib/demos/example_editor/_toolbar.dart b/super_editor/example/lib/demos/example_editor/_toolbar.dart index 85b5aa32c3..d90e6d5e7b 100644 --- a/super_editor/example/lib/demos/example_editor/_toolbar.dart +++ b/super_editor/example/lib/demos/example_editor/_toolbar.dart @@ -66,8 +66,9 @@ class _EditorToolbarState extends State { late FollowerBoundary _screenBoundary; bool _showUrlField = false; + late FocusNode _popoverFocusNode; late FocusNode _urlFocusNode; - AttributedTextEditingController? _urlController; + ImeAttributedTextEditingController? _urlController; @override void initState() { @@ -75,9 +76,13 @@ class _EditorToolbarState extends State { _toolbarAligner = CupertinoPopoverToolbarAligner(widget.editorViewportKey); + _popoverFocusNode = FocusNode(); + _urlFocusNode = FocusNode(); - _urlController = SingleLineAttributedTextEditingController(_applyLink) // - ..text = AttributedText("https://"); + _urlController = + ImeAttributedTextEditingController(controller: SingleLineAttributedTextEditingController(_applyLink)) // + ..onPerformActionPressed = _onPerformAction + ..text = AttributedText("https://"); } @override @@ -94,6 +99,7 @@ class _EditorToolbarState extends State { void dispose() { _urlFocusNode.dispose(); _urlController!.dispose(); + _popoverFocusNode.dispose(); super.dispose(); } @@ -457,6 +463,12 @@ class _EditorToolbarState extends State { } } + void _onPerformAction(TextInputAction action) { + if (action == TextInputAction.done) { + _applyLink(); + } + } + @override Widget build(BuildContext context) { return BuildInOrder( @@ -477,15 +489,19 @@ class _EditorToolbarState extends State { } Widget _buildToolbars() { - return Column( - mainAxisSize: MainAxisSize.min, - children: [ - _buildToolbar(), - if (_showUrlField) ...[ - const SizedBox(height: 8), - _buildUrlField(), + return SuperEditorPopover( + popoverFocusNode: _popoverFocusNode, + editorFocusNode: widget.editorFocusNode, + child: Column( + mainAxisSize: MainAxisSize.min, + children: [ + _buildToolbar(), + if (_showUrlField) ...[ + const SizedBox(height: 8), + _buildUrlField(), + ], ], - ], + ), ); } @@ -619,9 +635,9 @@ class _EditorToolbarState extends State { child: Row( children: [ Expanded( - child: FocusWithCustomParent( + child: Focus( focusNode: _urlFocusNode, - parentFocusNode: widget.editorFocusNode, + 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. diff --git a/super_editor/example/lib/demos/in_the_lab/popover_list.dart b/super_editor/example/lib/demos/in_the_lab/popover_list.dart index 9c75a49a2f..c7e8b445df 100644 --- a/super_editor/example/lib/demos/in_the_lab/popover_list.dart +++ b/super_editor/example/lib/demos/in_the_lab/popover_list.dart @@ -135,12 +135,12 @@ class _PopoverListState extends State { @override Widget build(BuildContext context) { - return GestureDetector( - onTap: () => !_focusNode.hasPrimaryFocus ? _focusNode.requestFocus() : null, - child: Focus( - focusNode: _focusNode, - parentNode: widget.editorFocusNode, - onKeyEvent: _onKeyEvent, + return SuperEditorPopover( + popoverFocusNode: _focusNode, + editorFocusNode: widget.editorFocusNode, + onKeyEvent: _onKeyEvent, + child: GestureDetector( + onTap: () => !_focusNode.hasPrimaryFocus ? _focusNode.requestFocus() : null, child: ListenableBuilder( listenable: _focusNode, builder: (context, child) { diff --git a/super_editor/lib/src/core/edit_context.dart b/super_editor/lib/src/core/edit_context.dart index 05f7c88c14..60f4e9e5d2 100644 --- a/super_editor/lib/src/core/edit_context.dart +++ b/super_editor/lib/src/core/edit_context.dart @@ -25,7 +25,6 @@ class SuperEditorContext { required DocumentLayout Function() getDocumentLayout, required this.composer, required this.scroller, - required this.hasPrimaryFocus, required this.commonOps, }) : _getDocumentLayout = getDocumentLayout; @@ -49,9 +48,6 @@ class SuperEditorContext { /// scrolling. final DocumentScroller scroller; - /// Whether `SuperEditor` currently has primary focus. - final ValueListenable hasPrimaryFocus; - /// Common operations that can be executed to apply common, complex changes to /// the document. final CommonEditorOperations commonOps; diff --git a/super_editor/lib/src/default_editor/document_hardware_keyboard/document_keyboard_actions.dart b/super_editor/lib/src/default_editor/document_hardware_keyboard/document_keyboard_actions.dart index 18951fc760..700348be9b 100644 --- a/super_editor/lib/src/default_editor/document_hardware_keyboard/document_keyboard_actions.dart +++ b/super_editor/lib/src/default_editor/document_hardware_keyboard/document_keyboard_actions.dart @@ -211,17 +211,6 @@ ExecutionInstruction sendKeyEventToMacOs({ // For the full list of selectors handled by SuperEditor, see the MacOsSelectors class. // // This is needed for the interaction with the accent panel to work. - - if (!editContext.hasPrimaryFocus.value) { - // SuperEditor has focus, but not primary focus. This can happen, for example, - // when an app displays a popover that takes primary focus. In this case, because - // SuperEditor no longer has primary focus, Flutter might intercept keys and do - // things we don't want, like move focus around when the user presses arrow keys. - // To prevent Flutter from doing things we don't want, in this case we run the - // standard key handlers instead of sending the signal to the OS. - return ExecutionInstruction.continueExecution; - } - return ExecutionInstruction.blocked; } @@ -517,7 +506,7 @@ ExecutionInstruction moveUpAndDownWithArrowKeys({ return ExecutionInstruction.continueExecution; } - if (isWeb && (editContext.hasPrimaryFocus.value) && (editContext.composer.composingRegion.value != null)) { + if (isWeb && (editContext.composer.composingRegion.value != null)) { // We are composing a character on web. It's possible that a native element is being displayed, // like an emoji picker or a character selection panel. // We need to let the OS handle the key so the user can navigate @@ -578,7 +567,7 @@ ExecutionInstruction moveLeftAndRightWithArrowKeys({ return ExecutionInstruction.continueExecution; } - if (isWeb && (editContext.hasPrimaryFocus.value) && (editContext.composer.composingRegion.value != null)) { + if (isWeb && (editContext.composer.composingRegion.value != null)) { // We are composing a character on web. It's possible that a native element is being displayed, // like an emoji picker or a character selection panel. // We need to let the OS handle the key so the user can navigate diff --git a/super_editor/lib/src/default_editor/super_editor.dart b/super_editor/lib/src/default_editor/super_editor.dart index 56e20f9548..e55f2da2dc 100644 --- a/super_editor/lib/src/default_editor/super_editor.dart +++ b/super_editor/lib/src/default_editor/super_editor.dart @@ -427,7 +427,6 @@ class SuperEditorState extends State { composer: _composer, getDocumentLayout: () => _docLayoutKey.currentState as DocumentLayout, scroller: _scroller, - hasPrimaryFocus: _primaryFocusListener, commonOps: CommonEditorOperations( editor: widget.editor, document: widget.document, diff --git a/super_editor/lib/src/infrastructure/popovers.dart b/super_editor/lib/src/infrastructure/popovers.dart new file mode 100644 index 0000000000..0d59763417 --- /dev/null +++ b/super_editor/lib/src/infrastructure/popovers.dart @@ -0,0 +1,64 @@ +import 'package:flutter/material.dart'; +import 'package:super_editor/src/infrastructure/focus.dart'; +import 'package:super_editor/src/infrastructure/platforms/mac/mac_ime.dart'; + +/// A popover that shares focus with a `SuperEditor`. +/// +/// Popovers often need to handle keyboard input, such as arrow keys for +/// selection movement. But `SuperEditor` also needs to continue handling +/// keyboard input to move the caret, enter text, etc. In such a case, the +/// popover has primary focus, and `SuperEditor` has non-primary focus. +/// Due to the way that Flutter's `Actions` system works, along with +/// Flutter's default response to certain key events, a few careful +/// adjustments need to be made so that a popover works with +/// `SuperEditor` as expected. This widget handles those adjustments. +/// +/// Despite this widget being a "Super Editor popover", this widget can be +/// placed anywhere in the widget tree, so long as it's able to share focus +/// with `SuperEditor`. +/// +/// This widget is purely logical - it doesn't impose any particular layout +/// or constraints. It's up to you whether this widget tightly hugs your +/// popover [child], or whether it expands to fill a space. +/// +/// It's possible to create a `SuperEditor` popover without this widget. +/// This widget doesn't have any special access to `SuperEditor` +/// properties or behavior. But, if you choose to display a popover +/// without using this widget, you'll likely need to re-implement this +/// behavior to avoid unexpected user interaction results. +class SuperEditorPopover extends StatelessWidget { + const SuperEditorPopover({ + super.key, + required this.popoverFocusNode, + required this.editorFocusNode, + this.onKeyEvent, + required this.child, + }); + + /// The [FocusNode] attached to the popover. + final FocusNode popoverFocusNode; + + /// The [FocusNode] attached to the editor. + /// + /// The [popoverFocusNode] will be reparented with this [FocusNode]. + final FocusNode editorFocusNode; + + /// Callback that notifies key events. + final FocusOnKeyEventCallback? onKeyEvent; + + /// The popover to display. + final Widget child; + + @override + Widget build(BuildContext context) { + return Actions( + actions: disabledMacIntents, + child: FocusWithCustomParent( + focusNode: popoverFocusNode, + parentFocusNode: editorFocusNode, + onKeyEvent: onKeyEvent, + child: child, + ), + ); + } +} diff --git a/super_editor/lib/super_editor.dart b/super_editor/lib/super_editor.dart index e835bb30f5..c50d5d8e03 100644 --- a/super_editor/lib/super_editor.dart +++ b/super_editor/lib/super_editor.dart @@ -78,6 +78,7 @@ export 'src/super_textfield/super_textfield.dart'; export 'src/infrastructure/touch_controls.dart'; export 'src/infrastructure/text_input.dart'; export 'src/infrastructure/viewport_size_reporting.dart'; +export 'src/infrastructure/popovers.dart'; // Super Reader export 'src/super_reader/read_only_document_android_touch_interactor.dart'; diff --git a/super_editor/test/super_editor/text_entry/text_test.dart b/super_editor/test/super_editor/text_entry/text_test.dart index 69a2bf24cd..fac3e18486 100644 --- a/super_editor/test/super_editor/text_entry/text_test.dart +++ b/super_editor/test/super_editor/text_entry/text_test.dart @@ -418,7 +418,6 @@ SuperEditorContext _createEditContext() { getDocumentLayout: () => fakeLayout, composer: composer, scroller: FakeSuperEditorScroller(), - hasPrimaryFocus: ValueNotifier(false), commonOps: CommonEditorOperations( editor: documentEditor, document: document,