Responsive dashboard #1256
Responsive dashboard #1256
Conversation
WalkthroughGuarded list rendering in the dashboard: user, instrument, and record lists now render only when their data is present; explicit error messages are shown when instrument or record data is missing; RecordCounter mapping preserves ids but adjusts field placement. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @apps/web/src/routes/_app/dashboard.tsx:
- Line 386: The React list is using instrument.instrumentTitle as the key which
may not be unique; change the key to use the unique identifier (instrument.id)
instead—locate the JSX element with key={instrument.instrumentTitle} in
dashboard.tsx and replace it with key={instrument.id} so React can properly
track list items.
- Around line 359-366: The error message never shows because recordCounter is
initialized with a `?? []` fallback so it's always a truthy array; change the
condition to check the actual source data or emptiness instead. Replace the
`!recordCounter` condition with a check like `!instrumentInfo ||
instrumentInfo.length === 0` (or `recordCounter.length === 0`) so the message
displays when there are no items; update the conditional around the paragraph
that renders the error to use that new check referencing `instrumentInfo` or
`recordCounter.length`.
🧹 Nitpick comments (2)
apps/web/src/routes/_app/dashboard.tsx (2)
290-327: Add loading state for better UX.The error fallback is correctly implemented, but consider adding a loading state check similar to the users modal for consistency and better user experience.
🔎 Proposed addition of loading state
<ul className="flex flex-col gap-5 overflow-auto"> + {instrumentInfoQuery.isLoading && <Spinner />} {!instrumentInfo && ( <p> {t({
367-396: Add loading state for better UX.Similar to the instruments modal, consider adding a loading state check for consistency with the users modal.
🔎 Proposed addition of loading state
<ul className="flex flex-col gap-5 overflow-auto"> + {recordsQuery.isLoading && <Spinner />} + {recordsQuery.isError && ( + <p> + {t({ + en: 'Error loading records', + fr: "Erreur lors du chargement d'enregistrements" + })} + </p> + )} {!instrumentInfo && (Note: After fixing the conditional check issue, you should use
recordsQuery.isErrorfor the error state rather than checking the derived data.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/src/routes/_app/dashboard.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/routes/_app/dashboard.tsx (1)
packages/runtime-internal/src/interactive/bootstrap.js (1)
instrument(48-48)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: lint-and-test
🔇 Additional comments (2)
apps/web/src/routes/_app/dashboard.tsx (2)
60-60: LGTM - Good addition of the id field.Adding the
idfield improves the object structure and enables better React key usage.
215-234: LGTM - Proper conditional rendering.The conditional check correctly prevents rendering when data is unavailable.
|
@joshunrau changes are added now |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/src/routes/_app/dashboard.tsx (2)
290-327: Add loading state check for consistency.The instruments modal shows an error message when
!instrumentInfo, which includes the loading state. This creates a misleading UX where "Error finding instruments" displays while data is still loading.For consistency with the users modal pattern (lines 206-214), add a loading state check.
🔎 Suggested improvement
<ul className="flex flex-col gap-5 overflow-auto"> + {instrumentInfoQuery.isLoading && <Spinner />} + {!instrumentInfoQuery.isLoading && !instrumentInfo && ( - {!instrumentInfo && ( <p> {t({ en: 'Error finding instruments', fr: 'Erreur lors de la recherche les instruments' })} </p> )} {instrumentInfo && (
359-396: Add loading state check for consistency.The records modal shows an error message when
!instrumentInfo, which includes the loading state. This creates a misleading UX where "Error finding records" displays while data is still loading.For consistency with the users modal pattern (lines 206-214), add a loading state check.
🔎 Suggested improvement
<ul className="flex flex-col gap-5 overflow-auto"> + {instrumentInfoQuery.isLoading && <Spinner />} + {!instrumentInfoQuery.isLoading && !instrumentInfo && ( - {!instrumentInfo && ( <p> {t({ en: 'Error finding records', fr: "Erreur lors de la recherche d'enregistrements" })} </p> )} {instrumentInfo && (
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/src/routes/_app/dashboard.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/routes/_app/dashboard.tsx (1)
packages/runtime-internal/src/interactive/bootstrap.js (1)
instrument(48-48)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: lint-and-test
🔇 Additional comments (2)
apps/web/src/routes/_app/dashboard.tsx (2)
56-63: LGTM: React key now uses unique identifier.The
idfield addition enables proper React list keying on line 386, addressing the previous concern about using non-uniqueinstrumentTitleas the key.
215-234: LGTM: Proper loading and error state handling.The user list correctly checks loading and error states before rendering, with smooth AnimatePresence transitions.
2 participants
added error messages for when data does not exist in the modal.
Summary by CodeRabbit
Bug Fixes
UI/Animation
✏️ Tip: You can customize this high-level summary in your review settings.