Unverified Commit 92992550 authored by Kate Lovett's avatar Kate Lovett Committed by GitHub

Fix widgets with built-in scrollbars (#83828)

parent a4215931
......@@ -365,44 +365,48 @@ class CupertinoAlertDialog extends StatelessWidget {
// iOS does not shrink dialog content below a 1.0 scale factor
textScaleFactor: math.max(textScaleFactor, 1.0),
),
child: LayoutBuilder(
builder: (BuildContext context, BoxConstraints constraints) {
return AnimatedPadding(
padding: MediaQuery.of(context).viewInsets +
const EdgeInsets.symmetric(horizontal: 40.0, vertical: 24.0),
duration: insetAnimationDuration,
curve: insetAnimationCurve,
child: MediaQuery.removeViewInsets(
removeLeft: true,
removeTop: true,
removeRight: true,
removeBottom: true,
context: context,
child: Center(
child: Container(
margin: const EdgeInsets.symmetric(vertical: _kDialogEdgePadding),
width: isInAccessibilityMode
? _kAccessibilityCupertinoDialogWidth
: _kCupertinoDialogWidth,
child: CupertinoPopupSurface(
isSurfacePainted: false,
child: Semantics(
namesRoute: true,
scopesRoute: true,
explicitChildNodes: true,
label: localizations.alertDialogLabel,
child: _CupertinoDialogRenderWidget(
contentSection: _buildContent(context),
actionsSection: _buildActions(),
dividerColor: CupertinoColors.separator,
child: ScrollConfiguration(
// A CupertinoScrollbar is built-in below.
behavior: ScrollConfiguration.of(context).copyWith(scrollbars: false),
child: LayoutBuilder(
builder: (BuildContext context, BoxConstraints constraints) {
return AnimatedPadding(
padding: MediaQuery.of(context).viewInsets +
const EdgeInsets.symmetric(horizontal: 40.0, vertical: 24.0),
duration: insetAnimationDuration,
curve: insetAnimationCurve,
child: MediaQuery.removeViewInsets(
removeLeft: true,
removeTop: true,
removeRight: true,
removeBottom: true,
context: context,
child: Center(
child: Container(
margin: const EdgeInsets.symmetric(vertical: _kDialogEdgePadding),
width: isInAccessibilityMode
? _kAccessibilityCupertinoDialogWidth
: _kCupertinoDialogWidth,
child: CupertinoPopupSurface(
isSurfacePainted: false,
child: Semantics(
namesRoute: true,
scopesRoute: true,
explicitChildNodes: true,
label: localizations.alertDialogLabel,
child: _CupertinoDialogRenderWidget(
contentSection: _buildContent(context),
actionsSection: _buildActions(),
dividerColor: CupertinoColors.separator,
),
),
),
),
),
),
),
);
},
);
},
),
),
),
);
......@@ -694,23 +698,27 @@ class CupertinoActionSheet extends StatelessWidget {
}
return SafeArea(
child: Semantics(
namesRoute: true,
scopesRoute: true,
explicitChildNodes: true,
label: 'Alert',
child: CupertinoUserInterfaceLevel(
data: CupertinoUserInterfaceLevelData.elevated,
child: Container(
width: actionSheetWidth,
margin: const EdgeInsets.symmetric(
horizontal: _kActionSheetEdgeHorizontalPadding,
vertical: _kActionSheetEdgeVerticalPadding,
),
child: Column(
mainAxisSize: MainAxisSize.min,
crossAxisAlignment: CrossAxisAlignment.stretch,
children: children,
child: ScrollConfiguration(
// A CupertinoScrollbar is built-in below
behavior: ScrollConfiguration.of(context).copyWith(scrollbars: false),
child: Semantics(
namesRoute: true,
scopesRoute: true,
explicitChildNodes: true,
label: 'Alert',
child: CupertinoUserInterfaceLevel(
data: CupertinoUserInterfaceLevelData.elevated,
child: Container(
width: actionSheetWidth,
margin: const EdgeInsets.symmetric(
horizontal: _kActionSheetEdgeHorizontalPadding,
vertical: _kActionSheetEdgeVerticalPadding,
),
child: Column(
mainAxisSize: MainAxisSize.min,
crossAxisAlignment: CrossAxisAlignment.stretch,
children: children,
),
),
),
),
......
......@@ -925,8 +925,12 @@ class _PackageLicensePageState extends State<_PackageLicensePage> {
child: Localizations.override(
locale: const Locale('en', 'US'),
context: context,
child: Scrollbar(
child: ListView(padding: padding, children: listWidgets),
child: ScrollConfiguration(
// A Scrollbar is built-in below.
behavior: ScrollConfiguration.of(context).copyWith(scrollbars: false),
child: Scrollbar(
child: ListView(padding: padding, children: listWidgets),
),
),
),
),
......
......@@ -285,9 +285,10 @@ class _DropdownMenuState<T> extends State<_DropdownMenu<T>> {
textStyle: route.style,
child: ScrollConfiguration(
// Dropdown menus should never overscroll or display an overscroll indicator.
// The default scrollbar platforms will apply.
// Scrollbars are built-in below.
// Platform must use Theme and ScrollPhysics must be Clamping.
behavior: ScrollConfiguration.of(context).copyWith(
scrollbars: false,
overscroll: false,
physics: const ClampingScrollPhysics(),
platform: Theme.of(context).platform,
......
......@@ -3,6 +3,7 @@
// found in the LICENSE file.
import 'package:flutter/cupertino.dart';
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
......@@ -993,6 +994,46 @@ void main() {
semantics.dispose();
});
testWidgets('Conflicting scrollbars are not applied by ScrollBehavior to CupertinoActionSheet', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/83819
final ScrollController actionScrollController = ScrollController();
await tester.pumpWidget(
createAppWithButtonThatLaunchesActionSheet(
Builder(builder: (BuildContext context) {
return MediaQuery(
data: MediaQuery.of(context).copyWith(textScaleFactor: 3.0),
child: CupertinoActionSheet(
title: const Text('The title'),
message: const Text('The message.'),
actions: <Widget>[
CupertinoActionSheetAction(
child: const Text('One'),
onPressed: () { },
),
CupertinoActionSheetAction(
child: const Text('Two'),
onPressed: () { },
),
],
actionScrollController: actionScrollController,
),
);
}),
),
);
await tester.tap(find.text('Go'));
await tester.pump();
// The inherited ScrollBehavior should not apply Scrollbars since they are
// already built in to the widget.
expect(find.byType(Scrollbar), findsNothing);
expect(find.byType(RawScrollbar), findsNothing);
// Built in CupertinoScrollbars should only number 2: one for the actions,
// one for the content.
expect(find.byType(CupertinoScrollbar), findsNWidgets(2));
}, variant: TargetPlatformVariant.all());
}
RenderBox findScrollableActionsSectionRenderBox(WidgetTester tester) {
......
......@@ -5,6 +5,7 @@
import 'dart:math';
import 'package:flutter/cupertino.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
......@@ -1247,6 +1248,45 @@ void main() {
await tester.restoreFrom(restorationData);
expect(find.byType(CupertinoAlertDialog), findsOneWidget);
}, skip: isBrowser); // https://github.com/flutter/flutter/issues/33615
testWidgets('Conflicting scrollbars are not applied by ScrollBehavior to CupertinoAlertDialog', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/83819
const double textScaleFactor = 1.0;
final ScrollController actionScrollController = ScrollController();
await tester.pumpWidget(
createAppWithButtonThatLaunchesDialog(
dialogBuilder: (BuildContext context) {
return MediaQuery(
data: MediaQuery.of(context).copyWith(textScaleFactor: textScaleFactor),
child: CupertinoAlertDialog(
title: const Text('Test Title'),
content: const Text('Test Content'),
actions: const <Widget>[
CupertinoDialogAction(
child: Text('One'),
),
CupertinoDialogAction(
child: Text('Two'),
),
],
actionScrollController: actionScrollController,
),
);
},
),
);
await tester.tap(find.text('Go'));
await tester.pump();
// The inherited ScrollBehavior should not apply Scrollbars since they are
// already built in to the widget.
expect(find.byType(Scrollbar), findsNothing);
expect(find.byType(RawScrollbar), findsNothing);
// Built in CupertinoScrollbars should only number 2: one for the actions,
// one for the content.
expect(find.byType(CupertinoScrollbar), findsNWidgets(2));
}, variant: TargetPlatformVariant.all());
}
RenderBox findActionButtonRenderBoxByTitle(WidgetTester tester, String title) {
......
......@@ -4,6 +4,7 @@
import 'dart:async';
import 'package:flutter/cupertino.dart';
import 'package:flutter/foundation.dart';
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
......@@ -705,6 +706,50 @@ void main() {
expect(materialDones[0].color, scaffoldColor);
expect(materialDones[1].color, cardColor);
});
testWidgets('Conflicting scrollbars are not applied by ScrollBehavior to _PackageLicensePage', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/83819
LicenseRegistry.addLicense(() {
return Stream<LicenseEntry>.fromIterable(<LicenseEntry>[
const LicenseEntryWithLineBreaks(<String>['AAA'], 'BBB'),
]);
});
await tester.pumpWidget(
const MaterialApp(
home: Center(
child: LicensePage(),
),
),
);
await tester.pumpAndSettle();
// Check for packages.
expect(find.text('AAA'), findsOneWidget);
// Check license is displayed after entering into license page for 'AAA'.
await tester.tap(find.text('AAA'));
await tester.pumpAndSettle(const Duration(milliseconds: 100));
// The inherited ScrollBehavior should not apply Scrollbars since they are
// already built in to the widget.
switch (debugDefaultTargetPlatformOverride) {
case TargetPlatform.android:
case TargetPlatform.fuchsia:
case TargetPlatform.linux:
case TargetPlatform.macOS:
case TargetPlatform.windows:
expect(find.byType(CupertinoScrollbar), findsNothing);
break;
case TargetPlatform.iOS:
expect(find.byType(CupertinoScrollbar), findsOneWidget);
break;
case null:
break;
}
expect(find.byType(Scrollbar), findsOneWidget);
expect(find.byType(RawScrollbar), findsNothing);
}, variant: TargetPlatformVariant.all());
}
class FakeLicenseEntry extends LicenseEntry {
......
......@@ -5,6 +5,7 @@
import 'dart:math' as math;
import 'dart:ui' show window;
import 'package:flutter/cupertino.dart';
import 'package:flutter/gestures.dart';
import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart';
......@@ -3449,4 +3450,26 @@ void main() {
await gesture.moveTo(offDropdownButton);
expect(RendererBinding.instance!.mouseTracker.debugDeviceActiveCursor(1), SystemMouseCursors.basic);
});
testWidgets('Conflicting scrollbars are not applied by ScrollBehavior to Dropdown', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/83819
// Open the dropdown menu
final Key buttonKey = UniqueKey();
await tester.pumpWidget(buildFrame(
buttonKey: buttonKey,
value: null, // nothing selected
items: List<String>.generate(100, (int index) => index.toString()),
onChanged: onChanged,
));
await tester.tap(find.byKey(buttonKey));
await tester.pump();
await tester.pumpAndSettle(); // finish the menu animation
// The inherited ScrollBehavior should not apply Scrollbars since they are
// already built in to the widget.
expect(find.byType(CupertinoScrollbar), findsNothing);
expect(find.byType(Scrollbar), findsOneWidget);
expect(find.byType(RawScrollbar), findsNothing);
}, variant: TargetPlatformVariant.all());
}
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