Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SuperTextField][mobile] Migrate popover toolbars to OverlayPortal (Resolves #1602) #1604

Merged
merged 3 commits into from
Nov 26, 2023
Merged

[SuperTextField][mobile] Migrate popover toolbars to OverlayPortal (Resolves #1602) #1604

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6418e03
[SuperTextField][mobile] Migrate popover toolbar to OverlayPortal (Re…
angelosilvestre Nov 18, 2023
e26a054
PR updates
angelosilvestre Nov 22, 2023
e185c0d
Remove test suffixes
angelosilvestre Nov 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 48 additions & 29 deletions super_editor/lib/src/super_textfield/android/android_textfield.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import 'package:super_editor/src/infrastructure/flutter/flutter_scheduler.dart';
import 'package:super_editor/src/infrastructure/focus.dart';
import 'package:super_editor/src/infrastructure/ime_input_owner.dart';
import 'package:super_editor/src/infrastructure/platforms/android/toolbar.dart';
import 'package:super_editor/src/infrastructure/signal_notifier.dart';
import 'package:super_editor/src/infrastructure/touch_controls.dart';
import 'package:super_editor/src/super_textfield/android/_editing_controls.dart';
import 'package:super_editor/src/super_textfield/android/_user_interaction.dart';
Expand Down Expand Up @@ -168,10 +169,12 @@ class SuperAndroidTextFieldState extends State<SuperAndroidTextField>

late TextScrollController _textScrollController;

// OverlayEntry that displays the toolbar and magnifier, and
// positions the invisible touch targets for base/extent
// dragging.
OverlayEntry? _controlsOverlayEntry;
/// Opens/closes the popover that displays the toolbar and magnifier, and
// positions the invisible touch targets for base/extent dragging.
final _popoverController = OverlayPortalController();

/// Notifies the popover toolbar to rebuild itself.
final _popoverRebuildSignal = SignalNotifier();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this rebuild signal here because we're scheduling extra frames to position the toolbar? If so, can we use a Leader and Follower instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this is used to position the toolbar. Currently, we are doing _controlsOverlayEntry?.markNeedsBuild(); to do this.

We can use Leader and Follower, but it will require big change. I'm not sure we should do that on this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. We can do it in a follow up. Can you file a ticket for that?


@override
void initState() {
Expand Down Expand Up @@ -293,6 +296,8 @@ class SuperAndroidTextFieldState extends State<SuperAndroidTextField>
..removeListener(_onTextScrollChange)
..dispose();

_popoverRebuildSignal.dispose();

WidgetsBinding.instance.removeObserver(this);

super.dispose();
Expand Down Expand Up @@ -359,46 +364,32 @@ class SuperAndroidTextFieldState extends State<SuperAndroidTextField>
}

void _onTextScrollChange() {
if (_controlsOverlayEntry != null) {
if (_popoverController.isShowing) {
_rebuildEditingOverlayControls();
}
}

/// Displays [AndroidEditingOverlayControls] in the app's [Overlay], if not already
/// Displays [AndroidEditingOverlayControls] in the [OverlayPortal], if not already
/// displayed.
void _showEditingControlsOverlay() {
if (_controlsOverlayEntry == null) {
_controlsOverlayEntry = OverlayEntry(builder: (overlayContext) {
return AndroidEditingOverlayControls(
editingController: _editingOverlayController,
textScrollController: _textScrollController,
textFieldLayerLink: _textFieldLayerLink,
textFieldKey: _textFieldKey,
textContentLayerLink: _textContentLayerLink,
textContentKey: _textContentKey,
tapRegionGroupId: widget.tapRegionGroupId,
handleColor: widget.handlesColor,
popoverToolbarBuilder: widget.popoverToolbarBuilder,
showDebugPaint: widget.showDebugPaint,
);
});

Overlay.of(context).insert(_controlsOverlayEntry!);
if (!_popoverController.isShowing) {
_popoverController.show();
}
}

/// Rebuilds the [AndroidEditingControls] in the app's [Overlay], if
/// Rebuilds the [AndroidEditingOverlayControls] in the [OverlayPortal], if
/// they're currently displayed.
void _rebuildEditingOverlayControls() {
_controlsOverlayEntry?.markNeedsBuild();
if (_popoverController.isShowing) {
_popoverRebuildSignal.notifyListeners();
}
}

/// Removes [AndroidEditingControls] from the app's [Overlay], if they're
/// Hides the [AndroidEditingOverlayControls] in the [OverlayPortal], if they're
/// currently displayed.
void _removeEditingOverlayControls() {
if (_controlsOverlayEntry != null) {
_controlsOverlayEntry!.remove();
_controlsOverlayEntry = null;
if (_popoverController.isShowing) {
_popoverController.hide();
}
}

Expand Down Expand Up @@ -497,6 +488,14 @@ class SuperAndroidTextFieldState extends State<SuperAndroidTextField>

@override
Widget build(BuildContext context) {
return OverlayPortal(
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that we're bringing an overlay portal into this, let's breakdown this build method so it's clear which pieces are what.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated.

controller: _popoverController,
overlayChildBuilder: _buildPopoverToolbar,
child: _buildTextField(),
);
}

Widget _buildTextField() {
return TapRegion(
groupId: widget.tapRegionGroupId,
child: NonReparentingFocus(
Expand Down Expand Up @@ -578,6 +577,26 @@ class SuperAndroidTextFieldState extends State<SuperAndroidTextField>
),
);
}

Widget _buildPopoverToolbar(BuildContext context) {
return ListenableBuilder(
listenable: _popoverRebuildSignal,
builder: (context, _) {
return AndroidEditingOverlayControls(
editingController: _editingOverlayController,
textScrollController: _textScrollController,
textFieldLayerLink: _textFieldLayerLink,
textFieldKey: _textFieldKey,
textContentLayerLink: _textContentLayerLink,
textContentKey: _textContentKey,
tapRegionGroupId: widget.tapRegionGroupId,
handleColor: widget.handlesColor,
popoverToolbarBuilder: widget.popoverToolbarBuilder,
showDebugPaint: widget.showDebugPaint,
);
},
);
}
}

Widget _defaultAndroidToolbarBuilder(
Expand Down
77 changes: 48 additions & 29 deletions super_editor/lib/src/super_textfield/ios/ios_textfield.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import 'package:super_editor/src/infrastructure/focus.dart';
import 'package:super_editor/src/infrastructure/ime_input_owner.dart';
import 'package:super_editor/src/infrastructure/platforms/ios/toolbar.dart';
import 'package:super_editor/src/infrastructure/platforms/mobile_documents.dart';
import 'package:super_editor/src/infrastructure/signal_notifier.dart';
import 'package:super_editor/src/super_textfield/infrastructure/fill_width_if_constrained.dart';
import 'package:super_editor/src/super_textfield/infrastructure/hint_text.dart';
import 'package:super_editor/src/super_textfield/infrastructure/text_scrollview.dart';
Expand Down Expand Up @@ -177,10 +178,12 @@ class SuperIOSTextFieldState extends State<SuperIOSTextField>

late MagnifierAndToolbarController _overlayController;

// OverlayEntry that displays the toolbar and magnifier, and
// positions the invisible touch targets for base/extent
// dragging.
OverlayEntry? _controlsOverlayEntry;
/// Opens/closes the popover that displays the toolbar and magnifier, and
// positions the invisible touch targets for base/extent dragging.
final _popoverController = OverlayPortalController();

/// Notifies the popover toolbar to rebuild itself.
final _popoverRebuildSignal = SignalNotifier();

@override
void initState() {
Expand Down Expand Up @@ -316,6 +319,8 @@ class SuperIOSTextFieldState extends State<SuperIOSTextField>

WidgetsBinding.instance.removeObserver(this);

_popoverRebuildSignal.dispose();

super.dispose();
}

Expand Down Expand Up @@ -380,46 +385,32 @@ class SuperIOSTextFieldState extends State<SuperIOSTextField>
}

void _onTextScrollChange() {
if (_controlsOverlayEntry != null) {
if (_popoverController.isShowing) {
_rebuildHandles();
}
}

/// Displays [IOSEditingControls] in the app's [Overlay], if not already
/// Displays [IOSEditingControls] in the [OverlayPortal], if not already
/// displayed.
void _showHandles() {
if (_controlsOverlayEntry == null) {
_controlsOverlayEntry = OverlayEntry(builder: (overlayContext) {
return IOSEditingControls(
editingController: _editingOverlayController,
textScrollController: _textScrollController,
textFieldLayerLink: _textFieldLayerLink,
textFieldKey: _textFieldKey,
textContentLayerLink: _textContentLayerLink,
textContentKey: _textContentKey,
tapRegionGroupId: widget.tapRegionGroupId,
handleColor: widget.handlesColor,
popoverToolbarBuilder: _defaultPopoverToolbarBuilder,
showDebugPaint: widget.showDebugPaint,
);
});

Overlay.of(context).insert(_controlsOverlayEntry!);
if (!_popoverController.isShowing) {
_popoverController.show();
}
}

/// Rebuilds the [IOSEditingControls] in the app's [Overlay], if
/// Rebuilds the [IOSEditingControls] in the [OverlayPortal], if
/// they're currently displayed.
void _rebuildHandles() {
_controlsOverlayEntry?.markNeedsBuild();
if (!_popoverController.isShowing) {
_popoverRebuildSignal.notifyListeners();
}
}

/// Removes [IOSEditingControls] from the app's [Overlay], if they're
/// Hides the [IOSEditingControls] in the [OverlayPortal], if they're
/// currently displayed.
void _removeEditingOverlayControls() {
if (_controlsOverlayEntry != null) {
_controlsOverlayEntry!.remove();
_controlsOverlayEntry = null;
if (_popoverController.isShowing) {
_popoverController.hide();
}
}

Expand Down Expand Up @@ -499,6 +490,14 @@ class SuperIOSTextFieldState extends State<SuperIOSTextField>

@override
Widget build(BuildContext context) {
return OverlayPortal(
controller: _popoverController,
overlayChildBuilder: _buildPopoverToolbar,
child: _buildTextField(),
);
}

Widget _buildTextField() {
return TapRegion(
groupId: widget.tapRegionGroupId,
child: NonReparentingFocus(
Expand Down Expand Up @@ -595,6 +594,26 @@ class SuperIOSTextFieldState extends State<SuperIOSTextField>
),
);
}

Widget _buildPopoverToolbar(BuildContext context) {
return ListenableBuilder(
listenable: _popoverRebuildSignal,
builder: (context, _) {
return IOSEditingControls(
editingController: _editingOverlayController,
textScrollController: _textScrollController,
textFieldLayerLink: _textFieldLayerLink,
textFieldKey: _textFieldKey,
textContentLayerLink: _textContentLayerLink,
textContentKey: _textContentKey,
tapRegionGroupId: widget.tapRegionGroupId,
handleColor: widget.handlesColor,
popoverToolbarBuilder: widget.popoverToolbarBuilder,
showDebugPaint: widget.showDebugPaint,
);
},
);
}
}

Widget _defaultPopoverToolbarBuilder(BuildContext context, IOSEditingOverlayController controller) {
Expand Down
3 changes: 1 addition & 2 deletions super_editor/test/super_editor/supereditor_content_insertion_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,8 +550,7 @@ Second paragraph"""). //
);
});

testWidgetsOnIos(
'when the user presses the newline button on the software keyboard at the end of an image (on iOS)',
testWidgetsOnIos('when the user presses the newline button on the software keyboard at the end of an image',
(tester) async {
final testContext = await tester
.createDocument()
Expand Down
2 changes: 1 addition & 1 deletion super_editor/test/super_editor/supereditor_gestures_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ spans multiple lines.''',
expect(find.byType(AndroidSelectionHandle), findsOneWidget);
});

testWidgetsOnIos('configures default gesture mode (on iOS)', (tester) async {
testWidgetsOnIos('configures default gesture mode', (tester) async {
await tester //
.createDocument()
.withSingleParagraph()
Expand Down
Loading
Loading