这是indexloc提供的服务,不要输入任何密码
Skip to content

Adds animateToItem to the CarouselController #162694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

Mairramer
Copy link
Contributor

@Mairramer Mairramer commented Feb 4, 2025

Closes #161368
This PR adds the animateToItem method to the CarouselController, enabling smooth, index-based navigation for carousels with fixed or dynamically-sized items (via flexWeights).

teste.mov

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Feb 4, 2025
@Mairramer Mairramer marked this pull request as draft February 5, 2025 12:50
@flutter-dashboard
Copy link

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, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@Mairramer Mairramer marked this pull request as ready for review February 5, 2025 12:55
@Piinks Piinks requested review from QuncCccccc and Piinks February 5, 2025 19:22
Copy link
Contributor

@QuncCccccc QuncCccccc left a 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!;
Copy link
Contributor

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]?

Copy link
Contributor Author

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.

Copy link
Contributor

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.

@Mairramer Mairramer requested a review from QuncCccccc February 12, 2025 12:48
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;
Copy link
Contributor

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

Copy link
Contributor Author

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.

@Mairramer Mairramer force-pushed the feat/add-animateToItem-into-carousel-controller branch from 4c4e849 to 3f923d1 Compare February 14, 2025 11:32
@Mairramer Mairramer requested a review from QuncCccccc February 14, 2025 13:01
expect(realOffset(), controller.offset);
});

testWidgets('CarouselView animateToItem with flexWeights', (WidgetTester tester) async {
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

@Piinks Piinks left a 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?

@Mairramer
Copy link
Contributor Author

Mairramer commented Feb 14, 2025

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?

Yes, works with itemSnapping.
About shrinkOffset I may have to adjust it, but I believe we won't have a problem (I'm trying to imagine some use cases).

@Mairramer Mairramer requested a review from Piinks February 14, 2025 18:10
}

final bool hasFlexWeights = _carouselState!._flexWeights?.isNotEmpty ?? false;
index = index.clamp(0, _carouselState!.widget.children.length - 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NICE! Thanks!

@Mairramer Mairramer requested a review from QuncCccccc February 25, 2025 11:51
Copy link
Contributor

@QuncCccccc QuncCccccc left a 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!

Copy link
Contributor

@Piinks Piinks left a 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!

@Piinks Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 5, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Mar 5, 2025
Merged via the queue into flutter:master with commit 40e4c5e Mar 5, 2025
74 of 75 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 5, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Mar 6, 2025
flutter/flutter@2578d97...321fbc0

2025-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 from 9e4684e to abba683 (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 roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC stuartmorgan@google.com,tarrinneal@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 21, 2025
androidseb pushed a commit to androidseb/packages that referenced this pull request Jun 8, 2025
flutter/flutter@2578d97...321fbc0

2025-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 from 9e4684e to abba683 (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 roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC stuartmorgan@google.com,tarrinneal@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation 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 request Jun 9, 2025
flutter/flutter@2578d97...321fbc0

2025-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 from 9e4684e to abba683 (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 roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC stuartmorgan@google.com,tarrinneal@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

carouselview add animateToItem
3 participants