- Notifications
You must be signed in to change notification settings - Fork30k
Adds animateToItem to the CarouselController#162694
Merged
auto-submit[bot] merged 22 commits intoflutter:masterfromMar 5, 2025
Merged
Adds animateToItem to the CarouselController#162694auto-submit[bot] merged 22 commits intoflutter:masterfrom
auto-submit[bot] merged 22 commits intoflutter:masterfrom
Conversation
This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again. For more guidance, visitWriting a golden file test for Reviewers: Read theTree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
Contributor
QuncCccccc left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks so much for adding this new feature! Overall this looks great. Just left some thoughts below:)
| double _getTargetOffset(_CarouselPosition position, int index, bool hasFlexWeights) { | ||
| if (!hasFlexWeights) { | ||
| return index * _carouselState!._itemExtent!; |
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Should we constrain the index to between [0, children.length - 1]?
ContributorAuthor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Although it doesn't cause an error, it results in a large scroll before returning to the last valid position. So yes, we can constrain the index.
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes, we might want to assert that the index is in range.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
| final int totalWeight = weights.reduce((int a, int b) => a + b); | ||
| final double dimension = position.viewportDimension; | ||
| final bool isLastItem = index == carouselState.widget.children.length - 1; |
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I actually think only controlling the last item is not enough. When consumeMaxWeight is false, the behavior animating to other items is still different than when consumeMaxWeight is true.
Screen.Recording.2025-02-13.at.2.58.44.PM.mov
ContributorAuthor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I made a small change to the logic, but I feel like I missed something.
4c4e849 to3f923d1Compare| expect(realOffset(), controller.offset); | ||
| }); | ||
| testWidgets('CarouselView animateToItem with flexWeights', (WidgetTester tester) async { |
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
There should be several variants of this test, with different flex weights and numbers of children.
ContributorAuthor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I just submitted several tests to cover various scenarios. If there is any missing, I will send it.
Contributor
Piinks left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Hey@Mairramer thanks for sending another PR! Some questions I wonder on this, how does setting a shrinkOffset affect this? Does this work when itemSnapping is true?
Uh oh!
There was an error while loading.Please reload this page.
ContributorAuthor
Mairramer commentedFeb 14, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Yes, works with itemSnapping. |
| } | ||
| final bool hasFlexWeights = _carouselState!._flexWeights?.isNotEmpty ?? false; | ||
| index = index.clamp(0, _carouselState!.widget.children.length - 1); |
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
NICE! Thanks!
Uh oh!
There was an error while loading.Please reload this page.
QuncCccccc approved these changesFeb 26, 2025
Contributor
QuncCccccc left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM:) Thanks for adding the new feature!
Uh oh!
There was an error while loading.Please reload this page.
Piinks approved these changesMar 5, 2025
Contributor
Piinks left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This also LGTM, thank you!
Merged via the queue intoflutter:master with commitMar 5, 2025
40e4c5e 74 of 75 checks passed
Uh oh!
There was an error while loading.Please reload this page.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull requestMar 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull requestMar 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull requestMar 6, 2025
auto-submitbot pushed a commit to flutter/packages that referenced this pull requestMar 6, 2025
flutter/flutter@2578d97...321fbc02025-03-06 engine-flutter-autoroll@skia.org Roll Skia from fefecd49e03a to ccd8cc23aa94 (1 revision) (flutter/flutter#164712)2025-03-06 mdebbar@google.com [web] Detect scrollable semantics nodes more reliably (flutter/flutter#164491)2025-03-06 matt.kosarek@canonical.com [windows] wire the focus request and the focus events through the Windows platform (flutter/flutter#164296)2025-03-06 engine-flutter-autoroll@skia.org Roll Skia from 02897747c7d5 to fefecd49e03a (1 revision) (flutter/flutter#164701)2025-03-06 engine-flutter-autoroll@skia.org Roll Skia from e315b0ab7c84 to 02897747c7d5 (1 revision) (flutter/flutter#164677)2025-03-06 engine-flutter-autoroll@skia.org Roll Skia from 0c3880f94970 to e315b0ab7c84 (1 revision) (flutter/flutter#164669)2025-03-06 jonahwilliams@google.com [Impeller] use device private on non-iOS devices. (flutter/flutter#164601)2025-03-05 engine-flutter-autoroll@skia.org Roll Skia from 43294a662fd0 to 0c3880f94970 (1 revision) (flutter/flutter#164661)2025-03-05 codefu@google.com Add a workflow (only triggered from rest events) for hasing experiment (flutter/flutter#164657)2025-03-05 engine-flutter-autoroll@skia.org Roll Skia from 4cf9f0b77d41 to 43294a662fd0 (4 revisions) (flutter/flutter#164649)2025-03-05 50643541+Mairramer@users.noreply.github.com Adds animateToItem to the CarouselController (flutter/flutter#162694)2025-03-05 30870216+gaaclarke@users.noreply.github.com Cleanup content context (flutter/flutter#164229)2025-03-05 34465683+rkishan516@users.noreply.github.com Fix: Update CupertinoSheetRoute transition rounded corner (flutter/flutter#163700)2025-03-05 jonahwilliams@google.com [Impeller] fix macOS managed memory. (flutter/flutter#164635)2025-03-05 jacksongardner@google.com [skwasm] Clear font collection cache when font is loaded manually. (flutter/flutter#164588)2025-03-05 victorsanniay@gmail.com Fix race condition causing crash when interacting with an animating scrollable (flutter/flutter#164392)2025-03-05 58529443+srujzs@users.noreply.github.com Use dwds 24.3.6 and pass uri for the reload scripts path to FrontendServerDdcLibraryBundleProvider (flutter/flutter#164582)2025-03-05 engine-flutter-autoroll@skia.org Roll Packages from9e4684e toabba683 (8 revisions) (flutter/flutter#164630)2025-03-05 engine-flutter-autoroll@skia.org Roll Skia from 7e4323f72c9d to 4cf9f0b77d41 (1 revision) (flutter/flutter#164622)2025-03-05 harri.kirik@lab.mobi Fix to Linux_pixel_7pro integration_ui_keyboard_resize test flakiness (flutter/flutter#162308)2025-03-05 engine-flutter-autoroll@skia.org Roll Skia from 03a3f653d64e to 7e4323f72c9d (1 revision) (flutter/flutter#164599)2025-03-05 34871572+gmackall@users.noreply.github.com Implement `clipPath` Mutator for hcpp (flutter/flutter#164525)2025-03-05 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] use DeviceLocal textures for gifs on non-iOS devices. (#164573)" (flutter/flutter#164600)2025-03-05 jonahwilliams@google.com [macos] prefer integrated GPU. (flutter/flutter#164569)2025-03-05 muhatashim@google.com Enforce minSdk constraint for Android Flutter (flutter/flutter#164251)2025-03-05 dkwingsmt@users.noreply.github.com Add `clipRSuperellipse`, and use them for dialogs (flutter/flutter#161111)2025-03-05 engine-flutter-autoroll@skia.org Roll Skia from 46705a22edc3 to 03a3f653d64e (1 revision) (flutter/flutter#164590)2025-03-05 psturm@esri.com when resetting FlutterPlatformViewsController, clear out some additional internal state to prevent it from carrying over across a Hot Restart (flutter/flutter#164456)2025-03-05 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from Rt6pxGFLVAJHduM0V... to fhm5z889sA5T1AQao... (flutter/flutter#164583)If this roll has caused a breakage, revert this CL and stop the rollerusing the controls here:https://autoroll.skia.org/r/flutter-packagesPlease CC stuartmorgan@google.com,tarrinneal@google.com on the revert to ensure that a humanis aware of the problem.To file a bug in Packages:https://github.com/flutter/flutter/issues/new/chooseTo report a problem with the AutoRoller itself, please file a bug:https://issues.skia.org/issues/new?component=1389291&template=1850622Documentation for the AutoRoller is here:https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
9 tasks
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull requestMay 20, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull requestMay 20, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull requestMay 21, 2025
androidseb pushed a commit to androidseb/packages that referenced this pull requestJun 8, 2025
flutter/flutter@2578d97...321fbc02025-03-06 engine-flutter-autoroll@skia.org Roll Skia from fefecd49e03a to ccd8cc23aa94 (1 revision) (flutter/flutter#164712)2025-03-06 mdebbar@google.com [web] Detect scrollable semantics nodes more reliably (flutter/flutter#164491)2025-03-06 matt.kosarek@canonical.com [windows] wire the focus request and the focus events through the Windows platform (flutter/flutter#164296)2025-03-06 engine-flutter-autoroll@skia.org Roll Skia from 02897747c7d5 to fefecd49e03a (1 revision) (flutter/flutter#164701)2025-03-06 engine-flutter-autoroll@skia.org Roll Skia from e315b0ab7c84 to 02897747c7d5 (1 revision) (flutter/flutter#164677)2025-03-06 engine-flutter-autoroll@skia.org Roll Skia from 0c3880f94970 to e315b0ab7c84 (1 revision) (flutter/flutter#164669)2025-03-06 jonahwilliams@google.com [Impeller] use device private on non-iOS devices. (flutter/flutter#164601)2025-03-05 engine-flutter-autoroll@skia.org Roll Skia from 43294a662fd0 to 0c3880f94970 (1 revision) (flutter/flutter#164661)2025-03-05 codefu@google.com Add a workflow (only triggered from rest events) for hasing experiment (flutter/flutter#164657)2025-03-05 engine-flutter-autoroll@skia.org Roll Skia from 4cf9f0b77d41 to 43294a662fd0 (4 revisions) (flutter/flutter#164649)2025-03-05 50643541+Mairramer@users.noreply.github.com Adds animateToItem to the CarouselController (flutter/flutter#162694)2025-03-05 30870216+gaaclarke@users.noreply.github.com Cleanup content context (flutter/flutter#164229)2025-03-05 34465683+rkishan516@users.noreply.github.com Fix: Update CupertinoSheetRoute transition rounded corner (flutter/flutter#163700)2025-03-05 jonahwilliams@google.com [Impeller] fix macOS managed memory. (flutter/flutter#164635)2025-03-05 jacksongardner@google.com [skwasm] Clear font collection cache when font is loaded manually. (flutter/flutter#164588)2025-03-05 victorsanniay@gmail.com Fix race condition causing crash when interacting with an animating scrollable (flutter/flutter#164392)2025-03-05 58529443+srujzs@users.noreply.github.com Use dwds 24.3.6 and pass uri for the reload scripts path to FrontendServerDdcLibraryBundleProvider (flutter/flutter#164582)2025-03-05 engine-flutter-autoroll@skia.org Roll Packages from9e4684e toabba683 (8 revisions) (flutter/flutter#164630)2025-03-05 engine-flutter-autoroll@skia.org Roll Skia from 7e4323f72c9d to 4cf9f0b77d41 (1 revision) (flutter/flutter#164622)2025-03-05 harri.kirik@lab.mobi Fix to Linux_pixel_7pro integration_ui_keyboard_resize test flakiness (flutter/flutter#162308)2025-03-05 engine-flutter-autoroll@skia.org Roll Skia from 03a3f653d64e to 7e4323f72c9d (1 revision) (flutter/flutter#164599)2025-03-05 34871572+gmackall@users.noreply.github.com Implement `clipPath` Mutator for hcpp (flutter/flutter#164525)2025-03-05 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] use DeviceLocal textures for gifs on non-iOS devices. (#164573)" (flutter/flutter#164600)2025-03-05 jonahwilliams@google.com [macos] prefer integrated GPU. (flutter/flutter#164569)2025-03-05 muhatashim@google.com Enforce minSdk constraint for Android Flutter (flutter/flutter#164251)2025-03-05 dkwingsmt@users.noreply.github.com Add `clipRSuperellipse`, and use them for dialogs (flutter/flutter#161111)2025-03-05 engine-flutter-autoroll@skia.org Roll Skia from 46705a22edc3 to 03a3f653d64e (1 revision) (flutter/flutter#164590)2025-03-05 psturm@esri.com when resetting FlutterPlatformViewsController, clear out some additional internal state to prevent it from carrying over across a Hot Restart (flutter/flutter#164456)2025-03-05 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from Rt6pxGFLVAJHduM0V... to fhm5z889sA5T1AQao... (flutter/flutter#164583)If this roll has caused a breakage, revert this CL and stop the rollerusing the controls here:https://autoroll.skia.org/r/flutter-packagesPlease CC stuartmorgan@google.com,tarrinneal@google.com on the revert to ensure that a humanis aware of the problem.To file a bug in Packages:https://github.com/flutter/flutter/issues/new/chooseTo report a problem with the AutoRoller itself, please file a bug:https://issues.skia.org/issues/new?component=1389291&template=1850622Documentation for the AutoRoller is here:https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
FMorschel pushed a commit to FMorschel/packages that referenced this pull requestJun 9, 2025
flutter/flutter@2578d97...321fbc02025-03-06 engine-flutter-autoroll@skia.org Roll Skia from fefecd49e03a to ccd8cc23aa94 (1 revision) (flutter/flutter#164712)2025-03-06 mdebbar@google.com [web] Detect scrollable semantics nodes more reliably (flutter/flutter#164491)2025-03-06 matt.kosarek@canonical.com [windows] wire the focus request and the focus events through the Windows platform (flutter/flutter#164296)2025-03-06 engine-flutter-autoroll@skia.org Roll Skia from 02897747c7d5 to fefecd49e03a (1 revision) (flutter/flutter#164701)2025-03-06 engine-flutter-autoroll@skia.org Roll Skia from e315b0ab7c84 to 02897747c7d5 (1 revision) (flutter/flutter#164677)2025-03-06 engine-flutter-autoroll@skia.org Roll Skia from 0c3880f94970 to e315b0ab7c84 (1 revision) (flutter/flutter#164669)2025-03-06 jonahwilliams@google.com [Impeller] use device private on non-iOS devices. (flutter/flutter#164601)2025-03-05 engine-flutter-autoroll@skia.org Roll Skia from 43294a662fd0 to 0c3880f94970 (1 revision) (flutter/flutter#164661)2025-03-05 codefu@google.com Add a workflow (only triggered from rest events) for hasing experiment (flutter/flutter#164657)2025-03-05 engine-flutter-autoroll@skia.org Roll Skia from 4cf9f0b77d41 to 43294a662fd0 (4 revisions) (flutter/flutter#164649)2025-03-05 50643541+Mairramer@users.noreply.github.com Adds animateToItem to the CarouselController (flutter/flutter#162694)2025-03-05 30870216+gaaclarke@users.noreply.github.com Cleanup content context (flutter/flutter#164229)2025-03-05 34465683+rkishan516@users.noreply.github.com Fix: Update CupertinoSheetRoute transition rounded corner (flutter/flutter#163700)2025-03-05 jonahwilliams@google.com [Impeller] fix macOS managed memory. (flutter/flutter#164635)2025-03-05 jacksongardner@google.com [skwasm] Clear font collection cache when font is loaded manually. (flutter/flutter#164588)2025-03-05 victorsanniay@gmail.com Fix race condition causing crash when interacting with an animating scrollable (flutter/flutter#164392)2025-03-05 58529443+srujzs@users.noreply.github.com Use dwds 24.3.6 and pass uri for the reload scripts path to FrontendServerDdcLibraryBundleProvider (flutter/flutter#164582)2025-03-05 engine-flutter-autoroll@skia.org Roll Packages from9e4684e toabba683 (8 revisions) (flutter/flutter#164630)2025-03-05 engine-flutter-autoroll@skia.org Roll Skia from 7e4323f72c9d to 4cf9f0b77d41 (1 revision) (flutter/flutter#164622)2025-03-05 harri.kirik@lab.mobi Fix to Linux_pixel_7pro integration_ui_keyboard_resize test flakiness (flutter/flutter#162308)2025-03-05 engine-flutter-autoroll@skia.org Roll Skia from 03a3f653d64e to 7e4323f72c9d (1 revision) (flutter/flutter#164599)2025-03-05 34871572+gmackall@users.noreply.github.com Implement `clipPath` Mutator for hcpp (flutter/flutter#164525)2025-03-05 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] use DeviceLocal textures for gifs on non-iOS devices. (#164573)" (flutter/flutter#164600)2025-03-05 jonahwilliams@google.com [macos] prefer integrated GPU. (flutter/flutter#164569)2025-03-05 muhatashim@google.com Enforce minSdk constraint for Android Flutter (flutter/flutter#164251)2025-03-05 dkwingsmt@users.noreply.github.com Add `clipRSuperellipse`, and use them for dialogs (flutter/flutter#161111)2025-03-05 engine-flutter-autoroll@skia.org Roll Skia from 46705a22edc3 to 03a3f653d64e (1 revision) (flutter/flutter#164590)2025-03-05 psturm@esri.com when resetting FlutterPlatformViewsController, clear out some additional internal state to prevent it from carrying over across a Hot Restart (flutter/flutter#164456)2025-03-05 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from Rt6pxGFLVAJHduM0V... to fhm5z889sA5T1AQao... (flutter/flutter#164583)If this roll has caused a breakage, revert this CL and stop the rollerusing the controls here:https://autoroll.skia.org/r/flutter-packagesPlease CC stuartmorgan@google.com,tarrinneal@google.com on the revert to ensure that a humanis aware of the problem.To file a bug in Packages:https://github.com/flutter/flutter/issues/new/chooseTo report a problem with the AutoRoller itself, please file a bug:https://issues.skia.org/issues/new?component=1389291&template=1850622Documentation for the AutoRoller is here:https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Labels
f: material designflutter/packages/flutter/material repository. frameworkflutter/packages/flutter repository. See also f: labels.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading.Please reload this page.
Closes#161368
This PR adds the
animateToItemmethod to theCarouselController, enabling smooth, index-based navigation for carousels with fixed or dynamically-sized items (viaflexWeights).teste.mov
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel onDiscord.