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.

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

[SuperEditor][Android] Hide expanded drag handles after deleting the selected text (Resolves #1936) #1951

Merged
merged 3 commits into from
May 20, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -1332,6 +1332,7 @@ class SuperEditorAndroidControlsOverlayManagerState extends State<SuperEditorAnd
_controlsController = SuperEditorAndroidControlsScope.rootOf(context);
// TODO: Replace Cupertino aligner with a generic aligner because this code runs on Android.
_toolbarAligner = CupertinoPopoverToolbarAligner();
widget.document.addListener(_onDocumentChange);
}

@override
Expand All @@ -1345,6 +1346,11 @@ class SuperEditorAndroidControlsOverlayManagerState extends State<SuperEditorAnd
widget.scrollChangeSignal.addListener(_onDocumentScroll);
}
}

if (widget.document != oldWidget.document) {
oldWidget.document.removeListener(_onDocumentChange);
widget.document.addListener(_onDocumentChange);
}
}

@override
Expand All @@ -1353,6 +1359,7 @@ class SuperEditorAndroidControlsOverlayManagerState extends State<SuperEditorAnd
// stop listening for document scroll changes.
widget.dragHandleAutoScroller.value?.stopAutoScrollHandleMonitoring();
widget.scrollChangeSignal.removeListener(_onDocumentScroll);
widget.document.removeListener(_onDocumentChange);

super.dispose();
}
Expand All @@ -1363,6 +1370,23 @@ class SuperEditorAndroidControlsOverlayManagerState extends State<SuperEditorAnd
@visibleForTesting
bool get wantsToDisplayMagnifier => _controlsController!.shouldShowMagnifier.value;

void _onDocumentChange(_) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this only moved the problem I identified before. The problem is that you're trying to identify a user interaction through a document change. My original question, which is still my question, is why can't you identify the user interaction when the user interaction happens, instead of after the fact?

This issue says its about "deleting the selected text". I assume that involves an expanded selection, then some kind of keyboard interaction, then a collapsed selection.

Doesn't this widget already know when the selection is expanded, and when the selection is collapsed? Why can't the widget recognize that the selection has collapsed and then hide the expanded handles, without worrying about document changes at all?

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.

if (widget.selection.value?.isCollapsed == true &&
_controlsController!.shouldShowExpandedHandles.value == true &&
_dragHandleType == null) {
// The selection is collapsed, but the expanded handles are visible and the user isn't dragging a handle.
// This can happen when the selection is expanded, and the user deletes the selected text. The only situation
// where the expanded handles should be visible when the selection is collapsed is when the selection
// collapses while the user is dragging an expanded handle, which isn't the case here. Hide the handles.
_controlsController!
..hideCollapsedHandle()
..hideExpandedHandles()
..hideMagnifier()
..hideToolbar()
..blinkCaret();
}
}

void _toggleToolbarOnCollapsedHandleTap() {
_controlsController!.toggleToolbar();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import 'package:flutter/material.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/super_editor.dart';
import 'package:super_editor/super_editor_test.dart';
import 'package:super_text_layout/super_text_layout.dart';

import '../../test_runners.dart';
import '../../test_tools.dart';
Expand Down Expand Up @@ -212,6 +214,50 @@ void main() {
expect(SuperEditorInspector.isCaretVisible(), isTrue);
});

testWidgetsOnAndroid("hides expanded handles and toolbar when deleting an expanded selection", (tester) async {
// Configure BlinkController to animate, otherwise it won't blink. We want to make sure
// the caret blinks after deleting the content.
BlinkController.indeterminateAnimationsEnabled = true;
addTearDown(() => BlinkController.indeterminateAnimationsEnabled = false);

await _pumpSingleParagraphApp(tester);

// Double tap to select "Lorem".
await tester.doubleTapInParagraph("1", 1);
await tester.pump();

// Ensure the toolbar and the drag handles are visible.
expect(SuperEditorInspector.isMobileToolbarVisible(), isTrue);
expect(SuperEditorInspector.findMobileExpandedDragHandles(), findsNWidgets(2));

// Press backspace to delete the word "Lorem" while the expanded handles are visible.
await tester.ime.backspace(getter: imeClientGetter);

// Ensure the toolbar and the drag handles were hidden.
expect(SuperEditorInspector.isMobileToolbarVisible(), isFalse);
expect(SuperEditorInspector.findMobileExpandedDragHandles(), findsNothing);

// Ensure caret is blinking.

expect(SuperEditorInspector.isCaretVisible(), true);

// Duration to switch between visible and invisible.
final flashPeriod = SuperEditorInspector.caretFlashPeriod();

// Trigger a frame with an ellapsed time equal to the flashPeriod,
// so the caret should change from visible to invisible.
await tester.pump(flashPeriod);

// Ensure caret is invisible after the flash period.
expect(SuperEditorInspector.isCaretVisible(), false);

// Trigger another frame to make caret visible again.
await tester.pump(flashPeriod);

// Ensure caret is visible.
expect(SuperEditorInspector.isCaretVisible(), true);
});

group('shows magnifier above the caret when dragging the collapsed handle', () {
testWidgetsOnAndroid('with an ancestor scrollable', (tester) async {
final scrollController = ScrollController();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
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/super_editor_test.dart';
import 'package:super_text_layout/super_text_layout.dart';

import '../../test_runners.dart';
import '../supereditor_test_tools.dart';
Expand Down Expand Up @@ -109,6 +111,50 @@ void main() {
await tester.pump(const Duration(milliseconds: 100));
});

testWidgetsOnIos("hides expanded handles and toolbar when deleting an expanded selection", (tester) async {
// Configure BlinkController to animate, otherwise it won't blink. We want to make sure
// the caret blinks after deleting the content.
BlinkController.indeterminateAnimationsEnabled = true;
addTearDown(() => BlinkController.indeterminateAnimationsEnabled = false);

await _pumpSingleParagraphApp(tester);

// Double tap to select "Lorem".
await tester.doubleTapInParagraph("1", 1);
await tester.pump();

// Ensure the toolbar and the drag handles are visible.
expect(SuperEditorInspector.isMobileToolbarVisible(), isTrue);
expect(SuperEditorInspector.findMobileExpandedDragHandles(), findsNWidgets(2));

// Press backspace to delete the word "Lorem" while the expanded handles are visible.
await tester.ime.backspace(getter: imeClientGetter);

// Ensure the toolbar and the drag handles were hidden.
expect(SuperEditorInspector.isMobileToolbarVisible(), isFalse);
expect(SuperEditorInspector.findMobileExpandedDragHandles(), findsNothing);

// Ensure caret is blinking.

expect(SuperEditorInspector.isCaretVisible(), true);

// Duration to switch between visible and invisible.
final flashPeriod = SuperEditorInspector.caretFlashPeriod();

// Trigger a frame with an ellapsed time equal to the flashPeriod,
// so the caret should change from visible to invisible.
await tester.pump(flashPeriod);

// Ensure caret is invisible after the flash period.
expect(SuperEditorInspector.isCaretVisible(), false);

// Trigger another frame to make caret visible again.
await tester.pump(flashPeriod);

// Ensure caret is visible.
expect(SuperEditorInspector.isCaretVisible(), true);
});

group("on device and web > shows ", () {
testWidgetsOnIosDeviceAndWeb("caret", (tester) async {
await _pumpSingleParagraphApp(tester);
Expand Down
Loading