Skip to content

Commit 2cd4485

Browse files
polina-cpolina-c
authored
Reland "Remove hack from PageView." (#141533)
Fixes flutter/flutter#141119 Original PR: flutter/flutter#141138 Revert: flutter/flutter#141479 Reason for revert: https://fusion2.corp.google.com/presubmit/597877179/OCL:597877179:BASE:597883748:1705084754455:88d992fc/targets Fix: cl/599347719
1 parent e05d0dd commit 2cd4485

File tree

4 files changed

+134
-41
lines changed

4 files changed

+134
-41
lines changed

‎examples/api/test/widgets/page_view/page_view.0_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ void main() {
4949

5050
// Verify that page view index is also updated with same index to page indicator.
5151
final PageView pageView = tester.widget<PageView>(find.byType(PageView));
52-
expect(pageView.controller.page, 1);
52+
expect(pageView.controller!.page, 1);
5353

5454
// Tap backward button on page indicator area.
5555
await tester.tap(find.byIcon(Icons.arrow_left_rounded));

‎packages/flutter/lib/src/widgets/page_view.dart

Lines changed: 39 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -576,11 +576,6 @@ class PageScrollPhysics extends ScrollPhysics {
576576
bool get allowImplicitScrolling => false;
577577
}
578578

579-
// Having this global (mutable) page controller is a bit of a hack. We need it
580-
// to plumb in the factory for _PagePosition, but it will end up accumulating
581-
// a large list of scroll positions. As long as you don't try to actually
582-
// control the scroll positions, everything should be fine.
583-
final PageController _defaultPageController = PageController();
584579
const PageScrollPhysics _kPagePhysics = PageScrollPhysics();
585580

586581
/// A scrollable list that works page by page.
@@ -645,7 +640,7 @@ class PageView extends StatefulWidget {
645640
super.key,
646641
this.scrollDirection = Axis.horizontal,
647642
this.reverse = false,
648-
PageController? controller,
643+
this.controller,
649644
this.physics,
650645
this.pageSnapping = true,
651646
this.onPageChanged,
@@ -656,8 +651,7 @@ class PageView extends StatefulWidget {
656651
this.clipBehavior = Clip.hardEdge,
657652
this.scrollBehavior,
658653
this.padEnds = true,
659-
}) : controller = controller ?? _defaultPageController,
660-
childrenDelegate = SliverChildListDelegate(children);
654+
}) : childrenDelegate = SliverChildListDelegate(children);
661655

662656
/// Creates a scrollable list that works page by page using widgets that are
663657
/// created on demand.
@@ -688,7 +682,7 @@ class PageView extends StatefulWidget {
688682
super.key,
689683
this.scrollDirection = Axis.horizontal,
690684
this.reverse = false,
691-
PageController? controller,
685+
this.controller,
692686
this.physics,
693687
this.pageSnapping = true,
694688
this.onPageChanged,
@@ -701,8 +695,7 @@ class PageView extends StatefulWidget {
701695
this.clipBehavior = Clip.hardEdge,
702696
this.scrollBehavior,
703697
this.padEnds = true,
704-
}) : controller = controller ?? _defaultPageController,
705-
childrenDelegate = SliverChildBuilderDelegate(
698+
}) : childrenDelegate = SliverChildBuilderDelegate(
706699
itemBuilder,
707700
findChildIndexCallback: findChildIndexCallback,
708701
childCount: itemCount,
@@ -719,11 +712,11 @@ class PageView extends StatefulWidget {
719712
/// {@end-tool}
720713
///
721714
/// {@macro flutter.widgets.PageView.allowImplicitScrolling}
722-
PageView.custom({
715+
const PageView.custom({
723716
super.key,
724717
this.scrollDirection = Axis.horizontal,
725718
this.reverse = false,
726-
PageController? controller,
719+
this.controller,
727720
this.physics,
728721
this.pageSnapping = true,
729722
this.onPageChanged,
@@ -734,7 +727,7 @@ class PageView extends StatefulWidget {
734727
this.clipBehavior = Clip.hardEdge,
735728
this.scrollBehavior,
736729
this.padEnds = true,
737-
}) : controller = controller ?? _defaultPageController;
730+
});
738731

739732
/// Controls whether the widget's pages will respond to
740733
/// [RenderObject.showOnScreen], which will allow for implicit accessibility
@@ -776,7 +769,7 @@ class PageView extends StatefulWidget {
776769

777770
/// An object that can be used to control the position to which this page
778771
/// view is scrolled.
779-
final PageController controller;
772+
final PageController? controller;
780773

781774
/// How the page view should respond to user input.
782775
///
@@ -848,10 +841,37 @@ class PageView extends StatefulWidget {
848841
class _PageViewState extends State<PageView> {
849842
int _lastReportedPage = 0;
850843

844+
late PageController _controller;
845+
851846
@override
852847
void initState() {
853848
super.initState();
854-
_lastReportedPage = widget.controller.initialPage;
849+
_initController();
850+
_lastReportedPage = _controller.initialPage;
851+
}
852+
853+
@override
854+
void dispose() {
855+
if (widget.controller == null) {
856+
_controller.dispose();
857+
}
858+
super.dispose();
859+
}
860+
861+
862+
void _initController() {
863+
_controller = widget.controller ?? PageController();
864+
}
865+
866+
@override
867+
void didUpdateWidget(PageView oldWidget) {
868+
if (oldWidget.controller != widget.controller) {
869+
if (oldWidget.controller == null) {
870+
_controller.dispose();
871+
}
872+
_initController();
873+
}
874+
super.didUpdateWidget(oldWidget);
855875
}
856876

857877
AxisDirection _getDirection(BuildContext context) {
@@ -892,7 +912,7 @@ class _PageViewState extends State<PageView> {
892912
child: Scrollable(
893913
dragStartBehavior: widget.dragStartBehavior,
894914
axisDirection: axisDirection,
895-
controller: widget.controller,
915+
controller: _controller,
896916
physics: physics,
897917
restorationId: widget.restorationId,
898918
scrollBehavior: widget.scrollBehavior ?? ScrollConfiguration.of(context).copyWith(scrollbars: false),
@@ -908,7 +928,7 @@ class _PageViewState extends State<PageView> {
908928
clipBehavior: widget.clipBehavior,
909929
slivers: <Widget>[
910930
SliverFillViewport(
911-
viewportFraction: widget.controller.viewportFraction,
931+
viewportFraction: _controller.viewportFraction,
912932
delegate: widget.childrenDelegate,
913933
padEnds: widget.padEnds,
914934
),
@@ -924,7 +944,7 @@ class _PageViewState extends State<PageView> {
924944
super.debugFillProperties(description);
925945
description.add(EnumProperty<Axis>('scrollDirection', widget.scrollDirection));
926946
description.add(FlagProperty('reverse', value: widget.reverse, ifTrue: 'reversed'));
927-
description.add(DiagnosticsProperty<PageController>('controller', widget.controller, showName: false));
947+
description.add(DiagnosticsProperty<PageController>('controller', _controller, showName: false));
928948
description.add(DiagnosticsProperty<ScrollPhysics>('physics', widget.physics, showName: false));
929949
description.add(FlagProperty('pageSnapping', value: widget.pageSnapping, ifFalse: 'snapping disabled'));
930950
description.add(FlagProperty('allowImplicitScrolling', value: widget.allowImplicitScrolling, ifTrue: 'allow implicit scrolling'));

‎packages/flutter/test/material/tabs_test.dart

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1482,7 +1482,7 @@ void main() {
14821482
expect(tabController.index, 1);
14831483

14841484
final PageView pageView = tester.widget(find.byType(PageView));
1485-
final PageController pageController = pageView.controller;
1485+
final PageController pageController = pageView.controller!;
14861486
final ScrollPosition position = pageController.position;
14871487

14881488
// The TabBarView's page width is 400, so page 0 is at scroll offset 0.0,
@@ -1531,7 +1531,7 @@ void main() {
15311531
expect(tabController.index, 0);
15321532

15331533
final PageView pageView = tester.widget<PageView>(find.byType(PageView));
1534-
final PageController pageController = pageView.controller;
1534+
final PageController pageController = pageView.controller!;
15351535
final ScrollPosition position = pageController.position;
15361536

15371537
expect(position.pixels, 0.0);
@@ -1592,7 +1592,7 @@ void main() {
15921592
expect(tabController.index, 1);
15931593

15941594
final PageView pageView = tester.widget(find.byType(PageView));
1595-
final PageController pageController = pageView.controller;
1595+
final PageController pageController = pageView.controller!;
15961596

15971597
// The TabView was initialized with viewportFraction as 0.8
15981598
// So it's expected the PageView inside would obtain the same viewportFraction
@@ -1635,7 +1635,7 @@ void main() {
16351635
expect(tabController.index, 1);
16361636

16371637
final PageView pageView = tester.widget(find.byType(PageView));
1638-
final PageController pageController = pageView.controller;
1638+
final PageController pageController = pageView.controller!;
16391639

16401640
// The TabView was initialized with default viewportFraction
16411641
// So it's expected the PageView inside would obtain the value 1
@@ -1680,13 +1680,13 @@ void main() {
16801680

16811681
await tester.pumpWidget(buildFrame(0.8));
16821682
PageView pageView = tester.widget(find.byType(PageView));
1683-
PageController pageController = pageView.controller;
1683+
PageController pageController = pageView.controller!;
16841684
expect(pageController.viewportFraction, 0.8);
16851685

16861686
// Rebuild with a different viewport fraction.
16871687
await tester.pumpWidget(buildFrame(0.5));
16881688
pageView = tester.widget(find.byType(PageView));
1689-
pageController = pageView.controller;
1689+
pageController = pageView.controller!;
16901690
expect(pageController.viewportFraction, 0.5);
16911691
});
16921692

@@ -1861,7 +1861,7 @@ void main() {
18611861
expect(tabController.index, 1);
18621862

18631863
final PageView pageView = tester.widget(find.byType(PageView));
1864-
final PageController pageController = pageView.controller;
1864+
final PageController pageController = pageView.controller!;
18651865
final ScrollPosition position = pageController.position;
18661866

18671867
// The TabBarView's page width is 400, so page 0 is at scroll offset 0.0,
@@ -1905,7 +1905,7 @@ void main() {
19051905
expect(tabController.index, 0);
19061906

19071907
final PageView pageView = tester.widget(find.byType(PageView));
1908-
final PageController pageController = pageView.controller;
1908+
final PageController pageController = pageView.controller!;
19091909
final ScrollPosition position = pageController.position;
19101910

19111911
// The TabBarView's page width is 400, so page 0 is at scroll offset 0.0,
@@ -1950,7 +1950,7 @@ void main() {
19501950
expect(tabController.index, 0);
19511951

19521952
final PageView pageView = tester.widget(find.byType(PageView));
1953-
final PageController pageController = pageView.controller;
1953+
final PageController pageController = pageView.controller!;
19541954
final ScrollPosition position = pageController.position;
19551955

19561956
// The TabBarView's page width is 400, so page 0 is at scroll offset 0.0,
@@ -1999,7 +1999,7 @@ void main() {
19991999
expect(tabController.index, 0);
20002000

20012001
final PageView pageView = tester.widget(find.byType(PageView));
2002-
final PageController pageController = pageView.controller;
2002+
final PageController pageController = pageView.controller!;
20032003
final ScrollPosition position = pageController.position;
20042004

20052005
// The TabBarView's page width is 400, so page 0 is at scroll offset 0.0,
@@ -2050,7 +2050,7 @@ void main() {
20502050
expect(tabController.index, 0);
20512051

20522052
final PageView pageView = tester.widget(find.byType(PageView));
2053-
final PageController pageController = pageView.controller;
2053+
final PageController pageController = pageView.controller!;
20542054
final ScrollPosition position = pageController.position;
20552055

20562056
// The TabBarView's page width is 400, so page 0 is at scroll offset 0.0,
@@ -2246,7 +2246,7 @@ void main() {
22462246
expect(tabController.index, 1);
22472247

22482248
final PageView pageView = tester.widget(find.byType(PageView));
2249-
final PageController pageController = pageView.controller;
2249+
final PageController pageController = pageView.controller!;
22502250
final ScrollPosition position = pageController.position;
22512251

22522252
// The TabBarView's page width is 400, so page 0 is at scroll offset 0.0,
@@ -2432,7 +2432,7 @@ void main() {
24322432
expect(tabController.index, 1);
24332433

24342434
final PageView pageView = tester.widget(find.byType(PageView));
2435-
final PageController pageController = pageView.controller;
2435+
final PageController pageController = pageView.controller!;
24362436
final ScrollPosition position = pageController.position;
24372437

24382438
// The TabBarView's page width is 400, so page 0 is at scroll offset 0.0,
@@ -4208,15 +4208,15 @@ void main() {
42084208

42094209
await tester.pumpWidget(buildFrame(15));
42104210
PageView pageView = tester.widget(find.byType(PageView));
4211-
final PageController pageController1 = pageView.controller;
4211+
final PageController pageController1 = pageView.controller!;
42124212
TabController tabController = DefaultTabController.of(tester.element(find.text('Page 14')));
42134213
expect(tabController.index, 14);
42144214
expect(pageController1.page, 14);
42154215

42164216
// Rebuild with a new default tab controller with more tabs.
42174217
await tester.pumpWidget(buildFrame(10));
42184218
pageView = tester.widget(find.byType(PageView));
4219-
final PageController pageController2 = pageView.controller;
4219+
final PageController pageController2 = pageView.controller!;
42204220
tabController = DefaultTabController.of(tester.element(find.text('Page 9')));
42214221
expect(tabController.index, 9);
42224222
expect(pageController2.page, 9);
@@ -5065,7 +5065,7 @@ void main() {
50655065
double expectedIndicatorLeft = canvas.indicatorRect.left;
50665066

50675067
final PageView pageView = tester.widget(find.byType(PageView));
5068-
final PageController pageController = pageView.controller;
5068+
final PageController pageController = pageView.controller!;
50695069
void pageControllerListener() {
50705070
// Whenever TabBarView scrolls due to changing TabController's index,
50715071
// check if indicator stays idle in its expectedIndicatorLeft
@@ -5225,7 +5225,7 @@ void main() {
52255225
));
52265226

52275227
final PageView pageView = tester.widget(find.byType(PageView));
5228-
final PageController pageController = pageView.controller;
5228+
final PageController pageController = pageView.controller!;
52295229
final ScrollPosition position = pageController.position;
52305230

52315231
expect(tabController.index, 0);
@@ -5715,7 +5715,7 @@ void main() {
57155715

57165716
await tester.pumpWidget(buildFrame(controller1, showLast: true));
57175717
final PageView pageView = tester.widget(find.byType(PageView));
5718-
final PageController pageController = pageView.controller;
5718+
final PageController pageController = pageView.controller!;
57195719
await tester.tap(find.text('three'));
57205720
await tester.pumpAndSettle();
57215721
expect(controller1.index, 2);
@@ -5782,7 +5782,7 @@ void main() {
57825782

57835783
await tester.pumpWidget(buildFrame(controller1, showLast: true));
57845784
PageView pageView = tester.widget(find.byType(PageView));
5785-
PageController pageController = pageView.controller;
5785+
PageController pageController = pageView.controller!;
57865786
await tester.tap(find.text('three'));
57875787
await tester.pumpAndSettle();
57885788
expect(controller1.index, 2);
@@ -5792,15 +5792,15 @@ void main() {
57925792
await tester.pumpWidget(buildFrame(controller2, showLast: false));
57935793
await tester.pumpAndSettle();
57945794
pageView = tester.widget(find.byType(PageView));
5795-
pageController = pageView.controller;
5795+
pageController = pageView.controller!;
57965796
expect(controller2.index, 0);
57975797
expect(pageController.page, 0);
57985798

57995799
// Change TabController back to 'controller1' whose index is 2.
58005800
await tester.pumpWidget(buildFrame(controller1, showLast: true));
58015801
await tester.pumpAndSettle();
58025802
pageView = tester.widget(find.byType(PageView));
5803-
pageController = pageView.controller;
5803+
pageController = pageView.controller!;
58045804
expect(controller1.index, 2);
58055805
expect(pageController.page, 2);
58065806
});

0 commit comments

Comments
 (0)