Improved error message when PageController is not attached to PageView (#162422)

Fixes https://github.com/flutter/flutter/issues/138945
- Added asserts for jumpToPage and animateToPage methods in
PageController (https://github.com/flutter/flutter/issues/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
This commit is contained in:
Paulik8 2025-02-10 18:39:06 +01:00 committed by GitHub
parent 1d766ed8d9
commit 78e5d7995c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 171 additions and 0 deletions

View File

@ -185,11 +185,22 @@ class PageController extends ScrollController {
return position.page; return position.page;
} }
bool _debugCheckPageControllerAttached() {
assert(positions.isNotEmpty, 'PageController is not attached to a PageView.');
assert(
positions.length == 1,
'Multiple PageViews are attached to '
'the same PageController.',
);
return true;
}
/// Animates the controlled [PageView] from the current page to the given page. /// Animates the controlled [PageView] from the current page to the given page.
/// ///
/// The animation lasts for the given duration and follows the given curve. /// The animation lasts for the given duration and follows the given curve.
/// The returned [Future] resolves when the animation completes. /// The returned [Future] resolves when the animation completes.
Future<void> animateToPage(int page, {required Duration duration, required Curve curve}) { Future<void> animateToPage(int page, {required Duration duration, required Curve curve}) {
assert(_debugCheckPageControllerAttached());
final _PagePosition position = this.position as _PagePosition; final _PagePosition position = this.position as _PagePosition;
if (position._cachedPage != null) { if (position._cachedPage != null) {
position._cachedPage = page.toDouble(); position._cachedPage = page.toDouble();
@ -213,6 +224,7 @@ class PageController extends ScrollController {
/// Jumps the page position from its current value to the given value, /// Jumps the page position from its current value to the given value,
/// without animation, and without checking if the new value is in range. /// without animation, and without checking if the new value is in range.
void jumpToPage(int page) { void jumpToPage(int page) {
assert(_debugCheckPageControllerAttached());
final _PagePosition position = this.position as _PagePosition; final _PagePosition position = this.position as _PagePosition;
if (position._cachedPage != null) { if (position._cachedPage != null) {
position._cachedPage = page.toDouble(); position._cachedPage = page.toDouble();
@ -332,6 +344,7 @@ class _PagePosition extends ScrollPositionWithSingleContext implements PageMetri
final int initialPage; final int initialPage;
double _pageToUseOnStartup; double _pageToUseOnStartup;
// When the viewport has a zero-size, the `page` can not // When the viewport has a zero-size, the `page` can not
// be retrieved by `getPageFromPixels`, so we need to cache the page // be retrieved by `getPageFromPixels`, so we need to cache the page
// for use when resizing the viewport to non-zero next time. // for use when resizing the viewport to non-zero next time.
@ -362,6 +375,7 @@ class _PagePosition extends ScrollPositionWithSingleContext implements PageMetri
@override @override
double get viewportFraction => _viewportFraction; double get viewportFraction => _viewportFraction;
double _viewportFraction; double _viewportFraction;
set viewportFraction(double value) { set viewportFraction(double value) {
if (_viewportFraction == value) { if (_viewportFraction == value) {
return; return;

View File

@ -1403,6 +1403,163 @@ void main() {
}); });
}); });
group('Asserts in jumpToPage and animateToPage methods works properly', () {
Widget createPageView([PageController? controller]) {
return MaterialApp(
home: Scaffold(
body: PageView(
controller: controller,
children: <Widget>[
Container(color: Colors.red),
Container(color: Colors.green),
Container(color: Colors.blue),
],
),
),
);
}
group('One pageController is attached to multiple PageViews', () {
Widget createMultiplePageViews(PageController controller) {
return MaterialApp(
home: Scaffold(
body: Column(
children: <Widget>[
Expanded(
child: PageView(
controller: controller,
children: <Widget>[
Container(color: Colors.red),
Container(color: Colors.green),
Container(color: Colors.blue),
],
),
),
Expanded(
child: PageView(
controller: controller,
children: <Widget>[
Container(color: Colors.orange),
Container(color: Colors.purple),
Container(color: Colors.yellow),
],
),
),
],
),
),
);
}
testWidgets(
'animateToPage assertion is working properly when pageController is attached to multiple PageViews',
(WidgetTester tester) async {
final PageController controller = PageController();
addTearDown(controller.dispose);
await tester.pumpWidget(createMultiplePageViews(controller));
expect(
() => controller.animateToPage(
2,
duration: const Duration(milliseconds: 300),
curve: Curves.ease,
),
throwsA(
isAssertionError.having(
(AssertionError error) => error.message,
'message',
equals(
'Multiple PageViews are attached to '
'the same PageController.',
),
),
),
);
},
);
testWidgets(
'jumpToPage assertion is working properly when pageController is attached to multiple PageViews',
(WidgetTester tester) async {
final PageController controller = PageController();
addTearDown(controller.dispose);
await tester.pumpWidget(createMultiplePageViews(controller));
expect(
() => controller.jumpToPage(2),
throwsA(
isAssertionError.having(
(AssertionError error) => error.message,
'message',
equals(
'Multiple PageViews are attached to '
'the same PageController.',
),
),
),
);
},
);
});
group('PageController is attached or is not attached to PageView', () {
testWidgets('Assert behavior of animateToPage works properly', (WidgetTester tester) async {
final PageController controller = PageController();
addTearDown(controller.dispose);
// pageController is not attached to PageView
await tester.pumpWidget(createPageView());
expect(
() => controller.animateToPage(
2,
duration: const Duration(milliseconds: 300),
curve: Curves.ease,
),
throwsA(
isAssertionError.having(
(AssertionError error) => error.message,
'message',
equals('PageController is not attached to a PageView.'),
),
),
);
// pageController is attached to PageView
await tester.pumpWidget(createPageView(controller));
expect(
() => controller.animateToPage(
2,
duration: const Duration(milliseconds: 300),
curve: Curves.ease,
),
returnsNormally,
);
});
testWidgets('Assert behavior of jumpToPage works properly', (WidgetTester tester) async {
final PageController controller = PageController();
addTearDown(controller.dispose);
// pageController is not attached to PageView
await tester.pumpWidget(createPageView());
expect(
() => controller.jumpToPage(2),
throwsA(
isAssertionError.having(
(AssertionError error) => error.message,
'message',
equals('PageController is not attached to a PageView.'),
),
),
);
// pageController is attached to PageView
await tester.pumpWidget(createPageView(controller));
expect(() => controller.jumpToPage(2), returnsNormally);
});
});
});
testWidgets( testWidgets(
'Get the page value before the content dimension is determined,do not throw an assertion and return null', 'Get the page value before the content dimension is determined,do not throw an assertion and return null',
(WidgetTester tester) async { (WidgetTester tester) async {