-
-
Notifications
You must be signed in to change notification settings - Fork 0
Add item model list pagination #43
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
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.
Pull Request Overview
This PR implements pagination for the item model list functionality by replacing direct list operations with a paginated result structure. The main change transforms the item data handling from a simple List<ItemModel>
to a PaginatedResult<ItemModel>
wrapper throughout the application state and API interactions.
- Replaces
List<ItemModel>
withPaginatedResult<ItemModel>
in app state - Updates item actions to work with paginated data structure
- Adds pagination converter and helper functions for JSON serialization
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
lib/feature/item/item_page.dart | Updates action names from AddItemAction /RemoveItemAction to ItemAddAction /ItemRemoveAction |
lib/api/state/factory/item/item_vm_state.dart | Changes view model to access items through .items property of paginated result |
lib/api/state/app_state.g.dart | Updates JSON serialization to handle PaginatedResult instead of direct list |
lib/api/state/app_state.freezed.dart | Generated code changes for new paginated result type with pattern matching extensions |
lib/api/state/app_state.dart | Modifies state definition to use PaginatedResult<ItemModel> with converter annotations |
lib/api/state/actions/item_actions.dart | Refactors all item actions to work with paginated structure and adds helper function |
lib/api/model/item_model.freezed.dart | Generated code with new pattern matching extensions |
lib/api/model/item_model.dart | Adds JSON conversion helper functions for paginated result converter |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
AppState _updateItemInState( | ||
AppState state, | ||
List<ItemModel> newItems, | ||
ItemModel? selectedItem, { | ||
int? totalItems, | ||
}) { | ||
final updated = state.items.copyWith( | ||
items: newItems, | ||
totalItems: totalItems ?? state.items.totalItems, | ||
totalPages: state.items.totalPages, | ||
currentPage: state.items.currentPage, | ||
pageSize: state.items.pageSize, | ||
); | ||
return state.copyWith(items: updated, selectedItem: selectedItem); | ||
} |
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.
The helper function doesn't update totalPages
when totalItems
changes, which could lead to inconsistent pagination state. Consider recalculating totalPages
when totalItems
is provided: totalPages: totalItems != null ? (totalItems / state.items.pageSize).ceil() : state.items.totalPages
Copilot uses AI. Check for mistakes.
79e7da9
to
9564eb7
Compare
033efa7
to
2c747eb
Compare
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.
Pull Request Overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
tabPages: (pages) => pages, | ||
compareFunction: (model) => vm.isSelectedItem(model), | ||
tabs: _getTabs(), | ||
onLoadMore: vm.hasNextPage && !vm.isLoadingMore |
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.
[nitpick] The condition uses vm.hasNextPage && !vm.isLoadingMore
but the view model property is named isLoadingMore
while the state property is isLoadingMoreItems
. This could lead to confusion. Consider renaming for consistency.
onLoadMore: vm.hasNextPage && !vm.isLoadingMore | |
onLoadMore: vm.hasNextPage && !vm.isLoadingMoreItems |
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <[email protected]>
Proposed changes
The pull request overhauls the item logic to use the new pagination widget to improve the load of models.
Types of changes
What types of changes does your code introduce to this project?
Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any ofthem, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before
merging your code.