Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @Connorbelez, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 13 minutes and 34 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughRefactors admin detail UI from URL/search-param-driven sheets to a provider-backed sidebar. Adds Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Route as Admin Route
participant Provider as RecordSidebarProvider
participant Sidebar as RecordSidebar
participant Surface as AdminRecordDetailSurface
participant Query as Data Layer (useQuery)
User->>Route: Render admin page
Route->>Provider: Mount within RecordSidebarProvider
Provider->>Sidebar: Provide context (isOpen=false)
User->>Sidebar: Click record row -> open(recordRef)
Sidebar->>Provider: open({ entityType, recordId })
Provider->>Provider: Update state (isOpen=true, stack=[ref])
Provider->>Sidebar: current updated
Sidebar->>Surface: Render AdminRecordDetailSurface(reference)
Surface->>Query: Fetch objectDef, fieldDefs, recordReference
Query-->>Surface: Return data
Surface->>Sidebar: Render header + tabs (Details, Relations, Notes, Files, History)
User->>Sidebar: Click close or back
Sidebar->>Provider: close() or back()
Provider->>Sidebar: Update state (isOpen false or stack popped)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Owner
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Pull request overview
Builds out the admin shell foundation (navigation + layout + route states) and introduces a provider-backed record detail sidebar surface, aligning /admin routes with the new shell patterns.
Changes:
- Add a typed admin entity registry and use it to render grouped sidebar navigation + breadcrumbs.
- Introduce a provider-backed record sidebar/detail surface (sheet + full-page record detail routes).
- Add route-level pending/error states and admin-route detection helpers (e.g. header suppression, active-route matching).
Reviewed changes
Copilot reviewed 46 out of 48 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/admin/admin-shell.test.ts | Adds tests for admin route helper behaviors (active matching + pathname detection). |
| src/routeTree.gen.ts | Regenerates TanStack Router route tree to include new admin entity/detail routes. |
| src/routes/admin/underwriting/route.tsx | Adds admin route pending/error states and updates permission gating. |
| src/routes/admin/route.tsx | Wraps admin routes with the new layout + record sidebar provider, and keeps typed search validation. |
| src/routes/admin/properties/route.tsx | Adds properties list route with table shell + nested record route outlet behavior. |
| src/routes/admin/properties/$recordid.tsx | Adds properties record detail page route. |
| src/routes/admin/mortgages/route.tsx | Adds mortgages list route with table shell + nested record route outlet behavior. |
| src/routes/admin/mortgages/$recordid.tsx | Adds mortgages record detail page route. |
| src/routes/admin/listings/route.tsx | Adds listings list route with table shell + nested record route outlet behavior. |
| src/routes/admin/listings/$recordid.tsx | Adds listings record detail page route. |
| src/routes/admin/deals/route.tsx | Updates deals route to support nested record detail routes + admin pending/error states. |
| src/routes/admin/deals/$recordid.tsx | Adds deals record detail page route. |
| src/routes/admin/$entitytype.tsx | Adds generic entity list route with entity-type validation via registry. |
| src/routes/admin/$entitytype.$recordid.tsx | Adds generic entity record detail route with entity-type validation via registry. |
| src/routes/__root.tsx | Suppresses the global header on admin routes. |
| src/lib/admin-routes.ts | Adds isAdminPathname helper for admin route detection. |
| src/lib/admin-detail-search.ts | Adds typed parsing for admin detail-sheet search params. |
| src/hooks/useAdminDetailSheet.tsx | Introduces a compatibility hook that now drives the record sidebar via context. |
| src/components/workos-user.tsx | Enables view transitions for sign-in/sign-up links. |
| src/components/header.tsx | Enables view transitions for header navigation links. |
| src/components/admin/shell/team-switcher.tsx | Adds admin sidebar team switcher UI. |
| src/components/admin/shell/RecordSidebarProvider.tsx | Adds context provider managing record sidebar open state + navigation stack. |
| src/components/admin/shell/RecordSidebar.tsx | Implements the record detail sheet surface (tabs, relations, linked navigation). |
| src/components/admin/shell/nav-user.tsx | Adds admin sidebar user nav UI (unused by the new layout). |
| src/components/admin/shell/nav-projects.tsx | Adds admin sidebar “projects” nav UI (unused by the new layout). |
| src/components/admin/shell/nav-main.tsx | Adds admin sidebar “platform” nav UI (unused by the new layout). |
| src/components/admin/shell/FieldRenderer.tsx | Adds a generic field renderer for record detail pages. |
| src/components/admin/shell/EntityTable.tsx | Adds a generic entity table with loading/empty/error states and row click support. |
| src/components/admin/shell/entity-registry.ts | Defines admin domains/entities and provides navigation + active-route helpers. |
| src/components/admin/shell/DashboardShell.tsx | Adds a thin wrapper around AdminLayout (legacy compatibility). |
| src/components/admin/shell/AdminUserMenu.tsx | Adds the admin sidebar user menu (auth-aware). |
| src/components/admin/shell/AdminRouteStates.tsx | Adds shared admin route error boundary + skeleton states. |
| src/components/admin/shell/AdminRecordDetailPage.tsx | Adds full-page wrapper around the record detail surface. |
| src/components/admin/shell/AdminNavigation.tsx | Implements registry-driven admin sidebar navigation. |
| src/components/admin/shell/AdminLayout.tsx | Introduces the canonical admin layout (sidebar + header + breadcrumb + content). |
| src/components/admin/shell/AdminDetailSheet.tsx | Adds the admin record detail sheet mounted under /admin. |
| src/components/admin/shell/AdminBreadcrumbs.tsx | Adds registry-aware admin breadcrumbs. |
| src/components/admin/shell/admin-spa-href.ts | Adds helper to detect in-app admin SPA links. |
| specs/ENG-228/tasks.md | Adds ENG-228 task checklist. |
| specs/ENG-228/chunks/manifest.md | Adds chunk manifest for ENG-228. |
| specs/ENG-228/chunks/chunk-01-shell-foundation/tasks.md | Adds per-chunk task checklist. |
| specs/ENG-228/chunks/chunk-01-shell-foundation/status.md | Records status and tooling notes for the chunk. |
| specs/ENG-228/chunks/chunk-01-shell-foundation/context.md | Adds implementation context/constraints for ENG-228. |
| package.json | Updates lucide-react and adds zustand. |
| CLAUDE.md | Updates repository guidance/context documentation. |
| bun.lock | Updates lockfile for dependency changes. |
| .gitnexus/meta.json | Removes local indexing metadata from the repo. |
| .gitnexus/lbug | Adds an empty/placeholder .gitnexus artifact. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Owner
Author
|
@coderabbitai please re-review You didn't leave any comments |
Contributor
|
🧠 Learnings used✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/admin/route.tsx (1)
10-11:⚠️ Potential issue | 🟡 MinorRemove unused
validateSearchfrom the admin route configuration.The detail sheet state has been moved to
RecordSidebarProvidercontext, and no component in the admin route tree accesses the validated search parameters. TheuseAdminDetailSheethook is explicitly designed as a "backwards-compatible wrapper" that uses context-based state instead of URL search params. ThevalidateSearchdefinition and theparseAdminDetailSearchimport are dead code and should be removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/admin/route.tsx` around lines 10 - 11, Remove the unused validateSearch property and its parseAdminDetailSearch import from the admin route configuration because detail sheet state is now managed by RecordSidebarProvider and accessed via useAdminDetailSheet; locate the validateSearch: (search: Record<string, unknown>) => parseAdminDetailSearch(search) entry in the admin route (route.tsx) and delete that line, and also remove the corresponding parseAdminDetailSearch import at the top of the file and any unused references to it so the dead code is eliminated.
🧹 Nitpick comments (5)
src/components/admin/shell/FieldRenderer.tsx (1)
83-89: Consider parameterizing the locale for better i18n support.The
Intl.NumberFormatuses a hardcoded"en-US"locale. For better internationalization, consider accepting an optional locale parameter or usingundefinedto let the browser use the user's default locale.♻️ Optional: Use browser default locale
if (typeof value === "number") { return Number.isInteger(value) - ? new Intl.NumberFormat("en-US").format(value) - : new Intl.NumberFormat("en-US", { + ? new Intl.NumberFormat().format(value) + : new Intl.NumberFormat(undefined, { maximumFractionDigits: 2, }).format(value); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/admin/shell/FieldRenderer.tsx` around lines 83 - 89, The number formatting in FieldRenderer.tsx currently hardcodes "en-US" (the branch that checks typeof value === "number"); change the logic to accept an optional locale parameter (e.g., locale?: string) or default to undefined so the runtime uses the user's locale, and pass that variable into both Intl.NumberFormat calls instead of "en-US"; update the function/component signature that renders the field (where value is handled) to accept and forward this locale argument and ensure callers propagate or omit it for browser defaults.src/hooks/useAdminDetailSheet.tsx (2)
36-44: Silent no-op whenrouteEntityTypeis unavailable may obscure bugs.When
getAdminEntityByPathnamereturnsundefined(e.g., user is on an unrecognized admin path),open(recordId)silently does nothing. Callers likeAdminEntityActionsCell(entity-table-columns.tsx) won't receive feedback that the action failed.Consider at minimum a
console.warnin development, or returning a boolean success indicator so callers can react appropriately.💡 Optional: Add development warning for missing entity type
open: (recordId) => { if (!routeEntityType) { + if (process.env.NODE_ENV === "development") { + console.warn( + "[useAdminDetailSheet] open() called but no routeEntityType resolved from pathname:", + pathname + ); + } return; } open({ entityType: routeEntityType, recordId, }); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useAdminDetailSheet.tsx` around lines 36 - 44, The open(recordId) path in useAdminDetailSheet.tsx currently silently returns when routeEntityType is falsy; update the open function (the exported open in useAdminDetailSheet.tsx) to either (a) emit a development-only warning (e.g., console.warn) mentioning getAdminEntityByPathname returned undefined and the recordId, and/or (b) change its signature to return a boolean success value so callers (like AdminEntityActionsCell in entity-table-columns.tsx) can react to failure; ensure you update callers to handle the boolean if you choose that approach and keep the warning conditional on non-production environments.
46-64:setSearchhas multiple silent early-returns.Lines 52–54 and 57–59 silently return when
recordIdorresolvedEntityTypeis missing. While this may be intentional for backwards compatibility, it can make debugging difficult when callers expect the sidebar to open but nothing happens.If these are expected edge cases, the JSDoc comment could document them. Otherwise, consider adding development-only warnings similar to
open().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useAdminDetailSheet.tsx` around lines 46 - 64, The setSearch handler in useAdminDetailSheet.tsx currently performs silent early returns when recordId or resolvedEntityType is missing (inside setSearch: ({ detailOpen, entityType: nextEntityType, recordId }) => { ... }), which hampers debugging; either add development-only warnings/log calls (using the same mechanism used in open()) right before the early returns for missing recordId and missing resolvedEntityType to surface unexpected calls, or update the JSDoc for setSearch to explicitly document these allowed no-op edge cases so callers know the behavior—apply the chosen change near the early-return checks in setSearch and keep behavior otherwise the same.src/components/admin/shell/RecordSidebar.tsx (2)
282-296: Linked record navigation may fail ifobjectDefshasn't loaded yet.The
onNavigatecallback accessesobjectDefsfrom the closure, but if the query is still pending (objectDefs === undefined), theentityTypewill resolve toundefined. This could cause the sidebar to open with incomplete reference data.Consider guarding this or using optional chaining with a fallback:
💡 Safer navigation when objectDefs is loading
onNavigate={(recordId, linkedRecordKind, linkedObjectDefId) => { + const linkedObjectDef = objectDefs?.find( + (candidate) => String(candidate._id) === linkedObjectDefId + ); push({ - entityType: - objectDefs - ? getAdminEntityForObjectDef( - objectDefs.find( - (candidate) => - String(candidate._id) === linkedObjectDefId - ) ?? {} - )?.entityType - : undefined, + entityType: linkedObjectDef + ? getAdminEntityForObjectDef(linkedObjectDef)?.entityType + : undefined, objectDefId: linkedObjectDefId, recordId, recordKind: linkedRecordKind, }); }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/admin/shell/RecordSidebar.tsx` around lines 282 - 296, The onNavigate callback in RecordSidebar.tsx currently reads objectDefs from the closure and may pass entityType: undefined to push when objectDefs is still loading; update the onNavigate handler (the arrow passed to onNavigate that calls push and uses getAdminEntityForObjectDef) to guard against objectDefs being undefined by either early-returning (do nothing) until objectDefs is available, or compute entityType with optional chaining and a safe fallback (e.g., the current record/entityType) before calling push; ensure you reference the getAdminEntityForObjectDef(objectDefs.find(...)) lookup and the push({ entityType, objectDefId: linkedObjectDefId, recordId, recordKind: linkedRecordKind }) call when making the change.
592-594: Consider explicit date formatting options for consistency.
toLocaleString()without options relies on browser locale and timezone, which can vary across admin users. For audit/history purposes, consider specifying explicit options or using a shared formatter:💡 Example with explicit options
function formatTimestamp(timestamp: number) { - return new Date(timestamp).toLocaleString(); + return new Date(timestamp).toLocaleString(undefined, { + dateStyle: "medium", + timeStyle: "short", + }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/admin/shell/RecordSidebar.tsx` around lines 592 - 594, The formatTimestamp function uses toLocaleString() with no options which causes inconsistent date/time across browsers; update formatTimestamp to use an explicit formatter (e.g., Intl.DateTimeFormat with a chosen locale and options or format in UTC) so timestamps are consistent for audit/history—modify the function formatTimestamp(timestamp: number) to pass explicit DateTimeFormat options (or use a shared formatter util) that define timezone, dateStyle/timeStyle or specific components, and ensure all callers use that single formatter for uniform output.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/routes/admin/route.tsx`:
- Around line 10-11: Remove the unused validateSearch property and its
parseAdminDetailSearch import from the admin route configuration because detail
sheet state is now managed by RecordSidebarProvider and accessed via
useAdminDetailSheet; locate the validateSearch: (search: Record<string,
unknown>) => parseAdminDetailSearch(search) entry in the admin route (route.tsx)
and delete that line, and also remove the corresponding parseAdminDetailSearch
import at the top of the file and any unused references to it so the dead code
is eliminated.
---
Nitpick comments:
In `@src/components/admin/shell/FieldRenderer.tsx`:
- Around line 83-89: The number formatting in FieldRenderer.tsx currently
hardcodes "en-US" (the branch that checks typeof value === "number"); change the
logic to accept an optional locale parameter (e.g., locale?: string) or default
to undefined so the runtime uses the user's locale, and pass that variable into
both Intl.NumberFormat calls instead of "en-US"; update the function/component
signature that renders the field (where value is handled) to accept and forward
this locale argument and ensure callers propagate or omit it for browser
defaults.
In `@src/components/admin/shell/RecordSidebar.tsx`:
- Around line 282-296: The onNavigate callback in RecordSidebar.tsx currently
reads objectDefs from the closure and may pass entityType: undefined to push
when objectDefs is still loading; update the onNavigate handler (the arrow
passed to onNavigate that calls push and uses getAdminEntityForObjectDef) to
guard against objectDefs being undefined by either early-returning (do nothing)
until objectDefs is available, or compute entityType with optional chaining and
a safe fallback (e.g., the current record/entityType) before calling push;
ensure you reference the getAdminEntityForObjectDef(objectDefs.find(...)) lookup
and the push({ entityType, objectDefId: linkedObjectDefId, recordId, recordKind:
linkedRecordKind }) call when making the change.
- Around line 592-594: The formatTimestamp function uses toLocaleString() with
no options which causes inconsistent date/time across browsers; update
formatTimestamp to use an explicit formatter (e.g., Intl.DateTimeFormat with a
chosen locale and options or format in UTC) so timestamps are consistent for
audit/history—modify the function formatTimestamp(timestamp: number) to pass
explicit DateTimeFormat options (or use a shared formatter util) that define
timezone, dateStyle/timeStyle or specific components, and ensure all callers use
that single formatter for uniform output.
In `@src/hooks/useAdminDetailSheet.tsx`:
- Around line 36-44: The open(recordId) path in useAdminDetailSheet.tsx
currently silently returns when routeEntityType is falsy; update the open
function (the exported open in useAdminDetailSheet.tsx) to either (a) emit a
development-only warning (e.g., console.warn) mentioning
getAdminEntityByPathname returned undefined and the recordId, and/or (b) change
its signature to return a boolean success value so callers (like
AdminEntityActionsCell in entity-table-columns.tsx) can react to failure; ensure
you update callers to handle the boolean if you choose that approach and keep
the warning conditional on non-production environments.
- Around line 46-64: The setSearch handler in useAdminDetailSheet.tsx currently
performs silent early returns when recordId or resolvedEntityType is missing
(inside setSearch: ({ detailOpen, entityType: nextEntityType, recordId }) => {
... }), which hampers debugging; either add development-only warnings/log calls
(using the same mechanism used in open()) right before the early returns for
missing recordId and missing resolvedEntityType to surface unexpected calls, or
update the JSDoc for setSearch to explicitly document these allowed no-op edge
cases so callers know the behavior—apply the chosen change near the early-return
checks in setSearch and keep behavior otherwise the same.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c59f5820-6f07-4a5e-8957-d68a7b9b1238
📒 Files selected for processing (12)
src/components/admin/shell/AdminDetailSheet.tsxsrc/components/admin/shell/AdminRecordDetailPage.tsxsrc/components/admin/shell/FieldRenderer.tsxsrc/components/admin/shell/RecordSidebar.tsxsrc/components/admin/shell/RecordSidebarProvider.tsxsrc/components/admin/shell/entity-registry.tssrc/hooks/useAdminDetailSheet.tsxsrc/routes/admin/$entitytype.tsxsrc/routes/admin/listings/route.tsxsrc/routes/admin/mortgages/route.tsxsrc/routes/admin/properties/route.tsxsrc/routes/admin/route.tsx
- Move admin detail state into a shared sidebar context - Render a richer record detail surface with tabs, fields, and navigation - Add entity registry support for properties and object-def lookup
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/components/admin/shell/FieldRenderer.tsx (2)
44-48: Array key includes index, but consider edge cases with duplicate items.The key
${String(item)}-${index}handles duplicates correctly by including the index. However, ifitemis an object or complex type,String(item)yields[object Object], making keys like[object Object]-0,[object Object]-1. This works but may cause React reconciliation issues if items are reordered.Since this is a display-only component with no reordering, this is acceptable. Just noting for awareness.
♻️ Alternative: Use index alone for simpler keys
- <Badge key={`${String(item)}-${index}`} variant="secondary"> + <Badge key={index} variant="secondary">Since arrays displayed here are read-only snapshots with no reordering, a simple index key is sufficient and avoids the
String(item)edge case.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/admin/shell/FieldRenderer.tsx` around lines 44 - 48, The array item keys use `${String(item)}-${index}` in the value.map rendering which can produce `[object Object]-<index>` for non-primitive items; since this is a read-only snapshot with no reordering, replace the composite key with index-only to avoid misleading String(item) results—update the map callback that renders <Badge> (the value.map(...) that calls formatScalarValue(item, fieldType)) to use index as the key.
77-89: Minor locale inconsistency between date and number formatting.Dates use
toLocaleString()(browser locale) while numbers explicitly use"en-US". For admin UI consistency, consider either:
- Using a fixed locale for dates too, or
- Using the browser locale for both
This is a minor consistency note and acceptable as-is for an internal admin surface.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/admin/shell/FieldRenderer.tsx` around lines 77 - 89, The date formatting uses date.toLocaleString() (browser locale) while number formatting hardcodes "en-US"; update FieldRenderer.tsx to use a consistent locale for both by introducing a single locale variable (e.g., derive from navigator.language or a fixed "en-US") and use it for the date and number formatting paths—replace date.toLocaleString() with Intl.DateTimeFormat(locale).format(date) (or pass locale into toLocaleString) and replace the hardcoded "en-US" in the Intl.NumberFormat constructors with the same locale variable so both formats follow the same locale.src/components/admin/shell/AdminDetailSheet.tsx (1)
5-6:AdminDetailSheetis a thin wrapper that adds no value; consider removing it in favor of directRecordSidebarusage.
AdminDetailSheet(lines 5-6) simply renders<RecordSidebar />without any configuration. SinceRecordSidebaralready accepts an optionaladaptersprop and is used directly elsewhere in the codebase without adapters (e.g.,src/routes/demo/crm/route.tsx:116), the wrapper component can be removed. Updatesrc/routes/admin/route.tsx:31to render<RecordSidebar />directly instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/admin/shell/AdminDetailSheet.tsx` around lines 5 - 6, Remove the useless wrapper component AdminDetailSheet and replace its usages with RecordSidebar directly: delete the AdminDetailSheet function (the export named AdminDetailSheet) and update any imports/usages (e.g., where AdminDetailSheet is rendered in the admin route) to import and render RecordSidebar instead; ensure you remove the now-unused AdminDetailSheet export/import and keep RecordSidebar's existing props (adapters) unchanged so behavior remains identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/admin/shell/AdminDetailSheet.tsx`:
- Around line 5-6: Remove the useless wrapper component AdminDetailSheet and
replace its usages with RecordSidebar directly: delete the AdminDetailSheet
function (the export named AdminDetailSheet) and update any imports/usages
(e.g., where AdminDetailSheet is rendered in the admin route) to import and
render RecordSidebar instead; ensure you remove the now-unused AdminDetailSheet
export/import and keep RecordSidebar's existing props (adapters) unchanged so
behavior remains identical.
In `@src/components/admin/shell/FieldRenderer.tsx`:
- Around line 44-48: The array item keys use `${String(item)}-${index}` in the
value.map rendering which can produce `[object Object]-<index>` for
non-primitive items; since this is a read-only snapshot with no reordering,
replace the composite key with index-only to avoid misleading String(item)
results—update the map callback that renders <Badge> (the value.map(...) that
calls formatScalarValue(item, fieldType)) to use index as the key.
- Around line 77-89: The date formatting uses date.toLocaleString() (browser
locale) while number formatting hardcodes "en-US"; update FieldRenderer.tsx to
use a consistent locale for both by introducing a single locale variable (e.g.,
derive from navigator.language or a fixed "en-US") and use it for the date and
number formatting paths—replace date.toLocaleString() with
Intl.DateTimeFormat(locale).format(date) (or pass locale into toLocaleString)
and replace the hardcoded "en-US" in the Intl.NumberFormat constructors with the
same locale variable so both formats follow the same locale.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 80d43c33-6176-451d-8269-5340993e4e88
📒 Files selected for processing (12)
src/components/admin/shell/AdminDetailSheet.tsxsrc/components/admin/shell/AdminRecordDetailPage.tsxsrc/components/admin/shell/FieldRenderer.tsxsrc/components/admin/shell/RecordSidebar.tsxsrc/components/admin/shell/RecordSidebarProvider.tsxsrc/components/admin/shell/entity-registry.tssrc/hooks/useAdminDetailSheet.tsxsrc/routes/admin/$entitytype.tsxsrc/routes/admin/listings/route.tsxsrc/routes/admin/mortgages/route.tsxsrc/routes/admin/properties/route.tsxsrc/routes/admin/route.tsx
🚧 Files skipped from review as they are similar to previous changes (8)
- src/routes/admin/properties/route.tsx
- src/routes/admin/listings/route.tsx
- src/routes/admin/mortgages/route.tsx
- src/components/admin/shell/AdminRecordDetailPage.tsx
- src/components/admin/shell/RecordSidebarProvider.tsx
- src/components/admin/shell/entity-registry.ts
- src/hooks/useAdminDetailSheet.tsx
- src/routes/admin/route.tsx
This was referenced Apr 3, 2026
Merged
Connorbelez
added a commit
that referenced
this pull request
Apr 20, 2026
eng-228_eng-229_eng-230 feat(admin): build shell navigation and route states Add provider-backed admin record sidebar - Move admin detail state into a shared sidebar context - Render a richer record detail surface with tabs, fields, and navigation - Add entity registry support for properties and object-def lookup <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a tabbed record detail sidebar with Details, Relations, Notes, Files, and History. * Improved field rendering: formatted dates, numbers, booleans, arrays, and object previews. * Added in-app sidebar navigation with stack-based open/push/replace/back controls. * **Refactor** * Moved detail UI into a dedicated surface and sidebar provider; routes now render the table separately. * Consolidated admin entity lookup and routing resolution. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
eng-228_eng-229_eng-230
feat(admin): build shell navigation and route states
Add provider-backed admin record sidebar
Summary by CodeRabbit
New Features
Refactor