feat(ui): add infinity scroll pagination option to list components #157
Conversation
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
Introduce PaginationMode enum with loadMore and infinityScroll options. Implement infinity scroll functionality in all list components including app usage, habits, notes, tags, and tasks lists. Add scroll listeners, threshold detection, and auto-loading capabilities while maintaining backward compatibility with existing load more buttons.
Contributor
Summary of ChangesHello @ahmet-cetinkaya, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience across various list components in the application by introducing an infinity scroll pagination option. Instead of manually clicking a 'Load More' button, users can now seamlessly browse through long lists as new content loads automatically when they approach the end of the current view. This change provides a smoother and more intuitive interaction pattern for data-heavy sections of the app. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Reviewer's GuideAdds a configurable pagination mode for all major list-based features, introducing an infinity scroll option alongside the existing load-more button and wiring it through list widgets and their hosting pages, with scroll listeners, auto-load thresholds, and loading indicators. Sequence diagram for infinity scroll pagination flowsequenceDiagram
actor User
participant ListPage
participant ListWidget
participant ScrollController
participant DataService
User->>ListPage: Navigate to feature page
ListPage->>ListWidget: Create with paginationMode infinityScroll
ListWidget->>ListWidget: initState
ListWidget->>ListWidget: _setupScrollListener
ListWidget->>ScrollController: addListener(_onScroll)
ListWidget->>DataService: initial getList(pageIndex 0)
DataService-->>ListWidget: PagedResult(pageIndex 0, hasNext)
ListWidget->>ListWidget: build list, maybe _checkAndFillViewport
loop User scrolling
User->>ScrollController: Scroll events
ScrollController-->>ListWidget: _onScroll callback
ListWidget->>ListWidget: compute maxScroll, currentScroll, threshold
alt nearing bottom and hasNext and not _isLoadingMore
ListWidget->>ListWidget: _loadMoreInfinityScroll
ListWidget->>ListWidget: set _isLoadingMore true
ListWidget->>DataService: getList(pageIndex + 1)
DataService-->>ListWidget: PagedResult(next page, hasNext)
ListWidget->>ListWidget: merge items, update state
ListWidget->>ListWidget: set _isLoadingMore false
else above threshold or no next page
ListWidget-->>ScrollController: return
end
end
Class diagram for PaginationMode and updated list widgetsclassDiagram
direction LR
class PaginationMode {
<<enumeration>>
loadMore
infinityScroll
}
class HabitsList {
+int pageSize
+PaginationMode paginationMode
+HabitsList()
+createState() HabitsListState
}
class HabitsListState {
-ScrollController _scrollController
-bool _isLoadingMore
+initState()
+dispose()
-_setupScrollListener()
-_onScroll()
-_loadMoreInfinityScroll() Future~void~
-_checkAndFillViewport()
}
class TaskList {
+PaginationMode paginationMode
+TaskList()
+createState() TaskListState
}
class TaskListState {
-ScrollController _scrollController
-bool _isLoadingMore
+initState()
+dispose()
-_setupScrollListener()
-_onScroll()
-_loadMoreInfinityScroll() Future~void~
-_checkAndFillViewport()
}
class AppUsageList {
+PaginationMode paginationMode
+AppUsageList()
+createState() AppUsageListState
}
class AppUsageListState {
-ScrollController _scrollController
-bool _isLoadingMore
+initState()
+dispose()
-_setupScrollListener()
-_onScroll()
-_loadMoreInfinityScroll() Future~void~
-_checkAndFillViewport()
}
class NotesList {
+PaginationMode paginationMode
+NotesList()
+createState() NotesListState
}
class NotesListState {
-ScrollController _scrollController
-bool _isLoadingMore
+initState()
+dispose()
-_setupScrollListener()
-_onScroll()
-_loadMoreInfinityScroll() Future~void~
-_checkAndFillViewport()
}
class TagsList {
+PaginationMode paginationMode
+TagsList()
+createState() TagsListState
}
class TagsListState {
-ScrollController _scrollController
-bool _isLoadingMore
+initState()
+dispose()
-_setupScrollListener()
-_onScroll()
-_loadMoreInfinityScroll() Future~void~
-_checkAndFillViewport()
}
class AppUsageTagRuleList {
+PaginationMode paginationMode
+AppUsageTagRuleList()
+createState() AppUsageTagRuleListState
}
class AppUsageTagRuleListState {
-ScrollController _scrollController
-bool _isLoadingMore
+initState()
+dispose()
-_setupScrollListener()
-_onScroll()
-_loadMoreInfinityScroll() Future~void~
-_checkAndFillViewport()
}
class AppUsageIgnoreRuleList {
+PaginationMode paginationMode
+AppUsageIgnoreRuleList()
+createState() AppUsageIgnoreRuleListState
}
class AppUsageIgnoreRuleListState {
-ScrollController _scrollController
-bool _isLoadingMore
+initState()
+dispose()
-_setupScrollListener()
-_onScroll()
-_loadMoreInfinityScroll() Future~void~
-_checkAndFillViewport()
}
PaginationMode <.. HabitsList : uses
PaginationMode <.. TaskList : uses
PaginationMode <.. AppUsageList : uses
PaginationMode <.. NotesList : uses
PaginationMode <.. TagsList : uses
PaginationMode <.. AppUsageTagRuleList : uses
PaginationMode <.. AppUsageIgnoreRuleList : uses
HabitsList "1" o-- "1" HabitsListState
TaskList "1" o-- "1" TaskListState
AppUsageList "1" o-- "1" AppUsageListState
NotesList "1" o-- "1" NotesListState
TagsList "1" o-- "1" TagsListState
AppUsageTagRuleList "1" o-- "1" AppUsageTagRuleListState
AppUsageIgnoreRuleList "1" o-- "1" AppUsageIgnoreRuleListState
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- PaginationMode is only considered when setting up scroll listeners in initState; if a caller changes paginationMode after creation, the scroll listener won’t be updated, so consider handling add/remove of the listener in didUpdateWidget based on mode changes.
- The infinity scroll wiring (state flag, _setupScrollListener, _onScroll, _checkAndFillViewport, loading indicator logic) is duplicated across several list widgets; consider extracting this into a shared mixin/helper to reduce repetition and keep behavior consistent.
- The viewport fill check using maxScroll <= 0 may be fragile on different layouts; using ScrollMetrics.extentAfter (or similar) to decide when more items are needed would make the auto-load trigger more robust.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- PaginationMode is only considered when setting up scroll listeners in initState; if a caller changes paginationMode after creation, the scroll listener won’t be updated, so consider handling add/remove of the listener in didUpdateWidget based on mode changes.
- The infinity scroll wiring (state flag, _setupScrollListener, _onScroll, _checkAndFillViewport, loading indicator logic) is duplicated across several list widgets; consider extracting this into a shared mixin/helper to reduce repetition and keep behavior consistent.
- The viewport fill check using maxScroll <= 0 may be fragile on different layouts; using ScrollMetrics.extentAfter (or similar) to decide when more items are needed would make the auto-load trigger more robust.
## Individual Comments
### Comment 1
<location> `src/lib/presentation/ui/features/habits/components/habits_list.dart:112-113` </location>
<code_context>
super.dispose();
}
+ void _setupScrollListener() {
+ if (widget.paginationMode == PaginationMode.infinityScroll) {
+ _scrollController.addListener(_onScroll);
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Pagination mode changes at runtime are not reflected in the scroll listener setup.
The listener is configured only once in `initState` from the initial `widget.paginationMode`. If the parent rebuilds `HabitsList` with a different mode (e.g. switching between `loadMore` and `infinityScroll`), the listener setup won’t adjust: infinity mode may lack a listener, or a disabled mode may keep one attached. Handle this in `didUpdateWidget` by detecting changes in `paginationMode` and adding/removing the listener accordingly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
src/lib/presentation/ui/features/habits/components/habits_list.dart
Outdated
Show resolved
Hide resolved
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces infinity scroll functionality to several list components, including AppUsageIgnoreRuleList, AppUsageList, AppUsageTagRuleList, HabitsList, NotesList, TagsList, and TaskList. This involves adding a paginationMode enum to control whether a list uses a 'Load More' button or automatic infinity scrolling, and implementing corresponding scroll listener and loading logic within each component. Review comments indicate that the infinity scroll logic is highly duplicated across these components and suggest refactoring it into a reusable Dart mixin. Additionally, the itemCount logic for lists is noted as unstable, causing UI jumps, and should be adjusted to always reserve a slot for the loading indicator. Finally, the scroll threshold of 0.8 is identified as a magic number and should be extracted into a named constant for better maintainability.
src/lib/presentation/ui/features/app_usages/components/app_usage_ignore_rule_list.dart
Outdated
Show resolved
Hide resolved
src/lib/presentation/ui/features/app_usages/components/app_usage_ignore_rule_list.dart
Outdated
Show resolved
Hide resolved
Owner
Author
Code reviewFound 5 issues:
whph/src/lib/presentation/ui/features/tasks/components/tasks_list.dart Lines 154 to 162 in 563d610
whph/src/lib/presentation/ui/features/tasks/components/tasks_list.dart Lines 127 to 133 in 563d610
whph/src/lib/presentation/ui/features/tasks/components/tasks_list.dart Lines 145 to 147 in 563d610
whph/src/lib/presentation/ui/features/tasks/components/tasks_list.dart Lines 154 to 162 in 563d610 |
- Extract duplicated infinity scroll logic to PaginationMixin - Refactor list components to implement IPaginatedWidget - Fix race conditions in loading state - Fix memory leaks in scroll listener management - Improve viewport fill check robustness - Add error handling for data loading - Remove magic values
Owner
Author
Auto code reviewNo issues found. All previously identified issues have been fixed:
The infinity scroll pagination implementation is now robust and production-ready. |
- Wrap scrollController.position access in try-catch blocks - Protects against potential errors when scroll position is accessed - Maintains stability in _onScroll and checkAndFillViewport methods - Addresses reported issue where position could throw exceptions even when hasClients is true Fixes issue reported by Bug Agent 2 about missing null checks in pagination_mixin.dart
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
🎯 Context
This PR adds infinity scroll pagination as an alternative to the existing load more button across all list components in the app.
🛠️ Major Changes
🧪 Testing
Summary by Sourcery
Add an optional infinity scroll pagination mode alongside the existing load-more behavior and wire it into key list-based screens.
New Features:
Enhancements: