Skip to content

Commit 78e5d79

Browse files
Paulik8Paulik8
authored
Improved error message when PageController is not attached to PageView (flutter#162422)
Fixes flutter#138945 - Added asserts for jumpToPage and animateToPage methods in PageController (flutter#138945) - Covered by tests <!-- Thanks for filing a pull request! Reviewers are typically assigned within a week of filing a request. To learn more about code review, see our documentation on Tree Hygiene: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md --> *Replace this paragraph with a description of what this PR is changing or adding, and why. Consider including before/after screenshots.* *List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.* *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
1 parent 1d766ed commit 78e5d79

File tree

2 files changed

+171
-0
lines changed

2 files changed

+171
-0
lines changed

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,11 +185,22 @@ class PageController extends ScrollController {
185185
return position.page;
186186
}
187187

188+
bool _debugCheckPageControllerAttached() {
189+
assert(positions.isNotEmpty, 'PageController is not attached to a PageView.');
190+
assert(
191+
positions.length == 1,
192+
'Multiple PageViews are attached to '
193+
'the same PageController.',
194+
);
195+
return true;
196+
}
197+
188198
/// Animates the controlled [PageView] from the current page to the given page.
189199
///
190200
/// The animation lasts for the given duration and follows the given curve.
191201
/// The returned [Future] resolves when the animation completes.
192202
Future<void> animateToPage(int page, {required Duration duration, required Curve curve}) {
203+
assert(_debugCheckPageControllerAttached());
193204
final _PagePosition position = this.position as _PagePosition;
194205
if (position._cachedPage != null) {
195206
position._cachedPage = page.toDouble();
@@ -213,6 +224,7 @@ class PageController extends ScrollController {
213224
/// Jumps the page position from its current value to the given value,
214225
/// without animation, and without checking if the new value is in range.
215226
void jumpToPage(int page) {
227+
assert(_debugCheckPageControllerAttached());
216228
final _PagePosition position = this.position as _PagePosition;
217229
if (position._cachedPage != null) {
218230
position._cachedPage = page.toDouble();
@@ -332,6 +344,7 @@ class _PagePosition extends ScrollPositionWithSingleContext implements PageMetri
332344

333345
final int initialPage;
334346
double _pageToUseOnStartup;
347+
335348
// When the viewport has a zero-size, the `page` can not
336349
// be retrieved by `getPageFromPixels`, so we need to cache the page
337350
// for use when resizing the viewport to non-zero next time.
@@ -362,6 +375,7 @@ class _PagePosition extends ScrollPositionWithSingleContext implements PageMetri
362375
@override
363376
double get viewportFraction => _viewportFraction;
364377
double _viewportFraction;
378+
365379
set viewportFraction(double value) {
366380
if (_viewportFraction == value) {
367381
return;

‎packages/flutter/test/widgets/page_view_test.dart

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1403,6 +1403,163 @@ void main() {
14031403
});
14041404
});
14051405

1406+
group('Asserts in jumpToPage and animateToPage methods works properly', () {
1407+
Widget createPageView([PageController? controller]) {
1408+
return MaterialApp(
1409+
home: Scaffold(
1410+
body: PageView(
1411+
controller: controller,
1412+
children: <Widget>[
1413+
Container(color: Colors.red),
1414+
Container(color: Colors.green),
1415+
Container(color: Colors.blue),
1416+
],
1417+
),
1418+
),
1419+
);
1420+
}
1421+
1422+
group('One pageController is attached to multiple PageViews', () {
1423+
Widget createMultiplePageViews(PageController controller) {
1424+
return MaterialApp(
1425+
home: Scaffold(
1426+
body: Column(
1427+
children: <Widget>[
1428+
Expanded(
1429+
child: PageView(
1430+
controller: controller,
1431+
children: <Widget>[
1432+
Container(color: Colors.red),
1433+
Container(color: Colors.green),
1434+
Container(color: Colors.blue),
1435+
],
1436+
),
1437+
),
1438+
Expanded(
1439+
child: PageView(
1440+
controller: controller,
1441+
children: <Widget>[
1442+
Container(color: Colors.orange),
1443+
Container(color: Colors.purple),
1444+
Container(color: Colors.yellow),
1445+
],
1446+
),
1447+
),
1448+
],
1449+
),
1450+
),
1451+
);
1452+
}
1453+
1454+
testWidgets(
1455+
'animateToPage assertion is working properly when pageController is attached to multiple PageViews',
1456+
(WidgetTester tester) async {
1457+
final PageController controller = PageController();
1458+
addTearDown(controller.dispose);
1459+
await tester.pumpWidget(createMultiplePageViews(controller));
1460+
1461+
expect(
1462+
() => controller.animateToPage(
1463+
2,
1464+
duration: const Duration(milliseconds: 300),
1465+
curve: Curves.ease,
1466+
),
1467+
throwsA(
1468+
isAssertionError.having(
1469+
(AssertionError error) => error.message,
1470+
'message',
1471+
equals(
1472+
'Multiple PageViews are attached to '
1473+
'the same PageController.',
1474+
),
1475+
),
1476+
),
1477+
);
1478+
},
1479+
);
1480+
1481+
testWidgets(
1482+
'jumpToPage assertion is working properly when pageController is attached to multiple PageViews',
1483+
(WidgetTester tester) async {
1484+
final PageController controller = PageController();
1485+
addTearDown(controller.dispose);
1486+
await tester.pumpWidget(createMultiplePageViews(controller));
1487+
1488+
expect(
1489+
() => controller.jumpToPage(2),
1490+
throwsA(
1491+
isAssertionError.having(
1492+
(AssertionError error) => error.message,
1493+
'message',
1494+
equals(
1495+
'Multiple PageViews are attached to '
1496+
'the same PageController.',
1497+
),
1498+
),
1499+
),
1500+
);
1501+
},
1502+
);
1503+
});
1504+
1505+
group('PageController is attached or is not attached to PageView', () {
1506+
testWidgets('Assert behavior of animateToPage works properly', (WidgetTester tester) async {
1507+
final PageController controller = PageController();
1508+
addTearDown(controller.dispose);
1509+
1510+
// pageController is not attached to PageView
1511+
await tester.pumpWidget(createPageView());
1512+
expect(
1513+
() => controller.animateToPage(
1514+
2,
1515+
duration: const Duration(milliseconds: 300),
1516+
curve: Curves.ease,
1517+
),
1518+
throwsA(
1519+
isAssertionError.having(
1520+
(AssertionError error) => error.message,
1521+
'message',
1522+
equals('PageController is not attached to a PageView.'),
1523+
),
1524+
),
1525+
);
1526+
1527+
// pageController is attached to PageView
1528+
await tester.pumpWidget(createPageView(controller));
1529+
expect(
1530+
() => controller.animateToPage(
1531+
2,
1532+
duration: const Duration(milliseconds: 300),
1533+
curve: Curves.ease,
1534+
),
1535+
returnsNormally,
1536+
);
1537+
});
1538+
1539+
testWidgets('Assert behavior of jumpToPage works properly', (WidgetTester tester) async {
1540+
final PageController controller = PageController();
1541+
addTearDown(controller.dispose);
1542+
1543+
// pageController is not attached to PageView
1544+
await tester.pumpWidget(createPageView());
1545+
expect(
1546+
() => controller.jumpToPage(2),
1547+
throwsA(
1548+
isAssertionError.having(
1549+
(AssertionError error) => error.message,
1550+
'message',
1551+
equals('PageController is not attached to a PageView.'),
1552+
),
1553+
),
1554+
);
1555+
1556+
// pageController is attached to PageView
1557+
await tester.pumpWidget(createPageView(controller));
1558+
expect(() => controller.jumpToPage(2), returnsNormally);
1559+
});
1560+
});
1561+
});
1562+
14061563
testWidgets(
14071564
'Get the page value before the content dimension is determined,do not throw an assertion and return null',
14081565
(WidgetTester tester) async {

0 commit comments

Comments
 (0)