feat(ui): add prev/next buttons to modals #42

gladyshcodes · 2025-02-21T01:33:47Z

Issue: #38

What

This PR adds previous and next buttons to the Moderation and User modals, enabling users to navigate through a filtered list of items without closing the modal

Why

By allowing seamless navigation within the modal, we reduce the need for context switching, thereby enhancing the overall user experience

How

The items come from a TRPC endpoint and are stored in a shared React context that both the Tanstack table and the modals access. The new React context manages the active collection of items. The PrevNextButtons reusable component is added and provides callbacks for moving to the previous or next item. It also fetches additional items when the end of the collection is reached

UI

Before:

After:

CLAassistant · 2025-02-21T01:33:53Z

All committers have signed the CLA.

gladyshcodes · 2025-02-21T01:42:55Z

app/dashboard/records/[id]/record.tsx

@@ -187,6 +196,7 @@ export async function RecordDetail({ clerkOrganizationId, id }: { clerkOrganizat
          </Section>
        </>
      )}
+      {isModal && <RecordsCollectionNavigation currentRecordId={id} />}


Display buttons exclusively in modal view, not on a single page

gladyshcodes · 2025-02-21T01:45:50Z

app/dashboard/records/[id]/records-collection-navigation.tsx

+
+export function RecordsCollectionNavigation({ currentRecordId }: RecordsCollectionNavigationProps) {
+  const { query } = useActiveCollection<InfiniteData<RecordsPage>>();
+  const { data, fetchNextPage, hasNextPage } = query ?? {};


Query functions such as fetchNextPage are not stored in react-query cache. The only way to retrieve them is to keep its reference in some kind of state (react context in this case)

gladyshcodes · 2025-02-21T01:48:55Z

app/dashboard/records/[id]/records-collection-navigation.tsx

+    router.replace(`/dashboard/records/${recordId}`);
+  };
+
+  const fetchNextItems = async () => {


This is a wrapper function above fetchNextPage

It's there to:

Call react-query fetchNextPage

Aggregate data

Return aggregated data

The reason this function exist and it returns is because without it (when calling fetchNextPage directly) we could end up in race condition. It takes some ms for updated data to be retrieved by component via props (next page is fetched -> context updated -> context consumer rerendered -> new data received)

gladyshcodes · 2025-02-21T01:54:28Z

components/router-sheet.tsx

@@ -16,6 +16,7 @@ export function RouterSheet({ children, title }: { children: React.ReactNode; ti
          e.preventDefault();
          router.back();
        }}
+        disableAnimation={true}


Disable Sheet animation as using prev/next buttons will trigger it every time. If it's crucial to keep it, could disable it only for this type of navigation

Another idea here is not to change URL when prev/next is clicked. Since we already have that data into context, we could pull the data from there, without redirecting to a new page every time (and fetching data again). Although that's not how it's done in other parts of app currently

gladyshcodes · 2025-02-21T01:55:28Z

@s3ththompson Could you review, please?

s3ththompson · 2025-02-21T16:34:21Z

@gladyshcodes The following actions produce a crash:

Click on a moderation from the table at /moderations
In the modal, click on the username of the user who created the record
Rendering the user modal crashes with TypeError: Cannot read properties of undefined (reading 'id')

Perhaps it would solve things to hide the navigation completely, until the query in the context is populated?

s3ththompson

See error mentioned above

s3ththompson · 2025-02-21T16:36:03Z

@gladyshcodes If you hide the navigation completely unless the query is populated, would it solve the race condition allowing you to remove the fetchNextItems wrapper and also allow you to remove the manual isModal prop?

* feat: add client-side date components with timezone support Co-Authored-By: [email protected] <[email protected]> * add remaining spots for date components --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: [email protected] <[email protected]> Co-authored-by: Seth Thompson <[email protected]>

gladyshcodes · 2025-02-21T20:11:12Z

The following actions produce a crash

Thank you for noticing that. I ran seed manually and fixed TypeError: Cannot read properties of undefined (reading 'id')

Missed that because I didn't do proper seed. And as a result., I didn't have User section in Moderation modal at all. I have this error when trying to run npm run dev:db:seed. If you have it too and it's not something specific to my env, I could try to fix it in my next pr.

Details

gladyshcodes · 2025-02-21T20:18:33Z

@gladyshcodes If you hide the navigation completely unless the query is populated, would it solve the race condition allowing you to remove the fetchNextItems wrapper and also allow you to remove the manual isModal prop?

The problem was not caused by empty Context when modal is opened but because when you opened User modal from Moderation one, Context contained moderation records and the modal expected Users. Race condition happens when you try to fetch more data within the modal (you reach last fetched item, and click next button that will trigger next page fetch). This new data isn't immediately available from modal component as it needs to travel back from data-table in contrast to using awaited function data

Regarding isModal props, it isn't related to refetching, we need to pass it from @sheet route to differentiate views and show buttons in modal only: #42 (comment)

gladyshcodes · 2025-02-21T20:21:16Z

@s3ththompson Feedback addressed, CLA signed

gladyshcodes commented Feb 21, 2025

View reviewed changes

s3ththompson requested changes Feb 21, 2025

View reviewed changes

gladyshcodes and others added 5 commits February 21, 2025 21:04

feat(ui): add prev/next buttons to modals

f4ee8d0

chore: remove JSON.stringify (perf bottleneck with big data)

48dcabe

only pause/unpause payments (anti-work#37)

dc75e68

fix: bug when opening user modal from record modal

ea6c694

gladyshcodes force-pushed the gladish/prevnext-buttons branch from 04dda65 to ea6c694 Compare February 21, 2025 20:05

Merge branch 'main' into gladish/prevnext-buttons

602e7d7

gladyshcodes requested a review from s3ththompson February 21, 2025 20:21

gladyshcodes and others added 2 commits February 22, 2025 02:03

fix: bug when opening record modal from user modal

d809957

Merge branch 'main' into gladish/prevnext-buttons

d0bdedd

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ui): add prev/next buttons to modals #42

feat(ui): add prev/next buttons to modals #42

gladyshcodes commented Feb 21, 2025 •

edited

Loading

CLAassistant commented Feb 21, 2025 •

edited

Loading

gladyshcodes Feb 21, 2025

gladyshcodes Feb 21, 2025 •

edited

Loading

gladyshcodes Feb 21, 2025 •

edited

Loading

gladyshcodes Feb 21, 2025 •

edited

Loading

gladyshcodes commented Feb 21, 2025

s3ththompson commented Feb 21, 2025

s3ththompson left a comment

s3ththompson commented Feb 21, 2025

gladyshcodes commented Feb 21, 2025 •

edited

Loading

gladyshcodes commented Feb 21, 2025 •

edited

Loading

gladyshcodes commented Feb 21, 2025

feat(ui): add prev/next buttons to modals #42

Are you sure you want to change the base?

feat(ui): add prev/next buttons to modals #42

Conversation

gladyshcodes commented Feb 21, 2025 • edited Loading

What

Why

How

UI

CLAassistant commented Feb 21, 2025 • edited Loading

gladyshcodes Feb 21, 2025

Choose a reason for hiding this comment

gladyshcodes Feb 21, 2025 • edited Loading

Choose a reason for hiding this comment

gladyshcodes Feb 21, 2025 • edited Loading

Choose a reason for hiding this comment

gladyshcodes Feb 21, 2025 • edited Loading

Choose a reason for hiding this comment

gladyshcodes commented Feb 21, 2025

s3ththompson commented Feb 21, 2025

s3ththompson left a comment

Choose a reason for hiding this comment

s3ththompson commented Feb 21, 2025

gladyshcodes commented Feb 21, 2025 • edited Loading

gladyshcodes commented Feb 21, 2025 • edited Loading

gladyshcodes commented Feb 21, 2025

gladyshcodes commented Feb 21, 2025 •

edited

Loading

CLAassistant commented Feb 21, 2025 •

edited

Loading

gladyshcodes Feb 21, 2025 •

edited

Loading

gladyshcodes Feb 21, 2025 •

edited

Loading

gladyshcodes Feb 21, 2025 •

edited

Loading

gladyshcodes commented Feb 21, 2025 •

edited

Loading

gladyshcodes commented Feb 21, 2025 •

edited

Loading