diff --git a/super_editor/example/lib/demos/interaction_spot_checks/toolbar_following_content_in_layer.dart b/super_editor/example/lib/demos/interaction_spot_checks/toolbar_following_content_in_layer.dart index 8387e76970..8e556bd3b2 100644 --- a/super_editor/example/lib/demos/interaction_spot_checks/toolbar_following_content_in_layer.dart +++ b/super_editor/example/lib/demos/interaction_spot_checks/toolbar_following_content_in_layer.dart @@ -29,7 +29,10 @@ class _ToolbarFollowingContentInLayerState extends State { - final _messagePageController = MessagePageController(); - @override void initState() { super.initState(); - SuperKeyboard.startLogging(); + // SuperKeyboard.startLogging(); } @override void dispose() { - SuperKeyboard.stopLogging(); + // SuperKeyboard.stopLogging(); super.dispose(); } + @override + Widget build(BuildContext context) { + return _ChatPage( + inputRole: "Home", + ); + } +} + +class _ChatPage extends StatefulWidget { + const _ChatPage({ + required this.inputRole, + }); + + final String inputRole; + + @override + State<_ChatPage> createState() => _ChatPageState(); +} + +class _ChatPageState extends State<_ChatPage> { + final _messagePageController = MessagePageController(); + + @override + void dispose() { + _messagePageController.dispose(); + super.dispose(); + } + @override Widget build(BuildContext context) { return MessagePageScaffold( @@ -61,6 +89,7 @@ class _SuperEditorMessagePageDemoState extends State bottomSheetBuilder: (messageContext) { return _EditorBottomSheet( messagePageController: _messagePageController, + inputRole: widget.inputRole, ); }, ); @@ -97,6 +126,20 @@ class _ChatThread extends StatelessWidget { color: Colors.white.withValues(alpha: 0.5), child: ListTile( title: Text("Item $index"), + onTap: () { + Navigator.of(context).push( + MaterialPageRoute( + builder: (context) { + return Scaffold( + resizeToAvoidBottomInset: false, + body: _ChatPage( + inputRole: "Subpage-${Random().nextInt(1000)}", + ), + ); + }, + ), + ); + }, ), ), ); @@ -109,9 +152,11 @@ class _ChatThread extends StatelessWidget { class _EditorBottomSheet extends StatefulWidget { const _EditorBottomSheet({ required this.messagePageController, + required this.inputRole, }); final MessagePageController messagePageController; + final String inputRole; @override State<_EditorBottomSheet> createState() => _EditorBottomSheetState(); @@ -142,22 +187,22 @@ class _EditorBottomSheetState extends State<_EditorBottomSheet> { id: Editor.createNodeId(), text: AttributedText("message"), ), - ParagraphNode( - id: Editor.createNodeId(), - text: AttributedText("It's tall for quick"), - ), - ParagraphNode( - id: Editor.createNodeId(), - text: AttributedText("testing of"), - ), - ParagraphNode( - id: Editor.createNodeId(), - text: AttributedText("intrinsic height that"), - ), - ParagraphNode( - id: Editor.createNodeId(), - text: AttributedText("exceeds available space"), - ), + // ParagraphNode( + // id: Editor.createNodeId(), + // text: AttributedText("It's tall for quick"), + // ), + // ParagraphNode( + // id: Editor.createNodeId(), + // text: AttributedText("testing of"), + // ), + // ParagraphNode( + // id: Editor.createNodeId(), + // text: AttributedText("intrinsic height that"), + // ), + // ParagraphNode( + // id: Editor.createNodeId(), + // text: AttributedText("exceeds available space"), + // ), ], ), composer: MutableDocumentComposer(), @@ -256,6 +301,7 @@ class _EditorBottomSheetState extends State<_EditorBottomSheet> { child: _ChatEditor( key: _editorKey, editor: _editor, + inputRole: widget.inputRole, messagePageController: widget.messagePageController, scrollController: _scrollController, ), @@ -302,11 +348,13 @@ class _ChatEditor extends StatefulWidget { const _ChatEditor({ super.key, required this.editor, + required this.inputRole, required this.messagePageController, required this.scrollController, }); final Editor editor; + final String inputRole; final MessagePageController messagePageController; final ScrollController scrollController; @@ -373,7 +421,7 @@ class _ChatEditorState extends State<_ChatEditor> { // focus so that our app state synchronizes with the closed IME connection. final keyboardState = SuperKeyboard.instance.mobileGeometry.value.keyboardState; if (_isImeConnected.value && (keyboardState == KeyboardState.closing || keyboardState == KeyboardState.closed)) { - _editorFocusNode.unfocus(); + // _editorFocusNode.unfocus(); } } @@ -394,6 +442,48 @@ class _ChatEditorState extends State<_ChatEditor> { return KeyboardPanelScaffold( controller: _keyboardPanelController, isImeConnected: _isImeConnected, + contentBuilder: (BuildContext context, _Panel? openPanel) { + return Row( + crossAxisAlignment: CrossAxisAlignment.center, + children: [ + ListenableBuilder( + listenable: _editorFocusNode, + builder: (context, child) { + if (_editorFocusNode.hasFocus) { + return const SizedBox(); + } + + return child!; + }, + child: IconButton( + onPressed: () { + _editorFocusNode.requestFocus(); + + WidgetsBinding.instance.addPostFrameCallback((_) { + // We wait for the end of the frame to show the panel because giving + // focus to the editor will first cause the keyboard to show. If we + // opened the panel immediately then it would be covered by the keyboard. + _keyboardPanelController.showKeyboardPanel(_Panel.thePanel); + }); + }, + icon: Icon(Icons.add), + ), + ), + Expanded(child: _buildEditor()), + ListenableBuilder( + listenable: _editorFocusNode, + builder: (context, child) { + if (_editorFocusNode.hasFocus) { + return const SizedBox(); + } + + return child!; + }, + child: IconButton(onPressed: () {}, icon: Icon(Icons.multitrack_audio)), + ), + ], + ); + }, toolbarBuilder: (BuildContext context, _Panel? openPanel) { return Container( width: double.infinity, @@ -402,6 +492,18 @@ class _ChatEditorState extends State<_ChatEditor> { padding: const EdgeInsets.symmetric(horizontal: 16), child: Row( children: [ + GestureDetector( + onTap: () { + if (!_keyboardPanelController.isKeyboardPanelOpen) { + _keyboardPanelController.showKeyboardPanel(_Panel.thePanel); + } else { + // This line is here to debug an issue in ClickUp + _keyboardPanelController.hideKeyboardPanel(); + _keyboardPanelController.showSoftwareKeyboard(); + } + }, + child: Icon(Icons.add), + ), Spacer(), GestureDetector( onTap: () { @@ -414,32 +516,38 @@ class _ChatEditorState extends State<_ChatEditor> { ); }, keyboardPanelBuilder: (BuildContext context, _Panel? openPanel) { - return SizedBox(); + if (openPanel == null) { + return SizedBox(); + } + + return Container(width: double.infinity, height: 300, color: Colors.red); }, - contentBuilder: (BuildContext context, _Panel? openPanel) { - return SuperEditorFocusOnTap( - editorFocusNode: _editorFocusNode, + ); + } + + Widget _buildEditor() { + return SuperEditorFocusOnTap( + editorFocusNode: _editorFocusNode, + editor: widget.editor, + child: SuperEditorDryLayout( + controller: widget.scrollController, + superEditor: SuperEditor( + key: _editorKey, + focusNode: _editorFocusNode, editor: widget.editor, - child: SuperEditorDryLayout( - controller: widget.scrollController, - superEditor: SuperEditor( - key: _editorKey, - focusNode: _editorFocusNode, - editor: widget.editor, - softwareKeyboardController: _softwareKeyboardController, - isImeConnected: _isImeConnected, - imePolicies: SuperEditorImePolicies(), - selectionPolicies: SuperEditorSelectionPolicies(), - shrinkWrap: false, - stylesheet: _chatStylesheet, - componentBuilders: [ - const HintComponentBuilder("Send a message...", _hintTextStyleBuilder), - ...defaultComponentBuilders, - ], - ), - ), - ); - }, + inputRole: widget.inputRole, + softwareKeyboardController: _softwareKeyboardController, + isImeConnected: _isImeConnected, + imePolicies: SuperEditorImePolicies(), + selectionPolicies: SuperEditorSelectionPolicies(), + shrinkWrap: false, + stylesheet: _chatStylesheet, + componentBuilders: [ + const HintComponentBuilder("Send a message...", _hintTextStyleBuilder), + ...defaultComponentBuilders, + ], + ), + ), ); } } diff --git a/super_editor/lib/src/chat/message_page_scaffold.dart b/super_editor/lib/src/chat/message_page_scaffold.dart index 08c49fa9ea..1b3ba11680 100644 --- a/super_editor/lib/src/chat/message_page_scaffold.dart +++ b/super_editor/lib/src/chat/message_page_scaffold.dart @@ -25,6 +25,7 @@ class MessagePageScaffold extends RenderObjectWidget { required this.bottomSheetBuilder, this.bottomSheetMinimumTopGap = 200, this.bottomSheetMinimumHeight = 150, + this.bottomSheetCollapsedMaximumHeight = double.infinity, }); final MessagePageController? controller; @@ -47,6 +48,18 @@ class MessagePageScaffold extends RenderObjectWidget { /// height mode. final double bottomSheetMinimumHeight; + /// The maximum height that the bottom sheet can expand to, as the intrinsic height + /// of the content increases. + /// + /// E.g., The user starts with a single line of text and then starts inserting + /// newlines. As the user continues to add newlines, this height is where the sheet + /// stops growing taller. + /// + /// This height applies when the sheet is collapsed, i.e., not expanded. If the user + /// expands the sheet, then the maximum height of the sheet would be the maximum allowed + /// layout height, minus [bottomSheetMinimumTopGap]. + final double bottomSheetCollapsedMaximumHeight; + @override RenderObjectElement createElement() { return MessagePageElement(this); @@ -59,6 +72,7 @@ class MessagePageScaffold extends RenderObjectWidget { controller, bottomSheetMinimumTopGap: bottomSheetMinimumTopGap, bottomSheetMinimumHeight: bottomSheetMinimumHeight, + bottomSheetCollapsedMaximumHeight: bottomSheetCollapsedMaximumHeight, ); } @@ -66,7 +80,8 @@ class MessagePageScaffold extends RenderObjectWidget { void updateRenderObject(BuildContext context, RenderMessagePageScaffold renderObject) { renderObject ..bottomSheetMinimumTopGap = bottomSheetMinimumTopGap - ..bottomSheetMinimumHeight = bottomSheetMinimumHeight; + ..bottomSheetMinimumHeight = bottomSheetMinimumHeight + ..bottomSheetCollapsedMaximumHeight = bottomSheetCollapsedMaximumHeight; if (controller != null) { renderObject.controller = controller!; @@ -310,7 +325,7 @@ class MessagePageElement extends RenderObjectElement { @override void mount(Element? parent, Object? newSlot) { - messagePageElementLog.info('ChatScaffoldElement - mounting'); + messagePageElementLog.info('MessagePageElement - mounting'); super.mount(parent, newSlot); _content = inflateWidget( @@ -326,7 +341,7 @@ class MessagePageElement extends RenderObjectElement { @override void activate() { - messagePageElementLog.info('ContentLayersElement - activating'); + messagePageElementLog.info('MessagePageElement - activating'); _didActivateSinceLastBuild = false; super.activate(); } @@ -336,7 +351,7 @@ class MessagePageElement extends RenderObjectElement { @override void deactivate() { - messagePageElementLog.info('ContentLayersElement - deactivating'); + messagePageElementLog.info('MessagePageElement - deactivating'); _didDeactivateSinceLastBuild = false; super.deactivate(); } @@ -346,7 +361,7 @@ class MessagePageElement extends RenderObjectElement { @override void unmount() { - messagePageElementLog.info('ContentLayersElement - unmounting'); + messagePageElementLog.info('MessagePageElement - unmounting'); super.unmount(); } @@ -380,7 +395,7 @@ class MessagePageElement extends RenderObjectElement { } void buildContent(double bottomSpacing) { - messagePageElementLog.info('ContentLayersElement ($hashCode) - (re)building layers'); + messagePageElementLog.info('MessagePageScaffold ($hashCode) - (re)building content'); widget.controller?.debugMostRecentBottomSpacing.value = bottomSpacing; owner!.buildScope(this, () { @@ -430,6 +445,11 @@ class MessagePageElement extends RenderObjectElement { @override void insertRenderObjectChild(RenderObject child, Object? slot) { + assert( + _isChatScaffoldSlot(slot!), + 'Invalid ChatScaffold child slot: $slot', + ); + renderObject.insertChild(child, slot!); } @@ -573,8 +593,10 @@ class RenderMessagePageScaffold extends RenderBox { MessagePageController? controller, { required double bottomSheetMinimumTopGap, required double bottomSheetMinimumHeight, + required double bottomSheetCollapsedMaximumHeight, }) : _bottomSheetMinimumTopGap = bottomSheetMinimumTopGap, - _bottomSheetMinimumHeight = bottomSheetMinimumHeight { + _bottomSheetMinimumHeight = bottomSheetMinimumHeight, + _bottomSheetCollapsedMaximumHeight = bottomSheetCollapsedMaximumHeight { _controller = controller ?? MessagePageController(); _attachToController(); } @@ -701,7 +723,6 @@ class RenderMessagePageScaffold extends RenderBox { } void _onDragEnd() { - _isExpandingOrCollapsing = true; _velocityStopwatch.stop(); final velocity = _velocityTracker.getVelocityEstimate()?.pixelsPerSecond.dy ?? 0; @@ -716,7 +737,7 @@ class RenderMessagePageScaffold extends RenderBox { final minimizedHeight = switch (_controller.collapsedMode) { MessagePageSheetCollapsedMode.preview => _previewHeight, - MessagePageSheetCollapsedMode.intrinsic => _intrinsicHeight, + MessagePageSheetCollapsedMode.intrinsic => min(_intrinsicHeight, _bottomSheetCollapsedMaximumHeight), }; _controller.desiredSheetMode = velocity.abs() > 500 // @@ -740,18 +761,35 @@ class RenderMessagePageScaffold extends RenderBox { void _updateBottomSheetHeightSimulation({ required double velocity, }) { - _ticker.stop(); - final minimizedHeight = switch (_controller.collapsedMode) { MessagePageSheetCollapsedMode.preview => _previewHeight, - MessagePageSheetCollapsedMode.intrinsic => _intrinsicHeight, + MessagePageSheetCollapsedMode.intrinsic => min(_intrinsicHeight, _bottomSheetCollapsedMaximumHeight), }; _controller.isSliding = true; final startHeight = _bottomSheet!.size.height; _simulationGoalMode = _controller.desiredSheetMode; - _simulationGoalHeight = _simulationGoalMode! == MessagePageSheetMode.expanded ? _expandedHeight : minimizedHeight; + final newSimulationGoalHeight = + _simulationGoalMode! == MessagePageSheetMode.expanded ? _expandedHeight : minimizedHeight; + if ((newSimulationGoalHeight - startHeight).abs() < 1) { + // We're already at the destination. Fizzle. + _animatedHeight = newSimulationGoalHeight; + _animatedVelocity = 0; + _isExpandingOrCollapsing = false; + _desiredDragHeight = null; + _ticker.stop(); + return; + } + if (newSimulationGoalHeight == _simulationGoalHeight) { + // We're already simulating to this height. We short-circuit when the goal + // hasn't changed so that we don't get rapidly oscillating simulation artifacts. + return; + } + _simulationGoalHeight = newSimulationGoalHeight; + _isExpandingOrCollapsing = true; + + _ticker.stop(); messagePageLayoutLog.info('Creating expand/collapse simulation:'); messagePageLayoutLog.info( @@ -772,7 +810,10 @@ class RenderMessagePageScaffold extends RenderBox { ), startHeight, // Start value _simulationGoalHeight!, // End value - velocity, // Initial velocity + // Invert velocity because we measured velocity moving down the screen, but we + // want to apply velocity to the height of the sheet. A positive screen velocity + // corresponds to a negative sheet height velocity. + -velocity, // Initial velocity. ); _ticker.start(); @@ -822,8 +863,33 @@ class RenderMessagePageScaffold extends RenderBox { } double _bottomSheetMinimumHeight; + + set bottomSheetMaximumHeight(double newValue) { + if (newValue == _bottomSheetMaximumHeight) { + return; + } + + _bottomSheetMaximumHeight = newValue; + + // FIXME: Only invalidate layout if this change impacts the current rendering. + markNeedsLayout(); + } + double _bottomSheetMaximumHeight = double.infinity; + set bottomSheetCollapsedMaximumHeight(double newValue) { + if (newValue == _bottomSheetCollapsedMaximumHeight) { + return; + } + + _bottomSheetCollapsedMaximumHeight = newValue; + + // FIXME: Only invalidate layout if this change impacts the current rendering. + markNeedsLayout(); + } + + double _bottomSheetCollapsedMaximumHeight = double.infinity; + /// Whether this render object's layout information or its content /// layout information is dirty. /// @@ -848,7 +914,7 @@ class RenderMessagePageScaffold extends RenderBox { void _onExpandCollapseTick(Duration elapsedTime) { final seconds = elapsedTime.inMilliseconds / 1000; - _animatedHeight = _simulation!.x(seconds); + _animatedHeight = _simulation!.x(seconds).clamp(_bottomSheetMinimumHeight, _bottomSheetMaximumHeight); _animatedVelocity = _simulation!.dx(seconds); if (_simulation!.isDone(seconds)) { @@ -985,8 +1051,7 @@ class RenderMessagePageScaffold extends RenderBox { messagePageLayoutLog.info( "Measuring the bottom sheet's intrinsic height", ); - // Do a throw-away layout pass to get the intrinsic height of the bottom - // sheet, bounded within its min/max height. + // Do a throw-away layout pass to get the intrinsic height of the bottom sheet. _intrinsicHeight = _calculateBoundedIntrinsicHeight( constraints.copyWith(minHeight: 0), ); @@ -1001,6 +1066,7 @@ class RenderMessagePageScaffold extends RenderBox { MessagePageSheetCollapsedMode.intrinsic => _intrinsicHeight, }; + // Max height depends on whether we're collapsed or expanded. final bottomSheetConstraints = constraints.copyWith( minHeight: minimizedHeight, maxHeight: _bottomSheetMaximumHeight, @@ -1026,11 +1092,16 @@ class RenderMessagePageScaffold extends RenderBox { _updateBottomSheetHeightSimulation(velocity: _animatedVelocity); } + final minimumHeight = min( + _controller.collapsedMode == MessagePageSheetCollapsedMode.preview ? _previewHeight : _intrinsicHeight, + _bottomSheetCollapsedMaximumHeight); + final animatedHeight = _animatedHeight.clamp(minimumHeight, _bottomSheetMaximumHeight); + _bottomSheet!.layout( bottomSheetConstraints.copyWith( - minHeight: max(_animatedHeight - 1, 0), + minHeight: max(animatedHeight - 1, 0), // ^ prevent a layout boundary - maxHeight: _animatedHeight, + maxHeight: animatedHeight, ), parentUsesSize: true, ); @@ -1039,7 +1110,10 @@ class RenderMessagePageScaffold extends RenderBox { messagePageLayoutLog.info( ' - drag height: $_desiredDragHeight, minimized height: $minimizedHeight', ); - final strictHeight = _desiredDragHeight!.clamp(minimizedHeight, _bottomSheetMaximumHeight); + + final minimumHeight = min(minimizedHeight, _bottomSheetCollapsedMaximumHeight); + + final strictHeight = _desiredDragHeight!.clamp(minimumHeight, _bottomSheetMaximumHeight); messagePageLayoutLog.info(' - bounded drag height: $strictHeight'); _bottomSheet!.layout( @@ -1068,7 +1142,10 @@ class RenderMessagePageScaffold extends RenderBox { messagePageLayoutLog.info('>>>>>>>> Minimized'); messagePageLayoutLog.info('Running standard editor layout with constraints: $bottomSheetConstraints'); _bottomSheet!.layout( - bottomSheetConstraints, + bottomSheetConstraints.copyWith( + minHeight: 0, + maxHeight: _bottomSheetCollapsedMaximumHeight, + ), parentUsesSize: true, ); } @@ -1266,7 +1343,13 @@ class RenderMessageEditorHeight extends RenderBox // // If we find a missing layout invalidation for MessagePageScaffold, and we // make this call superfluous, then remove this. - _findAncestorMessagePageScaffold()!.markNeedsLayout(); + final ancestorMessagePageScaffold = _findAncestorMessagePageScaffold(); + // Ancestor scaffold might be null during various lifecycle events, e.g., + // `dropChild()` calls `markNeedsLayout()`, but when we're dropping our + // children, we have likely already been dropped by our parent, too. + if (ancestorMessagePageScaffold != null) { + ancestorMessagePageScaffold.markNeedsLayout(); + } } @override diff --git a/super_editor/lib/src/core/editor.dart b/super_editor/lib/src/core/editor.dart index 1bd69d3df0..68c7c7bccf 100644 --- a/super_editor/lib/src/core/editor.dart +++ b/super_editor/lib/src/core/editor.dart @@ -1,16 +1,15 @@ -import 'dart:math'; +import 'dart:math' hide log; import 'package:attributed_text/attributed_text.dart'; import 'package:clock/clock.dart'; import 'package:collection/collection.dart'; +import 'package:super_editor/src/core/document.dart'; +import 'package:super_editor/src/core/document_composer.dart'; import 'package:super_editor/src/default_editor/paragraph.dart'; import 'package:super_editor/src/default_editor/text.dart'; import 'package:super_editor/src/infrastructure/_logging.dart'; import 'package:uuid/uuid.dart'; -import 'document.dart'; -import 'document_composer.dart'; - /// Editor for a document editing experience. /// /// An [Editor] is the entry point for all mutations within a document editing experience. @@ -188,6 +187,11 @@ class Editor implements RequestDispatcher { return; } + assert( + _activeChangeList != null, + "Tried to end a transaction but the active change list is null. It should never be null during a transaction.", + ); + if (_transaction!.commands.isNotEmpty && isHistoryEnabled) { if (_history.isEmpty) { // Add this transaction onto the history stack. @@ -224,11 +228,18 @@ class Editor implements RequestDispatcher { _isInTransaction = false; _isImplicitTransaction = false; _transaction = null; + _activeCommandCount = 0; + + // Note: We need to pass the changes to the `Editable`s when we report the end of + // a transaction, but we also need to null out the active change list before we + // do that, because the transaction is officially over. We hold on to the active + // list locally, and then null out the shared active list. + final changeList = _activeChangeList!; + _activeChangeList = null; - // Note: The transaction isn't fully considered over until after the reactions run. - // This is because the reactions need access to the change list from the previous - // transaction. - _onTransactionEnd(); + for (final editable in context._resources.values) { + editable.onTransactionEnd(changeList); + } editorEditsLog.info("Finished transaction"); } @@ -281,9 +292,9 @@ class Editor implements RequestDispatcher { if (_activeCommandCount == 1 && _isImplicitTransaction && !_isReacting) { endTransaction(); + } else { + _activeCommandCount -= 1; } - - _activeCommandCount -= 1; } EditCommand _findCommandForRequest(EditRequest request) { @@ -326,14 +337,6 @@ class Editor implements RequestDispatcher { } } - void _onTransactionEnd() { - for (final editable in context._resources.values) { - editable.onTransactionEnd(_activeChangeList!); - } - - _activeChangeList = null; - } - void _reactToChanges() { if (_activeChangeList!.isEmpty) { return; diff --git a/super_editor/lib/src/default_editor/default_document_editor.dart b/super_editor/lib/src/default_editor/default_document_editor.dart index 8d247fb242..3f60fdced8 100644 --- a/super_editor/lib/src/default_editor/default_document_editor.dart +++ b/super_editor/lib/src/default_editor/default_document_editor.dart @@ -15,15 +15,15 @@ import 'common_editor_operations.dart'; import 'default_document_editor_reactions.dart'; Editor createDefaultDocumentEditor({ - required MutableDocument document, - required MutableDocumentComposer composer, + MutableDocument? document, + MutableDocumentComposer? composer, HistoryGroupingPolicy historyGroupingPolicy = defaultMergePolicy, bool isHistoryEnabled = false, }) { final editor = Editor( editables: { - Editor.documentKey: document, - Editor.composerKey: composer, + Editor.documentKey: document ?? MutableDocument.empty(), + Editor.composerKey: composer ?? MutableDocumentComposer(), }, requestHandlers: List.from(defaultRequestHandlers), historyGroupingPolicy: historyGroupingPolicy, 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 c0f84867d8..5b9d98e2e7 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 @@ -1402,8 +1402,9 @@ class SuperEditorAndroidControlsOverlayManagerState extends State _onHandlePanStart(details, HandleType.collapsed), @@ -1428,6 +1429,12 @@ class SuperEditorAndroidControlsOverlayManagerState extends State _onHandlePanEnd(details, HandleType.downstream), onPanCancel: () => _onHandlePanCancel(HandleType.downstream), ); + + onNextFrame((_) { + // Call `show()` at the end of the frame because calling during a build + // process blows up. + _overlayController.show(); + }); } @override @@ -1437,7 +1444,6 @@ class SuperEditorAndroidControlsOverlayManagerState extends State _currentTextEditingValue; + TextEditingValue _currentTextEditingValue = const TextEditingValue(); // What the platform IME *thinks* the current value is. diff --git a/super_editor/lib/src/default_editor/document_ime/document_ime_interaction_policies.dart b/super_editor/lib/src/default_editor/document_ime/document_ime_interaction_policies.dart index b8153a8d16..07d6f587b3 100644 --- a/super_editor/lib/src/default_editor/document_ime/document_ime_interaction_policies.dart +++ b/super_editor/lib/src/default_editor/document_ime/document_ime_interaction_policies.dart @@ -4,6 +4,7 @@ import 'package:flutter/widgets.dart'; import 'package:super_editor/src/core/document_composer.dart'; import 'package:super_editor/src/core/document_selection.dart'; import 'package:super_editor/src/core/editor.dart'; +import 'package:super_editor/src/default_editor/document_ime/shared_ime.dart'; import 'package:super_editor/src/infrastructure/_logging.dart'; import 'package:super_editor/src/infrastructure/flutter/flutter_scheduler.dart'; @@ -13,7 +14,7 @@ class ImeFocusPolicy extends StatefulWidget { const ImeFocusPolicy({ Key? key, this.focusNode, - required this.imeConnection, + required this.inputId, required this.imeClientFactory, required this.imeConfiguration, this.openImeOnPrimaryFocusGain = true, @@ -27,8 +28,12 @@ class ImeFocusPolicy extends StatefulWidget { /// on this widget's [closeImeOnPrimaryFocusLost] policy. final FocusNode? focusNode; - /// The connection between this app and the platform Input Method Engine (IME). - final ValueNotifier imeConnection; + /// The input ID of the widget that owns this [ImeFocusPolicy]. + /// + /// For example, this [ImeFocusPolicy] might be inside of a chat message + /// editor - this [inputId] would uniquely identify the chat message editor + /// vs any other input in the widget tree. It's used to manage IME ownership. + final SuperImeInputId inputId; /// Factory method that creates a [TextInputClient], which is used to /// attach to the platform IME based on this widget's policy. @@ -72,6 +77,13 @@ class _ImeFocusPolicyState extends State { void initState() { super.initState(); _focusNode = (widget.focusNode ?? FocusNode())..addListener(_onFocusChange); + + // Sync the keyboard with initial focus status. Do this at the end of the + // frame just to make sure that any downstream code that runs when we open/close + // the IME doesn't blow up by calling setState() during the build process. + WidgetsBinding.instance.addPostFrameCallback((_) { + _onFocusChange(); + }); } @override @@ -93,47 +105,68 @@ class _ImeFocusPolicyState extends State { } void _onFocusChange() { + print("IME interaction policies - _onFocusChange()"); + if (_focusNode.hasFocus && !SuperIme.instance.isOwner(widget.inputId)) { + // We have focus but we don't own the IME. Take it over. + print("Taking ownership of ime on focus: ${widget.inputId}"); + SuperIme.instance.takeOwnership(widget.inputId); + print("Now that we have ownership, here's the current state of things:"); + print(" - has primary focus? ${_focusNode.hasPrimaryFocus}"); + print(" - open on primary gain? ${widget.openImeOnPrimaryFocusGain}"); + print(" - open on non-primary gain? ${widget.openImeOnNonPrimaryFocusGain}"); + print(" - are we already attached? ${SuperIme.instance.isInputAttachedToOS(widget.inputId)}"); + } + bool shouldOpenIme = false; if (_focusNode.hasPrimaryFocus && widget.openImeOnPrimaryFocusGain && - (widget.imeConnection.value == null || !widget.imeConnection.value!.attached)) { + !SuperIme.instance.isInputAttachedToOS(widget.inputId)) { editorPoliciesLog .info("[${widget.runtimeType}] - Document editor gained primary focus. Opening an IME connection."); shouldOpenIme = true; } else if (!_focusNode.hasPrimaryFocus && _focusNode.hasFocus && widget.openImeOnNonPrimaryFocusGain && - (widget.imeConnection.value == null || !widget.imeConnection.value!.attached)) { + !SuperIme.instance.isInputAttachedToOS(widget.inputId)) { editorPoliciesLog .info("[${widget.runtimeType}] - Document editor gained non-primary focus. Opening an IME connection."); shouldOpenIme = true; } + if (shouldOpenIme) { + print("Opening the IME now or on next frame, due to focus change"); WidgetsBinding.instance.runAsSoonAsPossible(() { if (!mounted) { return; } editorImeLog.finer("[${widget.runtimeType}] - creating new TextInputConnection to IME"); - widget.imeConnection.value = TextInput.attach( + SuperIme.instance.openConnection( + widget.inputId, widget.imeClientFactory(), widget.imeConfiguration, - )..show(); + showKeyboard: true, + ); }, debugLabel: 'Open IME Connection on Primary Focus Change'); } bool shouldCloseIme = false; - if (!_focusNode.hasPrimaryFocus && widget.closeImeOnPrimaryFocusLost) { + if (!_focusNode.hasPrimaryFocus && widget.closeImeOnPrimaryFocusLost && SuperIme.instance.isOwner(widget.inputId)) { editorPoliciesLog .info("[${widget.runtimeType}] - Document editor lost primary focus. Closing the IME connection."); shouldCloseIme = true; - } else if (!_focusNode.hasFocus && widget.closeImeOnNonPrimaryFocusLost) { + } else if (!_focusNode.hasFocus && + widget.closeImeOnNonPrimaryFocusLost && + SuperIme.instance.isOwner(widget.inputId)) { editorPoliciesLog.info("[${widget.runtimeType}] - Document editor lost all focus. Closing the IME connection."); shouldCloseIme = true; } + if (shouldCloseIme) { - widget.imeConnection.value?.close(); - widget.imeConnection.value = null; + print("Closing IME due to no focus"); + SuperIme.instance + ..clearConnection(widget.inputId) + ..releaseOwnership(widget.inputId); } } @@ -157,7 +190,7 @@ class DocumentSelectionOpenAndCloseImePolicy extends StatefulWidget { this.isEnabled = true, required this.editor, required this.selection, - required this.imeConnection, + required this.inputId, required this.imeClientFactory, required this.imeConfiguration, this.openKeyboardOnSelectionChange = true, @@ -187,8 +220,7 @@ class DocumentSelectionOpenAndCloseImePolicy extends StatefulWidget { /// The document editor's current selection. final ValueListenable selection; - /// The current connection from this app to the platform IME. - final ValueNotifier imeConnection; + final SuperImeInputId inputId; /// Factory method that creates a [TextInputClient], which is used to /// attach to the platform IME based on this widget's selection policy. @@ -251,8 +283,8 @@ class _DocumentSelectionOpenAndCloseImePolicyState extends State imeConnection; + final SuperImeInputId inputId; final TextInputClient Function() createImeClient; @@ -58,33 +59,70 @@ class _SoftwareKeyboardOpenerState extends State impleme // their `dispose()` methods. If we `detach()` right now, the // ancestor widgets would cause errors in their `dispose()` methods. WidgetsBinding.instance.scheduleFrameCallback((timeStamp) { - widget.controller?.detach(); + // Check that we're still the delegate at the end of the frame, because + // some other widget may have replaced us as the delegate. + if (widget.controller?._delegate == this) { + widget.controller?.detach(); + } }); super.dispose(); } + bool get _ownsIme => SuperIme.instance.isOwner(widget.inputId); + @override - bool get isConnectedToIme => widget.imeConnection.value?.attached ?? false; + bool get isConnectedToIme => SuperIme.instance.isInputAttachedToOS(widget.inputId); @override void open({ required int viewId, }) { - editorImeLog.info("[SoftwareKeyboard] - showing keyboard"); - widget.imeConnection.value ??= TextInput.attach(widget.createImeClient(), widget.createImeConfiguration()); - widget.imeConnection.value!.show(); + // Wait until end of frame to try to open the keyboard so that all IME ownership + // changes have time to finish, and we can check if we're the final owner. + WidgetsBinding.instance.addPostFrameCallback((_) { + print("SoftwareKeyboardOpener - open()"); + if (!_ownsIme) { + print("Can't open because we don't have IME ownership"); + editorImeLog.info("[SoftwareKeyboard] - tried to show keyboard, but we don't own IME (${widget.inputId})"); + return; + } + + print("Opening IME connection and showing keyboard"); + editorImeLog.info("[SoftwareKeyboard] - showing keyboard"); + SuperIme.instance.openConnection( + widget.inputId, + widget.createImeClient(), + widget.createImeConfiguration(), + showKeyboard: true, + ); + }); } @override void hide() { - SystemChannels.textInput.invokeListMethod("TextInput.hide"); + // Wait until end of frame to try to hide the keyboard so that all IME ownership + // changes have time to finish, and we can check if we're the final owner. + WidgetsBinding.instance.addPostFrameCallback((_) { + print("SoftwareKeyboardOpener - hide()"); + if (!_ownsIme) { + print("Can't hide because we don't own the IME"); + editorImeLog.info("[SoftwareKeyboard] - tried to hide keyboard, but we don't own IME (${widget.inputId})"); + return; + } + + SystemChannels.textInput.invokeListMethod("TextInput.hide"); + }); } @override void close() { + if (!_ownsIme) { + editorImeLog.info("[SoftwareKeyboard] - tried to close keyboard, but we don't own IME (${widget.inputId})"); + return; + } + editorImeLog.info("[SoftwareKeyboard] - closing IME connection."); - widget.imeConnection.value?.close(); - widget.imeConnection.value = null; + SuperIme.instance.clearConnection(widget.inputId); } @override diff --git a/super_editor/lib/src/default_editor/document_ime/shared_ime.dart b/super_editor/lib/src/default_editor/document_ime/shared_ime.dart new file mode 100644 index 0000000000..8f11b3bd1f --- /dev/null +++ b/super_editor/lib/src/default_editor/document_ime/shared_ime.dart @@ -0,0 +1,228 @@ +import 'package:flutter/cupertino.dart'; +import 'package:flutter/services.dart'; + +/// A globally shared holder of an IME connection, so that the IME connection +/// can be seamlessly transferred between the same `SuperEditor` or `SuperTextField` +/// when their tree is rebuilt. +class SuperIme with ChangeNotifier { + static SuperIme? _instance; + static SuperIme get instance { + _instance ??= SuperIme._(); + return _instance!; + } + + SuperIme._(); + + SuperImeInputId? _owner; + TextInputConnection? _imeConnection; + + /// Returns `true` if [SuperIme] currently holds a Flutter [TextInputConnection] + /// in [imeConnection]. + /// + /// The existence of an [imeConnection] doesn't mean that connection is attached to + /// the operating system. To check that status, use [isAttachedToOS]. + bool get hasConnection => _imeConnection != null; + + /// Returns `true` if [SuperIme] currently holds a Flutter [TextInputConnection] + /// AND that connection is attached to the operating system. + /// + /// When this is `true`, the operating system software keyboard, or other IME + /// interface, is currently interacting with the app (e.g., inputting text). + bool get isAttachedToOS => _imeConnection?.attached ?? false; + + /// Returns `true` if the given [input] is the current owner of the shared IME, + /// and the shared IME is currently attached to the OS. + bool isInputAttachedToOS(SuperImeInputId input) => _owner == input && isAttachedToOS; + + /// If [owner] is the current IME owner, returns the shared [TextInputConnection], or `null` if + /// no such connection currently exists, or if the [owner] isn't actually the owner. + TextInputConnection? getImeConnectionForOwner(SuperImeInputId owner) { + if (owner != _owner) { + return null; + } + + return _imeConnection; + } + + /// If the given [ownerInputId] is the current owner, opens a new [TextInputConnection], and + /// optionally shows the software keyboard. + /// + /// The opened IME connection is available via [getImeConnectionForOwner]. + void openConnection( + SuperImeInputId ownerInputId, + TextInputClient client, + TextInputConfiguration configuration, { + bool showKeyboard = false, + }) { + if (!isOwner(ownerInputId)) { + return; + } + + if (false == _imeConnection?.attached) { + // We have a connection, but its been detached, and we can't re-attach + // without creating a new connection. Throw it away. + // + // While SuperIme might be a global, shared IME, we don't actually have + // global control of the IME. Only Flutter does. We need to be resilient to + // any other Flutter input messing with the IME. + _imeConnection = null; + } + + print("Opening IME connection for $ownerInputId"); + _imeConnection ??= TextInput.attach(client, configuration); + if (showKeyboard) { + _imeConnection!.show(); + } + + notifyListeners(); + } + + /// If the given [ownerInputId] is the current owner, then the current input connection + /// is closed, and the connection null'ed out. + void clearConnection(SuperImeInputId ownerInputId) { + if (!isOwner(ownerInputId)) { + return; + } + + print("Clearing IME connection for $ownerInputId"); + _imeConnection?.close(); + _imeConnection = null; + + notifyListeners(); + } + + /// Returns `true` if a [SuperImeInputId] has claimed ownership of the shared IME. + /// + /// The existence of an owner doesn't imply the existence of an [imeConnection]. It's the + /// owner's job to open and close [imeConnection]s, as needed. + bool get isOwned => _owner != null; + + /// Returns true if the given [inputId] is the current owner of the shared IME. + bool isOwner(SuperImeInputId? inputId) => _owner == inputId; + + /// Takes ownership of the shared IME. + /// + /// Ownership might be taken from another owner, or might be taken at a moment where no + /// other owner exists. Taking ownership doesn't open or close an existing IME connection, + /// it only changes the actor that's allowed to open and access the IME connection. + /// + /// One owner cannot prevent another owner from taking ownership. This mechanism is not + /// a security feature, it's a convenience feature for different areas of code to work + /// together around the fact that only a single IME connection exists per app. + void takeOwnership(SuperImeInputId newOwnerInputId) { + if (_owner == newOwnerInputId) { + return; + } + + print("Taking IME ownership: $newOwnerInputId"); + _owner = newOwnerInputId; + + notifyListeners(); + } + + /// Releases ownership of the IME, if [ownerInputId] is the current owner. + /// + /// We take an [ownerInputId] to reduce the possibility that one IME input accidentally + /// releases ownership when they're not the owner. + /// + /// For convenience, this method closes the open connection upon release, and then + /// throws away the connection, forcing the next owner to create a new connection, + /// and then open it. To prevent this, pass `false` for [clearConnectionOnRelease]. + void releaseOwnership( + SuperImeInputId ownerInputId, { + bool clearConnectionOnRelease = true, + }) { + if (_owner != ownerInputId) { + return; + } + + print("Releasing IME ownership: $ownerInputId"); + if (clearConnectionOnRelease) { + clearConnection(ownerInputId); + } + _owner = null; + + notifyListeners(); + } +} + +/// A specific IME input that might want to own the [SuperIme] shared IME. +/// +/// This class is just a composite ID, which is registered with [SuperIme] to +/// claim ownership over the IME. See [role] and [instance] for their individual +/// meaning. +class SuperImeInputId { + SuperImeInputId({ + required this.role, + required this.instance, + }); + + /// The role this owner is playing in the UI, or `null` if there's only a single + /// input widget in the whole widget tree. + /// + /// It's fine to provide a [role] even if there's only one input in the widget tree. + /// + /// Examples of possible [role] values include things like "chat", "document", "journal", or + /// any other type of content that an input might exist to compose. This choice is up + /// to the developer and the only thing that matters is uniqueness, e.g., "chat" is different + /// from "journal". + /// + /// ### How `role` works + /// The [role] is critical for dealing with `State` disposal and recreation when a + /// widget tree changes an ancestor, and therefore recreates the entire subtree. + /// + /// For example, imagine a widget tree like this: + /// + /// ```dart + /// SuperEditor( + /// //... + /// ) + /// ``` + /// + /// Then, something causes the widget tree to add a `SizedBox` above the `SuperEditor`: + /// + /// ```dart + /// SizedBox( + /// child: SuperEditor( + /// //... + /// ), + /// ); + /// ``` + /// + /// This change causes the `SuperEditor` and all of its internal widgets to be disposed + /// and recreated. More specifically, for each widget in the subtree, a new widget is + /// initialized, and the previous widget is then disposed. + /// + /// But these widgets don't have any idea that they're being replaced - as far as they know + /// they're being permanently destroyed. So should `SuperEditor`'s IME connection be closed + /// or not? + /// + /// This [role] is an ID that binds together the previous `SuperEditor` that's disposed + /// with the new `SuperEditor` that's being created. It tells the disposing `SuperEditor` + /// NOT to close its IME connection, so that the new `SuperEditor` can continue to use it. + /// This sharing prevents unexpected raising of the software keyboard across subtree + /// recreations. + final String? role; + + /// The specific owner of the IME, even within the same [role]. + /// + /// The purpose of [instance] is to differentiate between initializing widgets and + /// disposing widgets for the same input. See [role] for more info. + /// + /// A typical choice to provide as the [instance] is the [State] object that + /// owns a given IME connection. This is a naturally effective choice because the + /// concept of the [instance] is typically used to differentiate between initializing + /// and disposing [State] objects for the same widget. + final Object instance; + + @override + String toString() => "${role ?? 'Global editor'} ($instance)"; + + @override + bool operator ==(Object other) => + identical(this, other) || + other is SuperImeInputId && runtimeType == other.runtimeType && role == other.role && instance == other.instance; + + @override + int get hashCode => role.hashCode ^ instance.hashCode; +} 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 f049474400..19dafd5323 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 @@ -8,6 +8,7 @@ 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/document_gestures_touch_ios.dart'; +import 'package:super_editor/src/default_editor/document_ime/shared_ime.dart'; import 'package:super_editor/src/default_editor/text.dart'; import 'package:super_editor/src/infrastructure/_logging.dart'; import 'package:super_editor/src/infrastructure/actions.dart'; @@ -42,6 +43,7 @@ class SuperEditorImeInteractor extends StatefulWidget { this.clearSelectionWhenEditorLosesFocus = true, this.clearSelectionWhenImeConnectionCloses = true, this.softwareKeyboardController, + this.inputRole, this.imePolicies = const SuperEditorImePolicies(), this.imeConfiguration = const SuperEditorImeConfiguration(), this.imeOverrides, @@ -95,6 +97,8 @@ class SuperEditorImeInteractor extends StatefulWidget { /// might conflict with teh automated behavior. final SoftwareKeyboardController? softwareKeyboardController; + final String? inputRole; + /// Policies that dictate when and how `SuperEditor` should interact with the /// platform IME. final SuperEditorImePolicies imePolicies; @@ -152,11 +156,90 @@ class SuperEditorImeInteractor extends StatefulWidget { @visibleForTesting class SuperEditorImeInteractorState extends State implements ImeInputOwner { + static bool _willCheckUniqueInputsNextFrame = false; + + static final _registeredInputsThisFrame = <(SuperImeInputId inputId, StackTrace stacktrace)>[]; + + /// Ensures that there are no other inputs with the same [inputId] at the end of the current + /// Flutter frame. + /// + /// At the end of the frame, if 2+ inputs registered with the same [SuperImeInputId.role], + /// an exception is thrown, which includes the stack traces for each of those registrations, + /// so that developers can debug why it happened. + static _registerInput(SuperImeInputId inputId) { + if (!kDebugMode) { + return; + } + print("Register input: ${inputId.role}"); + + _registeredInputsThisFrame.add((inputId, StackTrace.current)); + + if (!_willCheckUniqueInputsNextFrame) { + _willCheckUniqueInputsNextFrame = true; + WidgetsBinding.instance.addPostFrameCallback(_verifyUniqueInputs); + } + } + + static void _unregisterInput(SuperImeInputId inputId) { + print("Unregister input: $inputId"); + final startLength = _registeredInputsThisFrame.length; + _registeredInputsThisFrame.removeWhere((entry) => entry.$1.instance == inputId.instance); + + assert( + startLength != _registeredInputsThisFrame.length, + "An IME interactor tried to unregister itself with the global tracker, but we " + "didn't find it in the global tracker. Either it was never added, or it was " + "already removed. Either way, this is a programming mistake that needs to be " + "corrected by the caller.", + ); + } + + static void _verifyUniqueInputs(Duration _) { + print("Running unique input verification"); + // Clear flag so the next time the "ensure" method is called, we'' + // register another post frame callback. + _willCheckUniqueInputsNextFrame = false; + + final inputsByRole = >{}; + + for (final input in _registeredInputsThisFrame) { + inputsByRole[input.$1.role] ??= []; + inputsByRole[input.$1.role]!.add(input); + } + + final duplicates = <(SuperImeInputId, StackTrace)>[]; + for (final entry in inputsByRole.entries) { + if (entry.value.length < 2) { + // No duplicate inputs here. + continue; + } + + duplicates.addAll(entry.value); + } + + if (duplicates.isEmpty) { + // No duplicate inputs anywhere this frame. All good. We're done. + return; + } + + // We found some number of duplicates. Write them all out to an exception message. + throw Exception([ + "Found ${duplicates.length} duplicate input IDs this frame:", + for (final input in duplicates) ...[ + "Input: ${input.$1.role} (${input.$1.instance})", + "This duplicate input was built at this moment:", + input.$2, + "------ END OF ${input.$1.role} (${input.$1.instance}) ----", + ], + ].join("\n")); + } + late FocusNode _focusNode; SuperEditorIosControlsController? _controlsController; - final _imeConnection = ValueNotifier(null); + late SuperImeInputId _myImeId; + final _ownedImeConnection = ValueNotifier(null); late TextInputConfiguration _textInputConfiguration; late DocumentImeInputClient _documentImeClient; // The _imeClient is setup in one of two ways at any given time: @@ -170,20 +253,20 @@ class SuperEditorImeInteractorState extends State impl // implementation of DocumentImeInputClient. If we find a less confusing // way to handle that scenario, then get rid of this property. final _documentImeConnection = ValueNotifier(null); - late TextDeltasDocumentEditor _textDeltasDocumentEditor; @override void initState() { super.initState(); _focusNode = (widget.focusNode ?? FocusNode()); - _setupImeConnection(); + _myImeId = SuperImeInputId(role: widget.inputRole, instance: this); + _registerInput(_myImeId); + SuperIme.instance.addListener(_onSharedImeChange); + _setupDocumentImeInputClient(); _imeClient = DeltaTextInputClientDecorator(); _configureImeClientDecorators(); - _imeConnection.addListener(_onImeConnectionChange); - WidgetsBinding.instance.addPostFrameCallback((_) { // Synchronize the IME connection notifier with our IME connection state. We run // this in a post-frame callback because the very first pump of the Super Editor @@ -208,17 +291,34 @@ class SuperEditorImeInteractorState extends State impl void didUpdateWidget(SuperEditorImeInteractor oldWidget) { super.didUpdateWidget(oldWidget); + if (widget.focusNode != oldWidget.focusNode) { + if (oldWidget.focusNode == null) { + _focusNode.dispose(); + } + } + + if (widget.inputRole != oldWidget.inputRole) { + // We changed roles. Release our previous claim to the shared IME. + SuperIme.instance.releaseOwnership(_myImeId); + + // Create a new IME input ID and re-take IME ownership. + _unregisterInput(_myImeId); + _myImeId = SuperImeInputId(role: widget.inputRole, instance: this); + _registerInput(_myImeId); + SuperIme.instance.takeOwnership(_myImeId); + } + if (widget.editContext != oldWidget.editContext) { - _setupImeConnection(); + _setupDocumentImeInputClient(); + _onSharedImeChange(); _documentImeClient.floatingCursorController = widget.floatingCursorController ?? _controlsController?.floatingCursorController; - _imeConnection.notifyListeners(); } if (widget.imeConfiguration != oldWidget.imeConfiguration) { _textInputConfiguration = widget.imeConfiguration.toTextInputConfiguration(viewId: View.of(context).viewId); if (isAttachedToIme) { - _imeConnection.value!.updateConfig(_textInputConfiguration); + SuperIme.instance.getImeConnectionForOwner(_myImeId)!.updateConfig(_textInputConfiguration); } } @@ -230,17 +330,25 @@ class SuperEditorImeInteractorState extends State impl @override void dispose() { - _imeConnection.removeListener(_onImeConnectionChange); - _imeConnection.value?.close(); - - widget.imeOverrides?.client = null; - _imeClient.client = null; - _documentImeClient.dispose(); + SuperIme.instance.removeListener(_onSharedImeChange); + if (SuperIme.instance.isOwner(_myImeId)) { + // We are the current owner of the IME. Close the IME as we dispose ourselves. + // Note: If this disposal were part of a subtree move somewhere else, the other + // version of this widget would have already registered itself as the IME owner + // in its `initState()` method. + SuperIme.instance.releaseOwnership(_myImeId); + } + _unregisterInput(_myImeId); if (widget.focusNode == null) { _focusNode.dispose(); } + // TODO: Only clear the client if we're still the owner. + widget.imeOverrides?.client = null; + _imeClient.client = null; + _documentImeClient.dispose(); + super.dispose(); } @@ -249,44 +357,56 @@ class SuperEditorImeInteractorState extends State impl DeltaTextInputClient get imeClient => _imeClient; @visibleForTesting - bool get isAttachedToIme => _imeConnection.value?.attached ?? false; + bool get isAttachedToIme => SuperIme.instance.isInputAttachedToOS(_myImeId); - void _setupImeConnection() { - _createTextDeltasDocumentEditor(); - _createDocumentImeClient(); - } + TextInputConnection get imeConnection { + assert(SuperIme.instance.isOwner(_myImeId), + "The imeConnection getter should only be called when you know you're the owner. You're not."); + assert(SuperIme.instance.getImeConnectionForOwner(_myImeId) != null, + "The imeConnection getter should only be called when you know there's an IME connection. There isn't."); - void _createTextDeltasDocumentEditor() { - _textDeltasDocumentEditor = TextDeltasDocumentEditor( - editor: widget.editContext.editor, - document: widget.editContext.document, - documentLayoutResolver: () => widget.editContext.documentLayout, - selection: widget.editContext.composer.selectionNotifier, - composerPreferences: widget.editContext.composer.preferences, - composingRegion: widget.editContext.composer.composingRegion, - commonOps: widget.editContext.commonOps, - onPerformAction: (action) => _imeClient.performAction(action), - ); + return SuperIme.instance.getImeConnectionForOwner(_myImeId)!; } - void _createDocumentImeClient() { + void _setupDocumentImeInputClient() { _documentImeClient = DocumentImeInputClient( selection: widget.editContext.composer.selectionNotifier, composingRegion: widget.editContext.composer.composingRegion, - textDeltasDocumentEditor: _textDeltasDocumentEditor, - imeConnection: _imeConnection, + textDeltasDocumentEditor: TextDeltasDocumentEditor( + editor: widget.editContext.editor, + document: widget.editContext.document, + documentLayoutResolver: () => widget.editContext.documentLayout, + selection: widget.editContext.composer.selectionNotifier, + composerPreferences: widget.editContext.composer.preferences, + composingRegion: widget.editContext.composer.composingRegion, + commonOps: widget.editContext.commonOps, + onPerformAction: (action) => _imeClient.performAction(action), + ), + imeConnection: _ownedImeConnection, onPerformSelector: _onPerformSelector, ); } - void _onImeConnectionChange() { - if (_imeConnection.value == null) { + void _onSharedImeChange() { + // TODO: If we're no longer the owner, do we need to close the IME connection, or did that + // already happen? + if (!SuperIme.instance.isOwner(_myImeId)) { + _ownedImeConnection.value = null; + + _documentImeConnection.value = null; + widget.imeOverrides?.client = null; + widget.isImeConnected?.value = false; + return; + } + + if (!SuperIme.instance.isInputAttachedToOS(_myImeId)) { _documentImeConnection.value = null; widget.imeOverrides?.client = null; widget.isImeConnected?.value = false; return; } + _ownedImeConnection.value = SuperIme.instance.getImeConnectionForOwner(_myImeId); _configureImeClientDecorators(); _documentImeConnection.value = _documentImeClient; @@ -354,7 +474,7 @@ class SuperEditorImeInteractorState extends State impl transform = renderSliver.getTransformTo(null); } - _imeConnection.value!.setEditableSizeAndTransform(size, transform); + SuperIme.instance.getImeConnectionForOwner(_myImeId)!.setEditableSizeAndTransform(size, transform); } void _reportCaretRectToIme() { @@ -367,7 +487,7 @@ class SuperEditorImeInteractorState extends State impl final caretRect = _computeCaretRectInViewportSpace(); if (caretRect != null) { - _imeConnection.value!.setCaretRect(caretRect); + SuperIme.instance.getImeConnectionForOwner(_myImeId)!.setCaretRect(caretRect); } } @@ -401,7 +521,7 @@ class SuperEditorImeInteractorState extends State impl DocumentComponent? selectedComponent = docLayout.getComponentByNodeId(selection.extent.nodeId); if (selectedComponent is ProxyDocumentComponent) { - // The selected componente is a proxy. + // The selected component is a proxy. // If this component displays text, the text component is bounded to childDocumentComponentKey. selectedComponent = selectedComponent.childDocumentComponentKey.currentState as DocumentComponent?; } @@ -417,13 +537,13 @@ class SuperEditorImeInteractorState extends State impl } 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, - ); + SuperIme.instance.getImeConnectionForOwner(_myImeId)!.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. @@ -497,7 +617,7 @@ class SuperEditorImeInteractorState extends State impl @override Widget build(BuildContext context) { return SuperEditorImeDebugVisuals( - imeConnection: _imeConnection, + imeConnection: _ownedImeConnection, child: IntentBlocker( intents: CurrentPlatform.isApple ? appleBlockedIntents : nonAppleBlockedIntents, child: SuperEditorHardwareKeyHandler( @@ -509,7 +629,7 @@ class SuperEditorImeInteractorState extends State impl focusNode: _focusNode, editor: widget.editContext.editor, selection: widget.editContext.composer.selectionNotifier, - imeConnection: _imeConnection, + inputId: _myImeId, imeClientFactory: () => _imeClient, imeConfiguration: _textInputConfiguration, openKeyboardOnSelectionChange: widget.imePolicies.openKeyboardOnSelectionChange, @@ -518,7 +638,7 @@ class SuperEditorImeInteractorState extends State impl clearSelectionWhenImeConnectionCloses: widget.clearSelectionWhenImeConnectionCloses, child: ImeFocusPolicy( focusNode: _focusNode, - imeConnection: _imeConnection, + inputId: _myImeId, imeClientFactory: () => _imeClient, imeConfiguration: _textInputConfiguration, openImeOnPrimaryFocusGain: widget.imePolicies.openKeyboardOnGainPrimaryFocus, @@ -527,7 +647,7 @@ class SuperEditorImeInteractorState extends State impl closeImeOnNonPrimaryFocusLost: widget.imePolicies.closeImeOnNonPrimaryFocusLost, child: SoftwareKeyboardOpener( controller: widget.softwareKeyboardController, - imeConnection: _imeConnection, + inputId: _myImeId, createImeClient: () => _imeClient, createImeConfiguration: () => _textInputConfiguration, child: widget.child, diff --git a/super_editor/lib/src/default_editor/super_editor.dart b/super_editor/lib/src/default_editor/super_editor.dart index 3e6adb9850..f607387960 100644 --- a/super_editor/lib/src/default_editor/super_editor.dart +++ b/super_editor/lib/src/default_editor/super_editor.dart @@ -121,6 +121,7 @@ class SuperEditor extends StatefulWidget { this.selectionPolicies = const SuperEditorSelectionPolicies(), this.inputSource, this.softwareKeyboardController, + this.inputRole, this.imePolicies = const SuperEditorImePolicies(), this.imeConfiguration, this.imeOverrides, @@ -248,6 +249,21 @@ class SuperEditor extends StatefulWidget { /// the automatic behavior might conflict with commands to this controller. final SoftwareKeyboardController? softwareKeyboardController; + /// A name/ID that differentiates this [SuperEditor]'s purpose from any other [SuperEditor] + /// that might be on screen. + /// + /// The [inputRole] is used to control access to the operating system's IME. Imagine that you have + /// Editor1 and Editor2 on screen. You want Editor1 to be able to hold onto the IME connection + /// across widget tree rebuilds, which requires a global connection, but you don't want Editor2 to + /// accidentally take over that global IME connection. The solution is to pass a different [inputRole] + /// for Editor1 and Editor2. + /// + /// If you're sure that you'll only have one editor on screen, you don't need to provide an [inputRole]. + /// + /// The value for [inputRole] is arbitrary. It can be any name you choose, so long as other editors + /// use different names. + final String? inputRole; + /// Policies that dictate when and how [SuperEditor] should interact with the /// platform IME, such as automatically opening the software keyboard when /// [SuperEditor]'s selection changes. @@ -800,6 +816,7 @@ class SuperEditorState extends State { focusNode: _focusNode, autofocus: widget.autofocus, editContext: editContext, + inputRole: widget.inputRole, clearSelectionWhenEditorLosesFocus: widget.selectionPolicies.clearSelectionWhenEditorLosesFocus, clearSelectionWhenImeConnectionCloses: widget.selectionPolicies.clearSelectionWhenImeConnectionCloses, softwareKeyboardController: _softwareKeyboardController, diff --git a/super_editor/lib/src/infrastructure/keyboard_panel_scaffold.dart b/super_editor/lib/src/infrastructure/keyboard_panel_scaffold.dart index ae6a85cc7a..4203673431 100644 --- a/super_editor/lib/src/infrastructure/keyboard_panel_scaffold.dart +++ b/super_editor/lib/src/infrastructure/keyboard_panel_scaffold.dart @@ -41,7 +41,7 @@ class KeyboardPanelScaffold extends StatefulWidget { required this.isImeConnected, required this.toolbarBuilder, required this.keyboardPanelBuilder, - this.fallbackPanelHeight = 250, + this.fallbackPanelHeight = 340, required this.contentBuilder, this.bypassMediaQuery = false, }); @@ -151,7 +151,9 @@ class _KeyboardPanelScaffoldState extends State extends State extends State extends State extends State extends State extends State extends State extends State extends State extends State extends State extends State _updateSafeArea()); break; case KeyboardState.closing: - if (!wantsToShowKeyboardPanel) { + if (!wantsToShowKeyboardPanel && !wantsToShowSoftwareKeyboard) { // The keyboard is collapsing and we don't want the keyboard panel to be visible. // Follow the keyboard back down. // @@ -530,6 +583,7 @@ class _KeyboardPanelScaffoldState extends State extends State extends State 0 || // The keyboard panel should be kept visible while the software keyboard is expanding // and the keyboard panel was previously visible. Otherwise, there will be an empty // region between the top of the software keyboard and the bottom of the above-keyboard panel. (wantsToShowSoftwareKeyboard && SuperKeyboard.instance.mobileGeometry.value.keyboardState != KeyboardState.open); + print("Build() - should show keyboard panel? $shouldShowKeyboardPanel"); + assert(() { keyboardPanelLog.fine(''' Building keyboard scaffold diff --git a/super_editor/lib/super_editor.dart b/super_editor/lib/super_editor.dart index 1d16ec4efc..8c185dc625 100644 --- a/super_editor/lib/super_editor.dart +++ b/super_editor/lib/super_editor.dart @@ -94,6 +94,7 @@ export 'src/infrastructure/platforms/mac/mac_ime.dart'; export 'src/infrastructure/platforms/mobile_documents.dart'; export 'src/infrastructure/scrolling_diagnostics/scrolling_diagnostics.dart'; export 'src/infrastructure/signal_notifier.dart'; +export 'src/infrastructure/sliver_hybrid_stack.dart'; export 'src/infrastructure/strings.dart'; export 'src/super_textfield/super_textfield.dart'; export 'src/infrastructure/touch_controls.dart'; diff --git a/super_editor/test/super_editor/super_editor_ime_ownership_test.dart b/super_editor/test/super_editor/super_editor_ime_ownership_test.dart new file mode 100644 index 0000000000..efb02b3e78 --- /dev/null +++ b/super_editor/test/super_editor/super_editor_ime_ownership_test.dart @@ -0,0 +1,172 @@ +import 'dart:async'; + +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:super_editor/src/default_editor/default_document_editor.dart'; +import 'package:super_editor/src/default_editor/document_ime/shared_ime.dart'; +import 'package:super_editor/src/default_editor/super_editor.dart'; + +void main() { + group("Super Editor > IME ownership >", () { + testWidgets("only claims ownership when it has focus", (tester) async { + final focusNode = FocusNode(debugLabel: "test-editor"); + final editor1 = createDefaultDocumentEditor(); + await _pumpScaffold( + tester, + SuperEditor( + focusNode: focusNode, + editor: editor1, + inputRole: _roleVariant.currentValue, + ), + ); + + expect(SuperIme.instance.isOwned, isFalse); + + // Focus the editor. We do this directly with the `FocusNode` instead of tapping + // on the editor because there many cases where developers give focus to the editor + // in this same way. + focusNode.requestFocus(); + await tester.pump(); + + expect(SuperIme.instance.isOwned, isTrue); + + // Take focus away from the editor. We expect the editor to give up the IME. + focusNode.unfocus(); + await tester.pump(); + + expect(SuperIme.instance.isOwned, isFalse); + }); + + group("catches duplicate roles in the same build >", () { + // Note about timing: This group tests when multiple editors `build()` in the same + // frame. If a given editor already exists, it might not need to `build()` in the + // same frame as another editor. That case is tested in a different group. + + testWidgets("throws exception on 2+", (tester) async { + final editor1 = createDefaultDocumentEditor(); + final editor2 = createDefaultDocumentEditor(); + + // Ensure that we can pump a single editor with a role. + await _pumpScaffold( + tester, + SuperEditor( + editor: editor1, + inputRole: _roleVariant.currentValue, + ), + ); + + // Expect that when we pump two editors with the same role, we get an exception. + final errors = await _captureFlutterErrors( + () => _pumpScaffold( + tester, + Column( + children: [ + Expanded( + child: SuperEditor( + editor: editor1, + inputRole: _roleVariant.currentValue, + ), + ), + Expanded( + child: SuperEditor( + editor: editor2, + // This is the same role as above, which isn't allowed. + inputRole: _roleVariant.currentValue, + ), + ), + ], + ), + ), + ); + + expect(errors.length, 1); + expect(errors.first.exception, isA()); + expect(errors.first.exception.toString(), startsWith("Exception: Found 2 duplicate input IDs this frame:")); + }, variant: _roleVariant); + }); + + group("catches duplicate roles in subsequent builds >", () { + // Note about timing: Multiple editors might run `build()` in the same frame, + // or one editor might already exist and not need to run build, but then a second + // editor builds in another area of the tree. This group tests this timing situation. + + testWidgets("throws exception on 2+", (tester) async { + final editor1 = createDefaultDocumentEditor(); + final editor2 = createDefaultDocumentEditor(); + final buildSecondEditor = ValueNotifier(false); + + // Ensure that we can pump a single editor with a role. + await _pumpScaffold( + tester, + Column( + children: [ + Expanded( + child: SuperEditor( + editor: editor1, + inputRole: _roleVariant.currentValue, + ), + ), + ListenableBuilder( + listenable: buildSecondEditor, + builder: (context, child) { + if (!buildSecondEditor.value) { + return const SizedBox(); + } + + return Expanded( + child: SuperEditor( + editor: editor2, + // This is the same role as above, which isn't allowed. + inputRole: _roleVariant.currentValue, + ), + ); + }), + ], + ), + ); + + // Flip the signal to show the second editor. This should result in the second + // editor building, but the first shouldn't re-run build. + buildSecondEditor.value = true; + + // Pump a frame to let Flutter build what it wants. We expect to capture an exception + // during this pump. + final errors = await _captureFlutterErrors( + () async => await tester.pump(), + ); + + expect(errors.length, 1); + expect(errors.first.exception, isA()); + expect(errors.first.exception.toString(), startsWith("Exception: Found 2 duplicate input IDs this frame:")); + }, variant: _roleVariant); + }); + }); +} + +final _roleVariant = ValueVariant({null, "Chat"}); + +Future _pumpScaffold(WidgetTester tester, Widget child) async { + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + body: child, + ), + ), + ); +} + +FutureOr> _captureFlutterErrors(FutureOr Function() test) async { + final errors = []; + + final originalOnError = FlutterError.onError; + FlutterError.onError = (FlutterErrorDetails details) { + errors.add(details); + }; + + await test(); + + // Restore the original handler to avoid affecting other tests + FlutterError.onError = originalOnError; + + return errors; +} diff --git a/super_editor/test/super_editor/supereditor_multi_editor_test.dart b/super_editor/test/super_editor/supereditor_multi_editor_test.dart index c0f74d6d6d..8129adfcbf 100644 --- a/super_editor/test/super_editor/supereditor_multi_editor_test.dart +++ b/super_editor/test/super_editor/supereditor_multi_editor_test.dart @@ -259,8 +259,6 @@ class _SwitchEditorsDemoState extends State<_SwitchEditorsDemo> { late MutableDocumentComposer _composer2; late Editor _docEditor2; - late Document _activeDocument; - late DocumentComposer _activeComposer; late Editor _activeDocumentEditor; @override @@ -274,13 +272,14 @@ class _SwitchEditorsDemoState extends State<_SwitchEditorsDemo> { _composer2 = widget.composer2 ?? MutableDocumentComposer(); _docEditor2 = createDefaultDocumentEditor(document: _doc2, composer: _composer2); - _activeDocument = _doc1; - _activeComposer = _composer1; _activeDocumentEditor = _docEditor1; } @override void dispose() { + _docEditor1.dispose(); + _docEditor2.dispose(); + super.dispose(); } @@ -293,6 +292,9 @@ class _SwitchEditorsDemoState extends State<_SwitchEditorsDemo> { _buildDocSelector(), Expanded( child: SuperEditor( + // Note: We give this `SuperEditor` the "global" input role because even though + // we're switching its backing `Editor`, we aren't switching between visibly + // different `SuperEditor`s. editor: _activeDocumentEditor, stylesheet: defaultStylesheet.copyWith( documentPadding: const EdgeInsets.symmetric(vertical: 56, horizontal: 24), @@ -313,8 +315,6 @@ class _SwitchEditorsDemoState extends State<_SwitchEditorsDemo> { key: const ValueKey("Editor1"), onPressed: () { setState(() { - _activeDocument = _doc1; - _activeComposer = _composer1; _activeDocumentEditor = _docEditor1; }); }, @@ -325,8 +325,6 @@ class _SwitchEditorsDemoState extends State<_SwitchEditorsDemo> { key: const ValueKey("Editor2"), onPressed: () { setState(() { - _activeDocument = _doc2; - _activeComposer = _composer2; _activeDocumentEditor = _docEditor2; }); },