Extract DataViewPaginatedResponse#24592
Conversation
Generated by 🚫 Danger |
kean
force-pushed
the
task/activity-logs-swiftui
branch
from
3219b88 to
9dd6c51
Compare
|
| App Name | Jetpack | |
| Configuration | Release-Alpha | |
| Build Number | 27883 | |
| Version | PR #24592 | |
| Bundle ID | com.jetpack.alpha | |
| Commit | f99f95c | |
| Installation URL | 35vrmjvi9m3ng |
|
| App Name | WordPress | |
| Configuration | Release-Alpha | |
| Build Number | 27883 | |
| Version | PR #24592 | |
| Bundle ID | org.wordpress.alpha | |
| Commit | f99f95c | |
| Installation URL | 7frij6e7ks93g |
| /// | ||
| /// - Parameter row: The row that appeared. | ||
| public func onRowAppeared(_ row: Element) { | ||
| guard items.suffix(16).contains(where: { $0.id == row.id }) else { |
There was a problem hiding this comment.
Should the suffix argument be 10 (according to the documentation), instead of 16?
| } | ||
|
|
||
| func getSubscribersService() throws -> SubscribersServiceRemote { | ||
| func maketSubscribersService() throws -> SubscribersServiceRemote { |
There was a problem hiding this comment.
| func maketSubscribersService() throws -> SubscribersServiceRemote { | |
| func makeSubscribersService() throws -> SubscribersServiceRemote { |
| /// - response: The paginated response handler that manages the data. | ||
| /// - content: A view builder that creates the content for each item. | ||
| public init( | ||
| response: Response, |
There was a problem hiding this comment.
If I'm not mistaken, the query is built into the Response instance. When you change the query (like sorting or filtering by a different property), you create a new Response instance and thus use a different DataViewPaginatedForEach view. If that's the case, the list would start from the first page after changing the query, since previously loaded data is lost (no matter how far the user has scrolled), which may not be a good UX?
It's not something we need to solve now, but I thought it might be something we should consider further down the road.
- Rename PaginatedResponse to DataViewPaginatedResponse for better naming consistency - Move files from Pagination to DataView directory - Update documentation to focus on UI usage with PaginatedForEach - Add comprehensive unit tests for DataViewPaginatedResponse
- Update deleteItem method to properly decrement total when removing items - Update tests to verify total count is correctly maintained after deletion
Replace custom SubscribersPaginatedResponse with generic DataViewPaginatedResponse system. Add notification-based subscriber deletion updates and improve pagination handling with DataViewPaginatedForEach component.
…sion - Rename getSubscribersService() to maketSubscribersService() to better reflect factory pattern - Move deleteSubscriber extension from SubsriberDetailsViewModel to dedicated SubscribersServiceRemote+Extensions file - Update all callers to use the renamed method
kean
force-pushed
the
task/activity-logs-swiftui
branch
from
d464cb1 to
c07e5bc
Compare
|
Something is wrong with this PR. It keeps marking my replies as "Pending". I addressed the feedback, and here was the reply about the filters UI/UX:
|
|
4 participants
This PR is a prerequisite for reimplementing "Jetpack Activity Logs" using "DataViews".
Summary
DataViewPaginatedResponsefor managing paginated data loadingDataViewPaginatedForEachSwiftUI component for displaying paginated listscount(should decrement by one)LoadMoreFooterViewto make it clear it's part of the "DataView" infrastructureTest Plan
Simulator.Screen.Recording.-.iPhone.16.Pro.-.2025-06-16.at.14.44.08.mov
Note: nearly all code was generated with Claude Code (after many-many prompts)