firka/flutter
firka/flutter
4
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix update drag error that made NestedScrollView un-scrollable (#127718)

Browse Source 
#### (plus some more docs)

Fixes https://github.com/flutter/flutter/issues/76760
Fixes https://github.com/flutter/flutter/issues/82391
Fixes https://github.com/flutter/flutter/issues/45619
Fixes #117316
Fixes #110956
Fixes #127282 
Fixes #32563
Fixes #46089 
Fixes #79077
Part of fixing #62833

This fixes (a bunch of) issues that have been reported differently over the years, but all have the same root cause. Sometimes the NestedScrollView would incorrectly calculate whether or not there is enough content to allow scrolling. This would only apply to drag scrolling (not mouse wheel scrolling for example). This did not relate to how the extent of the NestedScrollView is computed, but just the logic that enabled the actual drag gestures. This fixes that. :)
This commit is contained in:
Kate Lovett 2023-06-05 16:14:22 -05:00 committed by GitHub
parent 270b3d487c
commit 9a263466ae
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 323 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

66
packages/flutter/lib/src/widgets/nested_scroll_view.dart
View File

@ -197,6 +197,13 @@ class NestedScrollView extends StatefulWidget {
final ScrollController? controller;
/// {@macro flutter.widgets.scroll_view.scrollDirection}
///
/// This property only applies to the [Axis] of the outer scroll view,
/// composed of the slivers returned from [headerSliverBuilder]. Since the
/// inner scroll view is not directly configured by the [NestedScrollView],
/// for the axes to match, configure the scroll view of the [body] the same
/// way if they are expected to scroll in the same orientation. This allows
/// for flexible configurations of the NestedScrollView.
final Axis scrollDirection;
/// Whether the scroll view scrolls in the reading direction.
@ -210,6 +217,13 @@ class NestedScrollView extends StatefulWidget {
/// scrolls from top to bottom when [reverse] is false and from bottom to top
/// when [reverse] is true.
///
/// This property only applies to the outer scroll view, composed of the
/// slivers returned from [headerSliverBuilder]. Since the inner scroll view
/// is not directly configured by the [NestedScrollView]. For both to scroll
/// in reverse, configure the scroll view of the [body] the same way if they
/// are expected to match. This allows for flexible configurations of the
/// NestedScrollView.
///
/// Defaults to false.
final bool reverse;
@ -232,6 +246,22 @@ class NestedScrollView extends StatefulWidget {
/// [ScrollMetrics.maxScrollExtent] properties passed to that method. If that
/// invariant is not maintained, the nested scroll view may respond to user
/// scrolling erratically.
///
/// This property only applies to the outer scroll view, composed of the
/// slivers returned from [headerSliverBuilder]. Since the inner scroll view
/// is not directly configured by the [NestedScrollView]. For both to scroll
/// with the same [ScrollPhysics], configure the scroll view of the [body]
/// the same way if they are expected to match, or use a [ScrollBehavior] as
/// an ancestor so both the inner and outer scroll views inherit the same
/// [ScrollPhysics]. This allows for flexible configurations of the
/// NestedScrollView.
///
/// The [ScrollPhysics] also determine whether or not the [NestedScrollView]
/// can accept input from the user to change the scroll offset. For example,
/// [NeverScrollableScrollPhysics] typically will not allow the user to drag a
/// scroll view, but in this case, if one of the two scroll views can be
/// dragged, then dragging will be allowed. Configuring both scroll views with
/// [NeverScrollableScrollPhysics] will disallow dragging in this case.
final ScrollPhysics? physics;
/// A builder for any widgets that are to precede the inner scroll views (as
@ -845,17 +875,18 @@ class _NestedScrollCoordinator implements ScrollActivityDelegate, ScrollHoldCont
if (!_outerPosition!.haveDimensions) {
return;
}
double maxInnerExtent = 0.0;
bool innerCanDrag = false;
for (final _NestedScrollPosition position in _innerPositions) {
if (!position.haveDimensions) {
return;
}
maxInnerExtent = math.max(
maxInnerExtent,
position.maxScrollExtent - position.minScrollExtent,
);
innerCanDrag = innerCanDrag
// This refers to the physics of the actual inner scroll position, not
// the whole NestedScrollView, since it is possible to have different
// ScrollPhysics for the inner and outer positions.
|| position.physics.shouldAcceptUserOffset(position);
}
_outerPosition!.updateCanDrag(maxInnerExtent);
_outerPosition!.updateCanDrag(innerCanDrag);
}
Future<void> animateTo(
@ -1438,9 +1469,16 @@ class _NestedScrollPosition extends ScrollPosition implements ScrollActivityDele
coordinator.updateCanDrag();
}
void updateCanDrag(double totalExtent) {
context.setCanDrag(physics.allowUserScrolling &&
(totalExtent > (viewportDimension - maxScrollExtent) || minScrollExtent != maxScrollExtent));
void updateCanDrag(bool innerCanDrag) {
// This is only called for the outer position
assert(coordinator._outerPosition == this);
context.setCanDrag(
// This refers to the physics of the actual outer scroll position, not
// the whole NestedScrollView, since it is possible to have different
// ScrollPhysics for the inner and outer positions.
physics.shouldAcceptUserOffset(this)
|| innerCanDrag,
);
}
@override
@ -1756,17 +1794,9 @@ class RenderSliverOverlapAbsorber extends RenderSliver with RenderObjectWithChil
}
child!.layout(constraints, parentUsesSize: true);
final SliverGeometry childLayoutGeometry = child!.geometry!;
geometry = SliverGeometry(
geometry = childLayoutGeometry.copyWith(
scrollExtent: childLayoutGeometry.scrollExtent - childLayoutGeometry.maxScrollObstructionExtent,
paintExtent: childLayoutGeometry.paintExtent,
paintOrigin: childLayoutGeometry.paintOrigin,
layoutExtent: math.max(0, childLayoutGeometry.paintExtent - childLayoutGeometry.maxScrollObstructionExtent),
maxPaintExtent: childLayoutGeometry.maxPaintExtent,
maxScrollObstructionExtent: childLayoutGeometry.maxScrollObstructionExtent,
hitTestExtent: childLayoutGeometry.hitTestExtent,
visible: childLayoutGeometry.visible,
hasVisualOverflow: childLayoutGeometry.hasVisualOverflow,
scrollOffsetCorrection: childLayoutGeometry.scrollOffsetCorrection,
);
handle._setExtents(
childLayoutGeometry.maxScrollObstructionExtent,

279
packages/flutter/test/widgets/nested_scroll_view_test.dart
View File

@ -871,7 +871,11 @@ void main() {
headerSliverBuilder: (BuildContext context, bool innerBoxIsScrolled) {
return <Widget>[
const SliverPersistentHeader(
delegate: TestHeader(key: key1),
delegate: TestHeader(
key: key1,
minExtent: 100.0,
maxExtent: 100.0,
),
),
];
},
@ -2948,17 +2952,284 @@ void main() {
contains('SliverOverlapInjector has found no absorbed extent to inject.'),
);
});
group('NestedScrollView properly sets drag', () {
Future<bool> canDrag(WidgetTester tester) async {
await tester.drag(
find.byType(CustomScrollView),
const Offset(0.0, -20.0),
);
await tester.pumpAndSettle();
final NestedScrollViewState nestedScrollView = tester.state<NestedScrollViewState>(
find.byType(NestedScrollView)
);
return nestedScrollView.outerController.position.pixels > 0.0
|| nestedScrollView.innerController.position.pixels > 0.0;
}
Widget buildTest({
// The body length is to test when the nested scroll view should or
// should not be allowing drag.
required _BodyLength bodyLength,
Widget? header,
bool applyOverlap = false,
}) {
return MaterialApp(
home: Scaffold(
body: NestedScrollView(
headerSliverBuilder: (BuildContext context, _) {
if (applyOverlap) {
return <Widget>[
SliverOverlapAbsorber(
handle: NestedScrollView.sliverOverlapAbsorberHandleFor(context),
sliver: header,
),
];
}
return header != null ? <Widget>[ header ] : <Widget>[];
},
body: Builder(
builder: (BuildContext context) {
return CustomScrollView(
slivers: <Widget>[
SliverList.builder(
itemCount: switch (bodyLength) {
_BodyLength.short => 10,
_BodyLength.long => 100,
},
itemBuilder: (_, int index) => Text('Item $index'),
),
],
);
}
),
),
)
);
}
testWidgets('when headerSliverBuilder is empty', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/117316
// Regression test for https://github.com/flutter/flutter/issues/46089
// Short body / long body
for (final _BodyLength bodyLength in _BodyLength.values) {
await tester.pumpWidget(
buildTest(bodyLength: bodyLength),
);
await tester.pumpAndSettle();
switch (bodyLength) {
case _BodyLength.short:
expect(await canDrag(tester), isFalse);
case _BodyLength.long:
expect(await canDrag(tester), isTrue);
}
}
}, variant: TargetPlatformVariant.all());
testWidgets('when headerSliverBuilder extent is 0', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/79077
// Short body / long body
for (final _BodyLength bodyLength in _BodyLength.values) {
// SliverPersistentHeader
await tester.pumpWidget(
buildTest(
bodyLength: bodyLength,
header: const SliverPersistentHeader(
delegate: TestHeader(minExtent: 0.0, maxExtent: 0.0),
),
),
);
await tester.pumpAndSettle();
switch (bodyLength) {
case _BodyLength.short:
expect(await canDrag(tester), isFalse);
case _BodyLength.long:
expect(await canDrag(tester), isTrue);
}
// SliverPersistentHeader pinned
await tester.pumpWidget(
buildTest(
bodyLength: bodyLength,
header: const SliverPersistentHeader(
pinned: true,
delegate: TestHeader(minExtent: 0.0, maxExtent: 0.0),
),
),
);
await tester.pumpAndSettle();
switch (bodyLength) {
case _BodyLength.short:
expect(await canDrag(tester), isFalse);
case _BodyLength.long:
expect(await canDrag(tester), isTrue);
}
// SliverPersistentHeader floating
await tester.pumpWidget(
buildTest(
bodyLength: bodyLength,
header: const SliverPersistentHeader(
floating: true,
delegate: TestHeader(minExtent: 0.0, maxExtent: 0.0),
),
),
);
await tester.pumpAndSettle();
switch (bodyLength) {
case _BodyLength.short:
expect(await canDrag(tester), isFalse);
case _BodyLength.long:
expect(await canDrag(tester), isTrue);
}
// SliverPersistentHeader pinned+floating
await tester.pumpWidget(
buildTest(
bodyLength: bodyLength,
header: const SliverPersistentHeader(
pinned: true,
floating: true,
delegate: TestHeader(minExtent: 0.0, maxExtent: 0.0),
),
),
);
await tester.pumpAndSettle();
switch (bodyLength) {
case _BodyLength.short:
expect(await canDrag(tester), isFalse);
case _BodyLength.long:
expect(await canDrag(tester), isTrue);
}
// SliverPersistentHeader w/ overlap
await tester.pumpWidget(
buildTest(
bodyLength: bodyLength,
applyOverlap: true,
header: const SliverPersistentHeader(
delegate: TestHeader(minExtent: 0.0, maxExtent: 0.0),
),
),
);
await tester.pumpAndSettle();
switch (bodyLength) {
case _BodyLength.short:
expect(await canDrag(tester), isFalse);
case _BodyLength.long:
expect(await canDrag(tester), isTrue);
}
// SliverPersistentHeader pinned w/ overlap
await tester.pumpWidget(
buildTest(
bodyLength: bodyLength,
applyOverlap: true,
header: const SliverPersistentHeader(
pinned: true,
delegate: TestHeader(minExtent: 0.0, maxExtent: 0.0),
),
),
);
await tester.pumpAndSettle();
switch (bodyLength) {
case _BodyLength.short:
expect(await canDrag(tester), isFalse);
case _BodyLength.long:
expect(await canDrag(tester), isTrue);
}
// SliverPersistentHeader floating w/ overlap
await tester.pumpWidget(
buildTest(
bodyLength: bodyLength,
applyOverlap: true,
header: const SliverPersistentHeader(
floating: true,
delegate: TestHeader(minExtent: 0.0, maxExtent: 0.0),
),
),
);
await tester.pumpAndSettle();
switch (bodyLength) {
case _BodyLength.short:
expect(await canDrag(tester), isFalse);
case _BodyLength.long:
expect(await canDrag(tester), isTrue);
}
// SliverPersistentHeader pinned+floating w/ overlap
await tester.pumpWidget(
buildTest(
bodyLength: bodyLength,
applyOverlap: true,
header: const SliverPersistentHeader(
floating: true,
pinned: true,
delegate: TestHeader(minExtent: 0.0, maxExtent: 0.0),
),
),
);
await tester.pumpAndSettle();
switch (bodyLength) {
case _BodyLength.short:
expect(await canDrag(tester), isFalse);
case _BodyLength.long:
expect(await canDrag(tester), isTrue);
}
}
}, variant: TargetPlatformVariant.all());
testWidgets('With a pinned SliverAppBar', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/110956
// Regression test for https://github.com/flutter/flutter/issues/127282
// Regression test for https://github.com/flutter/flutter/issues/32563
// Regression test for https://github.com/flutter/flutter/issues/79077
// Short / long body
for (final _BodyLength bodyLength in _BodyLength.values) {
await tester.pumpWidget(
buildTest(
bodyLength: bodyLength,
applyOverlap: true,
header: const SliverAppBar(
title: Text('Test'),
pinned: true,
bottom: PreferredSize(
preferredSize: Size.square(25),
child: SizedBox(),
),
),
),
);
await tester.pumpAndSettle();
switch (bodyLength) {
case _BodyLength.short:
expect(await canDrag(tester), isFalse);
case _BodyLength.long:
expect(await canDrag(tester), isTrue);
}
}
});
});
}
double appBarHeight(WidgetTester tester) => tester.getSize(find.byType(AppBar, skipOffstage: false)).height;
enum _BodyLength {
short,
long,
}
class TestHeader extends SliverPersistentHeaderDelegate {
const TestHeader({ this.key });
const TestHeader({
this.key,
required this.minExtent,
required this.maxExtent,
});
final Key? key;
@override
double get minExtent => 100.0;
final double minExtent;
@override
double get maxExtent => 100.0;
final double maxExtent;
@override
Widget build(BuildContext context, double shrinkOffset, bool overlapsContent) {
return Placeholder(key: key);