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] Remove the usage of ScrollbarWithCustomPhysics (Resolves #2561) #2562

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
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
Next Next commit
[SuperEditor] Remove the usage of ScrollbarWithCustomPhysics (Resolves
angelosilvestre committed Feb 2, 2025
commit e725e83017e90653a72d3e2009495d91d400dfe6
12 changes: 1 addition & 11 deletions super_editor/lib/src/default_editor/document_scrollable.dart
Original file line number Diff line number Diff line change
@@ -7,7 +7,6 @@ import 'package:super_editor/src/infrastructure/_logging.dart';
import 'package:super_editor/src/infrastructure/documents/document_scroller.dart';
import 'package:super_editor/src/infrastructure/flutter/build_context.dart';
import 'package:super_editor/src/infrastructure/flutter/flutter_scheduler.dart';
import 'package:super_editor/src/infrastructure/flutter/material_scrollbar.dart';
import 'package:super_editor/src/infrastructure/scrolling_diagnostics/_scrolling_minimap.dart';

import '../infrastructure/document_gestures.dart';
@@ -240,17 +239,8 @@ class _DocumentScrollableState extends State<DocumentScrollable> with SingleTick
return child;
}

// As we handle the scrolling gestures ourselves,
// we use NeverScrollableScrollPhysics to prevent SingleChildScrollView
// from scrolling. This also prevents the user from interacting
// with the scrollbar.
// We use a modified version of Flutter's Scrollbar that allows
// configuring it with a different scroll physics.
//
// See https://github.com/superlistapp/super_editor/issues/1628 for more details.
return ScrollbarWithCustomPhysics(
return Scrollbar(
controller: _scrollController,
physics: behavior.getScrollPhysics(context),
child: child,
);
}
13 changes: 10 additions & 3 deletions super_editor/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -48,9 +48,16 @@ dependencies:
flutter_test_robots: ^0.0.24
clock: ^1.1.1

#dependency_overrides:
# Override to local mono-repo path so devs can test this repo
# against changes that they're making to other mono-repo packages
dependency_overrides:
# IMPORTANT! This is a temporary override to use the new scrollbar extension.
#
# Release a new flutter_test_robots version and remove this override before landing the PR.
flutter_test_robots:
git:
url: https://github.com/angelosilvestre/flutter_test_robots.git
ref: 22_scrollbar_extension
# Override to local mono-repo path so devs can test this repo
# against changes that they're making to other mono-repo packages
# attributed_text:
# path: ../attributed_text
# super_text_layout:
47 changes: 5 additions & 42 deletions super_editor/test/super_editor/supereditor_gestures_test.dart
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import 'dart:ui';

import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
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/links.dart';
import 'package:super_editor/src/infrastructure/platforms/android/selection_handles.dart';
@@ -520,26 +519,8 @@ spans multiple lines.''',
)),
);

// Find the approximate position of the scrollbar thumb.
final thumbLocation = tester.getTopRight(find.byType(SuperEditor)) + const Offset(-10, 10);

// Hover to make the thumb visible with a duration long enough to run the fade in animation.
final testPointer = TestPointer(1, PointerDeviceKind.mouse);

await tester.sendEventToBinding(testPointer.hover(thumbLocation, timeStamp: const Duration(seconds: 1)));
await tester.pumpAndSettle();

// Press the thumb.
await tester.sendEventToBinding(testPointer.down(thumbLocation));
await tester.pump(kTapMinTime);

// Move the thumb down.
await tester.sendEventToBinding(testPointer.move(thumbLocation + const Offset(0, 300)));
await tester.pump();

// Release the pointer.
await tester.sendEventToBinding(testPointer.up());
await tester.pump();
// Drag the scrollbar down.
await tester.dragScrollbar(300);

// Ensure the content scrolled to the end of the document.
expect(scrollController.position.pixels, moreOrLessEquals(770.0));
@@ -578,26 +559,8 @@ spans multiple lines.''',
scrollController.jumpTo(scrollController.position.maxScrollExtent);
await tester.pump();

// Find the approximate position of the scrollbar thumb.
final thumbLocation = tester.getBottomRight(find.byType(SuperEditor)) - const Offset(10, 10);

// Hover to make the thumb visible with a duration long enough to run the fade in animation.
final testPointer = TestPointer(1, PointerDeviceKind.mouse);

await tester.sendEventToBinding(testPointer.hover(thumbLocation, timeStamp: const Duration(seconds: 1)));
await tester.pumpAndSettle();

// Press the thumb.
await tester.sendEventToBinding(testPointer.down(thumbLocation));
await tester.pump(kTapMinTime);

// Move the thumb up.
await tester.sendEventToBinding(testPointer.move(thumbLocation - const Offset(0, 300)));
await tester.pump();

// Release the pointer.
await tester.sendEventToBinding(testPointer.up());
await tester.pump();
// Drag the scrollbar up.
await tester.dragScrollbar(-300);

// Ensure the content scrolled to the beginning of the document.
expect(scrollController.position.pixels, 0);
Original file line number Diff line number Diff line change
@@ -1455,7 +1455,7 @@ void main() {
expect(
find.descendant(
of: find.byType(SuperEditor),
matching: find.byType(ScrollbarWithCustomPhysics),
matching: find.byType(Scrollbar),
),
findsOneWidget,
);
@@ -1487,7 +1487,7 @@ void main() {
expect(
find.descendant(
of: find.byType(SuperEditor),
matching: find.byType(ScrollbarWithCustomPhysics),
matching: find.byType(Scrollbar),
),
findsNothing,
);