changes to implement previous and next buttons in moderation modal an… #93
Conversation
|
|
||
| export type ActiveCollectionQueryResult<TData> = UseInfiniteQueryResult<TData, unknown>; | ||
|
|
||
| interface ActiveCollectionContextType<TData> { |
There was a problem hiding this comment.
let's update the active collection context to store the baseUrl of the active collection. (Then we can return to the baseUrl when someone clicks outside the modal)
There was a problem hiding this comment.
If we implement this, when a user opens the user record modal and clicks outside, they will be redirected to the moderation listing, as we are using the same record modal for moderation records. Since the base URL will be /dashboard/moderations, I believe it makes sense to keep it this way to maintain consistency.
Additionally, we don't need to make any changes to the RouterSheet. We can keep router.back as it is.
|
|
||
| const router = useRouter(); | ||
| const onPrevious = (userId: string) => { | ||
| router.replace(`/dashboard/users/${userId}`); |
There was a problem hiding this comment.
If we store the baseUrl (see below) we can use router.push here and in onNext
| @@ -16,6 +16,7 @@ export function RouterSheet({ children, title }: { children: React.ReactNode; ti | |||
| e.preventDefault(); | |||
| router.back(); | |||
There was a problem hiding this comment.
if there's an active collection, let's read the baseUrl and navigate to that. otherwise, we can fall back to router.back()
| @@ -82,6 +85,10 @@ const DataTable = ({ clerkOrganizationId }: { clerkOrganizationId: string }) => | |||
| } | |||
| }, [fetchMoreOnBottomReached]); | |||
|
|
|||
| useEffect(() => { | |||
| setQuery(queryResult); | |||
There was a problem hiding this comment.
it seems like a code smell that setQuery is called in different places each time. For the moderations table it's in a useEffect at the table level. For the user's records list it's called in the onClick of the button prior to navigating. For the users table it's in a useEffect but only if the route includes a certain prefix. This will be brittle / hard to maintain going forward.
It's really not a side effect, but rather a part of navigating.
Let's always call setQuery imperatively right before navigating to the route. This means we should move the call to the button / link that routes to the member of the collection.
There was a problem hiding this comment.
that makes sense, and I believe this approach will work well for moderations since it's a single-level navigation. When a user clicks on a moderation record, we can set the collection, and closing the modal will naturally return to the previous state.
however, for users, the navigation involves two levels. While we can set the collection when a user moves deeper into the hierarchy, the challenge arises when they close the modal at the second level. In this case, we are unable to update the collection properly.
i believe the only way to handle this effectively is by using useEffect with pathname as a dependency. This would allow us to detect when the modal is closed and update the collection accordingly.
to implement this, I am keeping the useEffect in the users data table while removing it from moderations. Instead, for moderations, I will set setQuery when the user clicks on a row. If you have any other ideas, I’d be happy to explore them!
What:
Introduces previous/next navigation buttons in the Moderation and User modals.
Allows seamless navigation within the modal without returning to the table view.
How:
Implements a React context to store an active "collection" of moderation records or users.
This collection is populated based on the TRPC endpoint results for a given search term or filter setting.
The context is shared between the TanStack table and the modal, enabling consistent navigation.
Places the context in the top-level dashboard layout to ensure accessibility for the modals.
Why:
To enhances user experience by allowing quick record browsing within the modal.
Reduces the need for unnecessary navigation between the table view and the modal.
User modal nested navigation
https://github.com/user-attachments/assets/85fd80ec-8b47-4802-92c1-f6af25618c2d
Moderation modal navigation
moderations.navigation.mp4
Thanks for the feedback on my last PR to this issue. I understand the concern about the scope of changes. However, I haven't changed the architecture—instead, I achieved the requirement with a few adjustments to my previous PR while keeping the implementation simple and maintaining stability.
Let me know if you'd like any refinements, but I believe this approach effectively meets the goal without introducing unnecessary complexity.
Moderations modal
moderations.mp4
Users modal
users.mp4
this PR addresses issue #38