Skip to content

Commit

Permalink
Reverts "Remove hack from PageView." (#141479)
Browse files Browse the repository at this point in the history 
Reverts flutter/flutter#141138
Initiated by: itsjustkevin
This change reverts the following previous change:
Original Description:
Fixes flutter/flutter#141119

The change is breaking, because now controller is nullable.

Migration path: flutter/website#10033

Packages to fix:
  • Loading branch information
@auto-submit
auto-submit[bot] authored Jan 12, 2024
1 parent 8bab8a4 commit 8f797fc
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 134 deletions.
2 changes: 1 addition & 1 deletion examples/api/test/widgets/page_view/page_view.0_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ void main() {

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

// Tap backward button on page indicator area.
await tester.tap(find.byIcon(Icons.arrow_left_rounded));
Expand Down
58 changes: 19 additions & 39 deletions packages/flutter/lib/src/widgets/page_view.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,11 @@ class PageScrollPhysics extends ScrollPhysics {
bool get allowImplicitScrolling => false;
}

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

/// A scrollable list that works page by page.
Expand Down Expand Up @@ -640,7 +645,7 @@ class PageView extends StatefulWidget {
super.key,
this.scrollDirection = Axis.horizontal,
this.reverse = false,
this.controller,
PageController? controller,
this.physics,
this.pageSnapping = true,
this.onPageChanged,
Expand All @@ -651,7 +656,8 @@ class PageView extends StatefulWidget {
this.clipBehavior = Clip.hardEdge,
this.scrollBehavior,
this.padEnds = true,
}) : childrenDelegate = SliverChildListDelegate(children);
}) : controller = controller ?? _defaultPageController,
childrenDelegate = SliverChildListDelegate(children);

/// Creates a scrollable list that works page by page using widgets that are
/// created on demand.
Expand Down Expand Up @@ -682,7 +688,7 @@ class PageView extends StatefulWidget {
super.key,
this.scrollDirection = Axis.horizontal,
this.reverse = false,
this.controller,
PageController? controller,
this.physics,
this.pageSnapping = true,
this.onPageChanged,
Expand All @@ -695,7 +701,8 @@ class PageView extends StatefulWidget {
this.clipBehavior = Clip.hardEdge,
this.scrollBehavior,
this.padEnds = true,
}) : childrenDelegate = SliverChildBuilderDelegate(
}) : controller = controller ?? _defaultPageController,
childrenDelegate = SliverChildBuilderDelegate(
itemBuilder,
findChildIndexCallback: findChildIndexCallback,
childCount: itemCount,
Expand All @@ -712,11 +719,11 @@ class PageView extends StatefulWidget {
/// {@end-tool}
///
/// {@macro flutter.widgets.PageView.allowImplicitScrolling}
const PageView.custom({
PageView.custom({
super.key,
this.scrollDirection = Axis.horizontal,
this.reverse = false,
this.controller,
PageController? controller,
this.physics,
this.pageSnapping = true,
this.onPageChanged,
Expand All @@ -727,7 +734,7 @@ class PageView extends StatefulWidget {
this.clipBehavior = Clip.hardEdge,
this.scrollBehavior,
this.padEnds = true,
});
}) : controller = controller ?? _defaultPageController;

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

/// An object that can be used to control the position to which this page
/// view is scrolled.
final PageController? controller;
final PageController controller;

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

late PageController _controller;

@override
void initState() {
super.initState();
_initController();
_lastReportedPage = _controller.initialPage;
}

@override
void dispose() {
if (widget.controller == null) {
_controller.dispose();
}
super.dispose();
}


void _initController() {
_controller = widget.controller ?? PageController();
}

@override
void didUpdateWidget(PageView oldWidget) {
if (oldWidget.controller != widget.controller) {
if (oldWidget.controller == null) {
_controller.dispose();
}
_initController();
}
super.didUpdateWidget(oldWidget);
_lastReportedPage = widget.controller.initialPage;
}

AxisDirection _getDirection(BuildContext context) {
Expand Down Expand Up @@ -912,7 +892,7 @@ class _PageViewState extends State<PageView> {
child: Scrollable(
dragStartBehavior: widget.dragStartBehavior,
axisDirection: axisDirection,
controller: _controller,
controller: widget.controller,
physics: physics,
restorationId: widget.restorationId,
scrollBehavior: widget.scrollBehavior ?? ScrollConfiguration.of(context).copyWith(scrollbars: false),
Expand All @@ -928,7 +908,7 @@ class _PageViewState extends State<PageView> {
clipBehavior: widget.clipBehavior,
slivers: <Widget>[
SliverFillViewport(
viewportFraction: _controller.viewportFraction,
viewportFraction: widget.controller.viewportFraction,
delegate: widget.childrenDelegate,
padEnds: widget.padEnds,
),
Expand All @@ -944,7 +924,7 @@ class _PageViewState extends State<PageView> {
super.debugFillProperties(description);
description.add(EnumProperty<Axis>('scrollDirection', widget.scrollDirection));
description.add(FlagProperty('reverse', value: widget.reverse, ifTrue: 'reversed'));
description.add(DiagnosticsProperty<PageController>('controller', _controller, showName: false));
description.add(DiagnosticsProperty<PageController>('controller', widget.controller, showName: false));
description.add(DiagnosticsProperty<ScrollPhysics>('physics', widget.physics, showName: false));
description.add(FlagProperty('pageSnapping', value: widget.pageSnapping, ifFalse: 'snapping disabled'));
description.add(FlagProperty('allowImplicitScrolling', value: widget.allowImplicitScrolling, ifTrue: 'allow implicit scrolling'));
Expand Down
42 changes: 21 additions & 21 deletions packages/flutter/test/material/tabs_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1482,7 +1482,7 @@ void main() {
expect(tabController.index, 1);

final PageView pageView = tester.widget(find.byType(PageView));
final PageController pageController = pageView.controller!;
final PageController pageController = pageView.controller;
final ScrollPosition position = pageController.position;

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

final PageView pageView = tester.widget<PageView>(find.byType(PageView));
final PageController pageController = pageView.controller!;
final PageController pageController = pageView.controller;
final ScrollPosition position = pageController.position;

expect(position.pixels, 0.0);
Expand Down Expand Up @@ -1592,7 +1592,7 @@ void main() {
expect(tabController.index, 1);

final PageView pageView = tester.widget(find.byType(PageView));
final PageController pageController = pageView.controller!;
final PageController pageController = pageView.controller;

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

final PageView pageView = tester.widget(find.byType(PageView));
final PageController pageController = pageView.controller!;
final PageController pageController = pageView.controller;

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

await tester.pumpWidget(buildFrame(0.8));
PageView pageView = tester.widget(find.byType(PageView));
PageController pageController = pageView.controller!;
PageController pageController = pageView.controller;
expect(pageController.viewportFraction, 0.8);

// Rebuild with a different viewport fraction.
await tester.pumpWidget(buildFrame(0.5));
pageView = tester.widget(find.byType(PageView));
pageController = pageView.controller!;
pageController = pageView.controller;
expect(pageController.viewportFraction, 0.5);
});

Expand Down Expand Up @@ -1861,7 +1861,7 @@ void main() {
expect(tabController.index, 1);

final PageView pageView = tester.widget(find.byType(PageView));
final PageController pageController = pageView.controller!;
final PageController pageController = pageView.controller;
final ScrollPosition position = pageController.position;

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

final PageView pageView = tester.widget(find.byType(PageView));
final PageController pageController = pageView.controller!;
final PageController pageController = pageView.controller;
final ScrollPosition position = pageController.position;

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

final PageView pageView = tester.widget(find.byType(PageView));
final PageController pageController = pageView.controller!;
final PageController pageController = pageView.controller;
final ScrollPosition position = pageController.position;

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

final PageView pageView = tester.widget(find.byType(PageView));
final PageController pageController = pageView.controller!;
final PageController pageController = pageView.controller;
final ScrollPosition position = pageController.position;

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

final PageView pageView = tester.widget(find.byType(PageView));
final PageController pageController = pageView.controller!;
final PageController pageController = pageView.controller;
final ScrollPosition position = pageController.position;

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

final PageView pageView = tester.widget(find.byType(PageView));
final PageController pageController = pageView.controller!;
final PageController pageController = pageView.controller;
final ScrollPosition position = pageController.position;

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

final PageView pageView = tester.widget(find.byType(PageView));
final PageController pageController = pageView.controller!;
final PageController pageController = pageView.controller;
final ScrollPosition position = pageController.position;

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

await tester.pumpWidget(buildFrame(15));
PageView pageView = tester.widget(find.byType(PageView));
final PageController pageController1 = pageView.controller!;
final PageController pageController1 = pageView.controller;
TabController tabController = DefaultTabController.of(tester.element(find.text('Page 14')));
expect(tabController.index, 14);
expect(pageController1.page, 14);

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

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

final PageView pageView = tester.widget(find.byType(PageView));
final PageController pageController = pageView.controller!;
final PageController pageController = pageView.controller;
final ScrollPosition position = pageController.position;

expect(tabController.index, 0);
Expand Down Expand Up @@ -5715,7 +5715,7 @@ void main() {

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

await tester.pumpWidget(buildFrame(controller1, showLast: true));
PageView pageView = tester.widget(find.byType(PageView));
PageController pageController = pageView.controller!;
PageController pageController = pageView.controller;
await tester.tap(find.text('three'));
await tester.pumpAndSettle();
expect(controller1.index, 2);
Expand All @@ -5792,15 +5792,15 @@ void main() {
await tester.pumpWidget(buildFrame(controller2, showLast: false));
await tester.pumpAndSettle();
pageView = tester.widget(find.byType(PageView));
pageController = pageView.controller!;
pageController = pageView.controller;
expect(controller2.index, 0);
expect(pageController.page, 0);

// Change TabController back to 'controller1' whose index is 2.
await tester.pumpWidget(buildFrame(controller1, showLast: true));
await tester.pumpAndSettle();
pageView = tester.widget(find.byType(PageView));
pageController = pageView.controller!;
pageController = pageView.controller;
expect(controller1.index, 2);
expect(pageController.page, 2);
});
Expand Down
Loading

0 comments on commit 8f797fc

Please sign in to comment.