feat: paginating Users page #4792
Conversation
setSearchParams overwrites all search params, rather than merging
|
|
||
| export const usersMachine = createMachine( |
There was a problem hiding this comment.
A lot of this diff is just because I used the visualizer and it reformatted, so I'll comment where it's relevant
| states: { | ||
| startingPagination: { | ||
| entry: "assignPaginationRef", | ||
| always: { |
There was a problem hiding this comment.
start in this state to spawn the pagination machine, then go to gettingUsers. Since we always go there on mount, we don't need the useEffect kicking it off. The other thing the useEffect did was respond to filter changes, but we send events for that now.
| target: "updatingUserRoles", | ||
| actions: "assignUserIdToUpdateRoles", | ||
| }, | ||
| UPDATE_PAGE: { |
There was a problem hiding this comment.
UPDATE_PAGE and UPDATE_FILTER are new. UPDATE_FILTER sends RESET_PAGE to the pagination machine, which sends UPDATE_PAGE to this machine.
| // when it is mocked. This happen in the UsersPage tests inside of the | ||
| // "shows a success message and refresh the page" test case. | ||
| getUsers: (context) => { | ||
| const { offset, limit } = getPaginationData(context.paginationRef) |
There was a problem hiding this comment.
sending pagination data in the api call
| }) | ||
| }, | ||
| }), | ||
| assignPaginationRef: assign({ |
There was a problem hiding this comment.
These last two actions are new. Sorry about all the noise!
|
While doing some QA I noticed the pagination is being displayed during the loading phase. Screen.Recording.2022-10-27.at.17.06.01.mov |
|
I'm able to go next when there are no more results. Since we don't have the count endpoint for the users. I think we could check if the current size is "lower" than 25 (I'm guessing it is the number of items on each page). |
Yeah, I was just going to fix that by adding the count endpoint next. It's always possible you have exactly 25 records so sometimes we'll enable the next button when we shouldn't. |
Makes sense, because I'm letting it show when the count is undefined. This makes me think we might want the pagination controls to just show all the time. I like having them available before the count is loaded both because it enables me to break up this work and because, if we don't end up changing our strategy for fetching the count, it means we don't block page usage on waiting for the count to come back (it's an expensive query when the number of records is large). |
|
IMO, the pagination only should be visible when the table is visible since the pagination is kinda an extension of the table controls. |
Yeah, that makes sense, but right now since we don't have the count yet, nothing stops you from paging forward too far to an empty page, so we have to show the controls in that case. Once the endpoint is done, I might be able to change it, but it depends on other decisions (like how we handle navigating to a page with no entries and whether we want count to remain a separate endpoint). I think I'll just make the widget always show for now so at least it doesn't look bad. |
The users page doesn't have a count endpoint yet, so the pagination widget doesn't show page numbers.
I also updated a helper function in api.ts to be more widely usable and used it in all paginated endpoints.