Skip to content
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

Fixes last item size when ExpandableCarousel has no padsEnds #61

Merged
merged 2 commits into from
Feb 11, 2025

Conversation

Zigotote
Copy link
Contributor

@Zigotote Zigotote commented Feb 10, 2025

Pull Request Checklist

What does this PR do?

Fixes bugs or issues #48

When last item of ExpandableCarousel is bigger than others and padsEnds is false, its bottom is clipped.

That's because 2 carousel items are displayed in the same page, and real page index is floored. So when I switch to slide 4, pageIndex is 3.75 (or 3.xxx, depending on viewportFraction, padding etc etc). And (3.75).floor() is 3, so if the 3rd item is smaller than the 4th, the 4th is displayed but its size is not large enough.

I hope this is clear. You can reproduce with this options in your ExpandableCarousel example :

ExpandableCarouselOptions(
  // viewportFraction: 1.0,
 // enableInfiniteScroll: true, // Remove enabledInfiniteScroll because it makes slide 1 and 4 appear on the same page, so slide 4 sticks to the left of the page and realIndex is correct 
  // enlargeCenterPage: true,
  // initialPage: 1,
  autoPlay: true,
  controller: _controller,
  //showIndicator: false,
  floatingIndicator: false,
  padEnds: false, // Add padEnds
  restorationId: 'expandable_carousel',
)

Checklist

Code Changes

  • I have added new features to the package (e.g., autoplay, indicator customization, etc.)
  • I have fixed existing issues (e.g., performance, edge cases)
  • I have improved the overall structure or optimized the codebase

Documentation

  • I have updated the README file or relevant documentation with the changes
  • I have added code usage examples or updated existing examples to reflect changes
  • I have updated the package version in the pubspec.yaml file

Testing

General Tests

  • The carousel widget works correctly with default settings
  • The carousel can handle a dynamic number of children (images, texts, etc.)
  • The carousel supports custom widgets for carousel items

Autoplay Feature

  • Autoplay starts when enabled and stops when disabled
  • Autoplay pauses when the user interacts (swipes/taps) with the carousel
  • Autoplay resumes after interaction

Indicators and Customization

  • The carousel displays indicators correctly and updates when navigating through slides
  • Custom indicators (colors, shapes, positions) are rendered correctly
  • Custom animations or transitions between slides work as expected

Looping and Scrolling

  • Infinite loop mode works smoothly without any jumps or glitches
  • The carousel scrolls smoothly horizontally and/or vertically
  • Pagination and snapping behavior works as expected

Accessibility

  • The carousel widget supports screen readers (e.g., Semantics labels added)
  • Users can navigate between items using a keyboard (if necessary)

Responsiveness

  • The carousel adapts to different screen sizes (mobile, tablet, desktop)
  • The carousel responds correctly to device orientation changes

Error Handling

  • The carousel handles empty/null items gracefully
  • The carousel handles large data sets without crashes or performance drops

Performance

  • I ran performance tests to ensure no regressions
  • The carousel renders efficiently, even with a large number of items

How did you verify your code works?

  • I ran manual tests to check various carousel configurations (autoplay, infinite loop, custom widgets)
  • All tests pass locally (flutter test)

Copy link
Owner

@nixrajput nixrajput left a comment

Choose a reason for hiding this comment

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

Looking fine to be merged.
I will test and merge it soon.

Thank you for the PR.

@nixrajput nixrajput merged commit ba5eedb into nixrajput:master Feb 11, 2025
2 checks passed
nixrajput added a commit that referenced this pull request Mar 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants