Skip to content

Commit 9ee00ae

Browse files
Fix NestedScrollView inner position logic (flutter#157756)
closes flutter#40740 <p align="center"> <img src="https://github.com/user-attachments/assets/8a48abd0-466b-4e06-90f3-dd05869bac27" alt="NestedScrollController fix" height="720px" /> </p> <details> <summary><h4>Demo source code</h4> (click to expand)</summary> (this is a slightly more concise version of the code sample from flutter#40740) ```dart import 'package:flutter/material.dart'; void main() { runApp( const MaterialApp( debugShowCheckedModeBanner: false, home: Scaffold(body: NewsScreen()), ), ); } class NewsScreen extends StatelessWidget { const NewsScreen({super.key}); static const List<String> _tabs = <String>['Featured', 'Popular', 'Latest']; static final List<Widget> _tabViews = <Widget>[ for (final String name in _tabs) SafeArea( top: false, bottom: false, child: Builder(builder: (BuildContext context) { final handle = NestedScrollView.sliverOverlapAbsorberHandleFor(context); return NotificationListener<ScrollNotification>( onNotification: (ScrollNotification notification) => true, child: CustomScrollView( key: PageStorageKey<String>(name), slivers: <Widget>[ SliverOverlapInjector(handle: handle), SliverPadding( padding: const EdgeInsets.all(8.0), sliver: SliverList( delegate: SliverChildBuilderDelegate( childCount: 30, (BuildContext context, int index) => Container( margin: const EdgeInsets.only(bottom: 8), width: double.infinity, height: 150, color: const Color(0xFFB0A4C8), alignment: Alignment.center, child: Text( '$name $index', style: const TextStyle(fontWeight: FontWeight.w600), ), ), ), ), ), ], ), ); }), ), ]; @OverRide Widget build(BuildContext context) { return DefaultTabController( length: _tabs.length, child: NestedScrollView( headerSliverBuilder: (BuildContext context, bool innerBoxIsScrolled) => <Widget>[ SliverOverlapAbsorber( handle: NestedScrollView.sliverOverlapAbsorberHandleFor(context), sliver: SliverSafeArea( top: false, sliver: SliverAppBar( title: const Text('Tab Demo'), floating: true, pinned: true, snap: true, forceElevated: innerBoxIsScrolled, bottom: TabBar( tabs: _tabs.map((String name) => Tab(text: name)).toList(), ), ), ), ), ], body: TabBarView(children: _tabViews), ), ); } } ``` <br> </details> <br> This bug can be traced to a return statement inside `_NestedScrollPosition`: ```dart double applyClampedDragUpdate(double delta) { // ... return delta + offset; } ``` Thanks to some quirks of floating-point arithmetic, `applyClampedDragUpdate` would sometimes return a tiny non-zero value, which ends up ruining one of the scroll coordinator's equality checks. https://github.com/flutter/flutter/blob/8990ed6538592fc8aee63b0e137b7b52804a030f/packages/flutter/lib/src/widgets/nested_scroll_view.dart#L658-L664
1 parent 4a33136 commit 9ee00ae

File tree

2 files changed

+117
-1
lines changed

2 files changed

+117
-1
lines changed

packages/flutter/lib/src/widgets/nested_scroll_view.dart

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1320,7 +1320,12 @@ class _NestedScrollPosition extends ScrollPosition implements ScrollActivityDele
13201320
forcePixels(actualNewPixels);
13211321
didUpdateScrollPositionBy(offset);
13221322
}
1323-
return delta + offset;
1323+
1324+
final double result = delta + offset;
1325+
if (result.abs() < precisionErrorTolerance) {
1326+
return 0.0;
1327+
}
1328+
return result;
13241329
}
13251330

13261331
// Returns the overscroll.

packages/flutter/test/widgets/nested_scroll_view_test.dart

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3628,6 +3628,117 @@ void main() {
36283628
});
36293629
});
36303630

3631+
// Regression test for https://github.com/flutter/flutter/issues/40740.
3632+
testWidgets('Maintains scroll position of inactive tab', (WidgetTester tester) async {
3633+
const List<String> tabs = <String>['Featured', 'Popular', 'Latest'];
3634+
final List<Widget> tabViews = <Widget>[
3635+
for (final String name in tabs)
3636+
SafeArea(
3637+
top: false,
3638+
bottom: false,
3639+
child: Builder(
3640+
builder: (BuildContext context) => CustomScrollView(
3641+
key: PageStorageKey<String>(name),
3642+
slivers: <Widget>[
3643+
SliverOverlapInjector(
3644+
handle: NestedScrollView.sliverOverlapAbsorberHandleFor(context),
3645+
),
3646+
SliverPadding(
3647+
padding: const EdgeInsets.all(8.0),
3648+
sliver: SliverList(
3649+
delegate: SliverChildBuilderDelegate(
3650+
childCount: 30,
3651+
(BuildContext context, int index) {
3652+
return ListTile(title: Text('Item $index'));
3653+
},
3654+
),
3655+
),
3656+
),
3657+
],
3658+
),
3659+
),
3660+
),
3661+
];
3662+
3663+
await tester.pumpWidget(
3664+
MaterialApp(
3665+
home: Scaffold(
3666+
body: DefaultTabController(
3667+
length: tabs.length,
3668+
child: NestedScrollView(
3669+
headerSliverBuilder: (BuildContext context, bool innerBoxIsScrolled) => <Widget>[
3670+
SliverOverlapAbsorber(
3671+
handle: NestedScrollView.sliverOverlapAbsorberHandleFor(context),
3672+
sliver: SliverSafeArea(
3673+
top: false,
3674+
sliver: SliverAppBar(
3675+
title: const Text('Tab Demo'),
3676+
floating: true,
3677+
pinned: true,
3678+
snap: true,
3679+
forceElevated: innerBoxIsScrolled,
3680+
bottom: TabBar(
3681+
tabs: tabs.map((String name) => Tab(text: name)).toList(),
3682+
),
3683+
),
3684+
),
3685+
),
3686+
],
3687+
body: TabBarView(children: tabViews),
3688+
),
3689+
),
3690+
),
3691+
),
3692+
);
3693+
3694+
final Finder finder = find.text('Item 14', skipOffstage: false);
3695+
final Finder findAny = find.descendant(
3696+
of: find.byType(SliverList),
3697+
matching: find.byType(ListTile),
3698+
).first;
3699+
3700+
Future<void> scroll(VerticalDirection direction) async {
3701+
switch (direction) {
3702+
case VerticalDirection.down:
3703+
while (!tester.any(finder)) {
3704+
await tester.fling(find.byType(Scaffold), const Offset(0, -50), 20);
3705+
await tester.pumpAndSettle();
3706+
}
3707+
await tester.ensureVisible(finder);
3708+
case VerticalDirection.up:
3709+
await tester.fling(find.byType(Scaffold), const Offset(0, 20), 20);
3710+
await tester.pumpAndSettle();
3711+
}
3712+
}
3713+
3714+
double getScrollPosition() {
3715+
return Scrollable.of(tester.element(findAny)).position.pixels;
3716+
}
3717+
3718+
expect(getScrollPosition(), 0.0);
3719+
3720+
// Scroll toward the bottom of Tab 1.
3721+
await scroll(VerticalDirection.down);
3722+
final double tab1Position = getScrollPosition();
3723+
3724+
// Switch to second tab.
3725+
await tester.tap(find.text('Popular'));
3726+
await tester.pumpAndSettle();
3727+
3728+
// Scroll toward the bottom of the second tab.
3729+
await scroll(VerticalDirection.down);
3730+
final double tab2Position = getScrollPosition();
3731+
3732+
// Scroll up a bit in the second tab.
3733+
await scroll(VerticalDirection.up);
3734+
expect(getScrollPosition(), lessThan(tab2Position));
3735+
3736+
// Switch back to the first tab.
3737+
await tester.tap(find.text('Featured'));
3738+
await tester.pumpAndSettle();
3739+
expect(getScrollPosition(), tab1Position);
3740+
});
3741+
36313742
testWidgets('$SliverOverlapAbsorberHandle dispatches creation in constructor', (WidgetTester widgetTester) async {
36323743
await expectLater(
36333744
await memoryEvents(() => SliverOverlapAbsorberHandle().dispose(), SliverOverlapAbsorberHandle),

0 commit comments

Comments
 (0)