Skip to content

F
Front-End
Unverified Commit e9542ec6 authored by David Martos's avatar David Martos Committed by GitHub
Browse files
Options

Assert for valid ScrollController in Scrollbar gestures (#81278)

parent 8603ed99
Hide whitespace changes
Inline Side-by-side
Showing with 243 additions and 75 deletions
packages/flutter/lib/src/cupertino/dialog.dart
View file @ e9542ec6
......@@ -278,6 +278,9 @@ class CupertinoAlertDialog extends StatelessWidget {
/// section when there are many actions.
final ScrollController? scrollController;
ScrollController get _effectiveScrollController =>
scrollController ?? ScrollController();
/// A scroll controller that can be used to control the scrolling of the
/// actions in the dialog.
///
......@@ -289,6 +292,9 @@ class CupertinoAlertDialog extends StatelessWidget {
/// section when it is long.
final ScrollController? actionScrollController;
ScrollController get _effectiveActionScrollController =>
actionScrollController ?? ScrollController();
/// {@macro flutter.material.dialog.insetAnimationDuration}
final Duration insetAnimationDuration;
......@@ -305,7 +311,7 @@ class CupertinoAlertDialog extends StatelessWidget {
child: _CupertinoAlertContentSection(
title: title,
message: content,
scrollController: scrollController,
scrollController: _effectiveScrollController,
titlePadding: EdgeInsets.only(
left: _kDialogEdgePadding,
right: _kDialogEdgePadding,
......@@ -344,7 +350,7 @@ class CupertinoAlertDialog extends StatelessWidget {
);
if (actions.isNotEmpty) {
actionSection = _CupertinoAlertActionSection(
scrollController: actionScrollController,
scrollController: _effectiveActionScrollController,
isActionSheet: false,
children: actions,
);
......@@ -591,12 +597,18 @@ class CupertinoActionSheet extends StatelessWidget {
/// short.
final ScrollController? messageScrollController;
ScrollController get _effectiveMessageScrollController =>
messageScrollController ?? ScrollController();
/// A scroll controller that can be used to control the scrolling of the
/// [actions] in the action sheet.
///
/// This attribute is typically not needed.
final ScrollController? actionScrollController;
ScrollController get _effectiveActionScrollController =>
actionScrollController ?? ScrollController();
/// The optional cancel button that is grouped separately from the other
/// actions.
///
......@@ -609,7 +621,7 @@ class CupertinoActionSheet extends StatelessWidget {
final Widget titleSection = _CupertinoAlertContentSection(
title: title,
message: message,
scrollController: messageScrollController,
scrollController: _effectiveMessageScrollController,
titlePadding: const EdgeInsets.only(
left: _kActionSheetContentHorizontalPadding,
right: _kActionSheetContentHorizontalPadding,
......@@ -650,7 +662,7 @@ class CupertinoActionSheet extends StatelessWidget {
);
}
return _CupertinoAlertActionSection(
scrollController: actionScrollController,
scrollController: _effectiveActionScrollController,
hasCancelButton: cancelButton != null,
isActionSheet: true,
children: actions!,
......@@ -1501,6 +1513,7 @@ class _CupertinoAlertContentSection extends StatelessWidget {
}
return CupertinoScrollbar(
controller: scrollController,
child: SingleChildScrollView(
controller: scrollController,
child: Column(
......@@ -1565,6 +1578,7 @@ class _CupertinoAlertActionSectionState
}
return CupertinoScrollbar(
controller: widget.scrollController,
child: SingleChildScrollView(
controller: widget.scrollController,
child: _CupertinoDialogActionsRenderWidget(
......
packages/flutter/lib/src/widgets/scrollbar.dart
View file @ e9542ec6
......@@ -1026,84 +1026,105 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
// A scroll event is required in order to paint the thumb.
void _maybeTriggerScrollbar() {
WidgetsBinding.instance!.addPostFrameCallback((Duration duration) {
final ScrollController? scrollController = widget.controller ?? PrimaryScrollController.of(context);
if (showScrollbar) {
_fadeoutTimer?.cancel();
// Wait one frame and cause an empty scroll event. This allows the
// thumb to show immediately when isAlwaysShown is true. A scroll
// event is required in order to paint the thumb.
final ScrollController? scrollController = widget.controller ?? PrimaryScrollController.of(context);
final bool tryPrimary = widget.controller == null;
final String controllerForError = tryPrimary
? 'provided ScrollController'
: 'PrimaryScrollController';
assert(
scrollController != null,
'A ScrollController is required when Scrollbar.isAlwaysShown is true. '
'${tryPrimary ? 'The Scrollbar was not provided a ScrollController, '
'and attempted to use the PrimaryScrollController, but none was found.' :''}',
);
assert (() {
if (!scrollController!.hasClients) {
throw FlutterError.fromParts(<DiagnosticsNode>[
ErrorSummary(
"The Scrollbar's ScrollController has no ScrollPosition attached.",
),
ErrorDescription(
'A Scrollbar cannot be painted without a ScrollPosition. ',
),
ErrorHint(
'The Scrollbar attempted to use the $controllerForError. This '
'ScrollController should be associated with the ScrollView that '
'the Scrollbar is being applied to. '
'${tryPrimary
? 'A ScrollView with an Axis.vertical '
'ScrollDirection will automatically use the '
'PrimaryScrollController if the user has not provided a '
'ScrollController, but a ScrollDirection of Axis.horizontal will '
'not. To use the PrimaryScrollController explicitly, set ScrollView.primary '
'to true for the Scrollable widget.'
: 'When providing your own ScrollController, ensure both the '
'Scrollbar and the Scrollable widget use the same one.'
}',
),
]);
}
return true;
}());
assert (() {
try {
scrollController!.position;
} catch (_) {
throw FlutterError.fromParts(<DiagnosticsNode>[
ErrorSummary(
'The $controllerForError is currently attached to more than one '
'ScrollPosition.',
),
ErrorDescription(
'The Scrollbar requires a single ScrollPosition in order to be painted.',
),
ErrorHint(
'When Scrollbar.isAlwaysShown is true, the associated Scrollable '
'widgets must have unique ScrollControllers. '
'${tryPrimary
? 'The PrimaryScrollController is used by default for '
'ScrollViews with an Axis.vertical ScrollDirection, '
'unless the ScrollView has been provided its own '
'ScrollController. More than one Scrollable may have tried '
'to use the PrimaryScrollController of the current context.'
: 'The provided ScrollController must be unique to a '
'Scrollable widget.'
}',
),
]);
}
return true;
}());
_checkForValidScrollPosition();
scrollController!.position.didUpdateScrollPositionBy(0);
}
// Interactive scrollbars need to be properly configured.
// If there is no scroll controller, there will not be gestures at all.
if (scrollController != null && enableGestures) {
_checkForValidScrollPosition();
}
});
}
void _checkForValidScrollPosition() {
final ScrollController? scrollController = widget.controller ?? PrimaryScrollController.of(context);
final bool tryPrimary = widget.controller == null;
final String controllerForError = tryPrimary
? 'provided ScrollController'
: 'PrimaryScrollController';
String when = '';
if (showScrollbar) {
when = 'Scrollbar.isAlwaysShown is true';
} else if (enableGestures) {
when = 'the scrollbar is interactive';
} else {
when = 'using the Scrollbar';
}
assert(
scrollController != null,
'A ScrollController is required when $when. '
'${tryPrimary ? 'The Scrollbar was not provided a ScrollController, '
'and attempted to use the PrimaryScrollController, but none was found.' :''}',
);
assert (() {
if (!scrollController!.hasClients) {
throw FlutterError.fromParts(<DiagnosticsNode>[
ErrorSummary(
"The Scrollbar's ScrollController has no ScrollPosition attached.",
),
ErrorDescription(
'A Scrollbar cannot be painted without a ScrollPosition. ',
),
ErrorHint(
'The Scrollbar attempted to use the $controllerForError. This '
'ScrollController should be associated with the ScrollView that '
'the Scrollbar is being applied to. '
'${tryPrimary
? 'A ScrollView with an Axis.vertical '
'ScrollDirection will automatically use the '
'PrimaryScrollController if the user has not provided a '
'ScrollController, but a ScrollDirection of Axis.horizontal will '
'not. To use the PrimaryScrollController explicitly, set ScrollView.primary '
'to true for the Scrollable widget.'
: 'When providing your own ScrollController, ensure both the '
'Scrollbar and the Scrollable widget use the same one.'
}',
),
]);
}
return true;
}());
assert (() {
try {
scrollController!.position;
} catch (_) {
throw FlutterError.fromParts(<DiagnosticsNode>[
ErrorSummary(
'The $controllerForError is currently attached to more than one '
'ScrollPosition.',
),
ErrorDescription(
'The Scrollbar requires a single ScrollPosition in order to be painted.',
),
ErrorHint(
'When $when, the associated Scrollable '
'widgets must have unique ScrollControllers. '
'${tryPrimary
? 'The PrimaryScrollController is used by default for '
'ScrollViews with an Axis.vertical ScrollDirection, '
'unless the ScrollView has been provided its own '
'ScrollController. More than one Scrollable may have tried '
'to use the PrimaryScrollController of the current context.'
: 'The provided ScrollController must be unique to a '
'Scrollable widget.'
}',
),
]);
}
return true;
}());
}
/// This method is responsible for configuring the [scrollbarPainter]
/// according to the [widget]'s properties and any inherited widgets the
/// painter depends on, like [Directionality] and [MediaQuery].
......@@ -1191,6 +1212,7 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
@protected
@mustCallSuper
void handleThumbPress() {
_checkForValidScrollPosition();
if (getScrollbarDirection() == null) {
return;
}
......@@ -1203,6 +1225,7 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
@protected
@mustCallSuper
void handleThumbPressStart(Offset localPosition) {
_checkForValidScrollPosition();
_currentController = widget.controller ?? PrimaryScrollController.of(context);
final Axis? direction = getScrollbarDirection();
if (direction == null) {
......@@ -1219,6 +1242,7 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
@protected
@mustCallSuper
void handleThumbPressUpdate(Offset localPosition) {
_checkForValidScrollPosition();
final Axis? direction = getScrollbarDirection();
if (direction == null) {
return;
......@@ -1231,9 +1255,11 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
@protected
@mustCallSuper
void handleThumbPressEnd(Offset localPosition, Velocity velocity) {
_checkForValidScrollPosition();
final Axis? direction = getScrollbarDirection();
if (direction == null)
if (direction == null) {
return;
}
_maybeStartFadeoutTimer();
_dragScrollbarAxisOffset = null;
_currentController = null;
......@@ -1241,6 +1267,7 @@ class RawScrollbarState<T extends RawScrollbar> extends State<T> with TickerProv
void _handleTrackTapDown(TapDownDetails details) {
// The Scrollbar should page towards the position of the tap on the track.
_checkForValidScrollPosition();
_currentController = widget.controller ?? PrimaryScrollController.of(context);
double scrollIncrement;
......
packages/flutter/test/cupertino/action_sheet_test.dart
View file @ e9542ec6
......@@ -389,6 +389,36 @@ void main() {
expect(tester.getSize(find.byType(CupertinoActionSheet)).height, screenHeight);
});
testWidgets('CupertinoActionSheet scrollbars controllers should be different', (WidgetTester tester) async {
// https://github.com/flutter/flutter/pull/81278
await tester.pumpWidget(
createAppWithButtonThatLaunchesActionSheet(
CupertinoActionSheet(
title: const Text('The title'),
message: Text('Very long content' * 200),
actions: <Widget>[
CupertinoActionSheetAction(
child: const Text('One'),
onPressed: () { },
),
],
)
),
);
await tester.tap(find.text('Go'));
await tester.pump();
final List<CupertinoScrollbar> scrollbars =
find.descendant(
of: find.byType(CupertinoActionSheet),
matching: find.byType(CupertinoScrollbar),
).evaluate().map((Element e) => e.widget as CupertinoScrollbar).toList();
expect(scrollbars.length, 2);
expect(scrollbars[0].controller != scrollbars[1].controller, isTrue);
});
testWidgets('Tap on button calls onPressed', (WidgetTester tester) async {
bool wasPressed = false;
await tester.pumpWidget(
......
packages/flutter/test/cupertino/dialog_test.dart
View file @ e9542ec6
......@@ -1286,6 +1286,32 @@ void main() {
// one for the content.
expect(find.byType(CupertinoScrollbar), findsNWidgets(2));
}, variant: TargetPlatformVariant.all());
testWidgets('CupertinoAlertDialog scrollbars controllers should be different', (WidgetTester tester) async {
// https://github.com/flutter/flutter/pull/81278
await tester.pumpWidget(
const MaterialApp(
home: MediaQuery(
data: MediaQueryData(viewInsets: EdgeInsets.zero),
child: CupertinoAlertDialog(
actions: <Widget>[
CupertinoDialogAction(child: Text('OK')),
],
content: Placeholder(fallbackHeight: 200.0),
),
),
),
);
final List<CupertinoScrollbar> scrollbars =
find.descendant(
of: find.byType(CupertinoAlertDialog),
matching: find.byType(CupertinoScrollbar),
).evaluate().map((Element e) => e.widget as CupertinoScrollbar).toList();
expect(scrollbars.length, 2);
expect(scrollbars[0].controller != scrollbars[1].controller, isTrue);
});
}
RenderBox findActionButtonRenderBoxByTitle(WidgetTester tester, String title) {
......
packages/flutter/test/cupertino/scrollbar_test.dart
View file @ e9542ec6
......@@ -934,6 +934,41 @@ void main() {
);
});
testWidgets('Interactive scrollbars should have a valid scroll controller', (WidgetTester tester) async {
final ScrollController primaryScrollController = ScrollController();
final ScrollController scrollController = ScrollController();
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: MediaQuery(
data: const MediaQueryData(),
child: PrimaryScrollController(
controller: primaryScrollController,
child: CupertinoScrollbar(
child: SingleChildScrollView(
controller: scrollController,
child: const SizedBox(
height: 1000.0,
width: 1000.0,
),
),
),
),
),
),
);
await tester.pumpAndSettle();
final dynamic exception = tester.takeException();
expect(exception, isAssertionError);
expect(
(exception as AssertionError).message,
contains("The Scrollbar's ScrollController has no ScrollPosition attached."),
);
});
testWidgets('Simultaneous dragging and pointer scrolling does not cause a crash', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/70105
final ScrollController scrollController = ScrollController();
......
packages/flutter/test/material/scrollbar_test.dart
View file @ e9542ec6
......@@ -1051,8 +1051,9 @@ void main() {
),
child: Scrollbar(
controller: controller,
child: const SingleChildScrollView(
child: SizedBox(width: 4000.0, height: 4000.0),
child: SingleChildScrollView(
controller: controller,
child: const SizedBox(width: 4000.0, height: 4000.0),
),
),
),
......
packages/flutter/test/widgets/scrollbar_test.dart
View file @ e9542ec6
......@@ -1171,6 +1171,41 @@ void main() {
);
});
testWidgets('Interactive scrollbars should have a valid scroll controller', (WidgetTester tester) async {
final ScrollController primaryScrollController = ScrollController();
final ScrollController scrollController = ScrollController();
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: MediaQuery(
data: const MediaQueryData(),
child: PrimaryScrollController(
controller: primaryScrollController,
child: RawScrollbar(
child: SingleChildScrollView(
controller: scrollController,
child: const SizedBox(
height: 1000.0,
width: 1000.0,
),
),
),
),
),
),
);
await tester.pumpAndSettle();
final dynamic exception = tester.takeException();
expect(exception, isAssertionError);
expect(
(exception as AssertionError).message,
contains("The Scrollbar's ScrollController has no ScrollPosition attached."),
);
});
testWidgets('Simultaneous dragging and pointer scrolling does not cause a crash', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/70105
final ScrollController scrollController = ScrollController();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment

HIAST - GitLab