Skip to content

feat: loop scroll - #66

Merged
Limbou merged 9 commits into
mainfrom
feature/loop-scroll
Nov 28, 2025
Merged

Limbou merged 9 commits into
mainfrom
feature/loop-scroll

Conversation

@Limbou

@Limbou Limbou commented Nov 28, 2025 •

Copy link
Copy Markdown
Owner

Summary

Adds a loop parameter to ExpandablePageView and ExpandablePageView.builder that enables infinite scrolling by cycling through pages. Closes #50.
Changes

  • New loop parameter (default: false) - When enabled, users can scroll infinitely in both directions, wrapping from the last page to the first and vice versa
  • Virtual index approach - Uses a large virtual item count internally while exposing real indices (0 to itemCount-1) to the consumer
  • Runtime loop toggling - Supports changing loop property dynamically while preserving the current page
  • Validation - Assertions prevent loop: true with empty children

Usage

ExpandablePageView(
  loop: true,
  children: [Page1(), Page2(), Page3()],
)
ExpandablePageView.builder(
  loop: true,
  itemCount: 3,
  itemBuilder: (context, index) => pages[index],
)

When loop is true, the PageView cycles through pages infinitely:
- Swiping past the last page shows the first page
- Swiping before the first page shows the last page
- Works with both children list and builder constructors
- onPageChanged receives real index (0 to itemCount-1)
- Compatible with viewportFraction, vertical scroll, and other features

Added 12 comprehensive tests for loop functionality.

Closes #50
- Add assertions to prevent loop=true with empty children/itemCount=0
- Add _handleLoopChange() to properly transition when loop property changes at runtime
- Handle simultaneous loop and children count changes in didUpdateWidget
- Add guard to skip controller changes in loop mode (internal controller is used)
- Add documentation explaining controller limitation in loop mode
- Remove dead code: unused _maxVisibleSize getter and unreachable count==0 check
- Simplify _prepareSizes() and _previousPage initialization
- Add 15 additional tests for edge cases and runtime changes
- Test controller A -> controller B transition
- Test null -> controller B transition
- Test controller A -> null transition
- Test multiple controller changes (A -> B -> C)
- Test null -> controller -> null transition

All 80 tests passing.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds infinite loop scrolling functionality to the ExpandablePageView widget, allowing users to cycle through pages infinitely in both directions. This addresses issue #50 and provides a commonly requested feature for carousel-like experiences where pages wrap around from the last page to the first.

Key Changes

  • Added loop parameter to enable infinite page scrolling with automatic wrapping
  • Implemented virtual page index system using a large multiplier offset to support bidirectional infinite scrolling
  • Updated size calculation logic to handle wrapped pages when viewportFraction < 1.0

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
lib/src/expandable_page_view.dart Core implementation of loop functionality with virtual-to-real index mapping, internal controller management in loop mode, and viewport fraction calculations for wrapped pages
test/expandable_page_view_test.dart Comprehensive test suite with 58 new tests covering loop behavior, edge cases, controller changes, and integration scenarios
CHANGELOG.md Documentation of new feature addition
Comments suppressed due to low confidence (1)

lib/src/expandable_page_view.dart:351

  • Incorrect listener removal. At line 347, the code attempts to remove _updatePage listener from oldWidget.controller. However, in non-loop mode, the listener was added to _pageController (line 325 or 349), not to oldWidget.controller.

If oldWidget.controller was provided, then _pageController == oldWidget.controller, so this works. But if oldWidget.controller was null, then _pageController was an internally created controller, and oldWidget.controller?.removeListener(_updatePage) does nothing (null-safe call). Then the old _pageController still has the listener attached when it's replaced by the new controller at line 348.

This creates a memory leak and potential crashes because:

  1. The old internal controller (if it existed) is not properly cleaned up
  2. The _updatePage callback will continue to be called on the old controller
  3. The old controller is never disposed if _shouldDisposePageController was true

Fix:

if (oldWidget.controller != widget.controller && !widget.loop) {
  _pageController.removeListener(_updatePage);  // Remove from current controller
  if (_shouldDisposePageController) {
    _pageController.dispose();  // Dispose the old internal controller
  }
  _pageController = widget.controller ?? PageController();
  _pageController.addListener(_updatePage);
  _shouldDisposePageController = widget.controller == null;
}
    if (oldWidget.controller != widget.controller && !widget.loop) {
      // Only handle controller changes in non-loop mode
      // In loop mode, we manage our own internal controller
      oldWidget.controller?.removeListener(_updatePage);
      _pageController = widget.controller ?? PageController();
      _pageController.addListener(_updatePage);
      _shouldDisposePageController = widget.controller == null;
    }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/src/expandable_page_view.dart Outdated
Comment thread lib/src/expandable_page_view.dart
Comment thread lib/src/expandable_page_view.dart
Comment thread lib/src/expandable_page_view.dart

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

lib/src/expandable_page_view.dart:351

  • Memory leak: The old _pageController is not disposed when transitioning from an internal controller to an external controller in non-loop mode. When oldWidget.controller is null but widget.controller is provided, the old internal _pageController should be disposed before being replaced.

Consider adding:

if (oldWidget.controller != widget.controller && !widget.loop) {
  oldWidget.controller?.removeListener(_updatePage);
  if (oldWidget.controller == null && _shouldDisposePageController) {
    _pageController.dispose();
  }
  _pageController = widget.controller ?? PageController();
  _pageController.addListener(_updatePage);
  _shouldDisposePageController = widget.controller == null;
}
    if (oldWidget.controller != widget.controller && !widget.loop) {
      // Only handle controller changes in non-loop mode
      // In loop mode, we manage our own internal controller
      oldWidget.controller?.removeListener(_updatePage);
      _pageController = widget.controller ?? PageController();
      _pageController.addListener(_updatePage);
      _shouldDisposePageController = widget.controller == null;

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Extract _createLoopController and _createStandardController for PageController creation
- Extract _initializePageController to consolidate init logic
- Extract _wrapWithSizeReporting to unify OverflowPage creation
- Simplify _sizeReportingChildren using list comprehension
- Convert _loopInitialPage getter to _loopInitialPageFor method for reusability
Previously, when transitioning from no controller (internal) to an external
controller, the internal PageController was replaced but never disposed,
causing a memory leak. Now we properly remove the listener and dispose
the controller if we own it before switching.
…ild widget

Renamed OverflowPage to _SizeReportingChild and made it private.
Using a private widget class instead of a method that returns a widget
follows Flutter best practices for widget composition.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Limbou
Limbou merged commit 3c6413e into main Nov 28, 2025
8 checks passed
@Limbou
Limbou deleted the feature/loop-scroll branch November 28, 2025 20:48
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.

Looping through the pages

2 participants