feat(table): add row detail sidebar on row click#329
feat(table): add row detail sidebar on row click#329ifeelBALANCED wants to merge 25 commits intomainfrom
Conversation
|
This PR was not deployed automatically as @ifeelBALANCED does not have access to the Railway project. In order to get automatic PR deploys, please add @ifeelBALANCED to your workspace on Railway. |
ifeelBALANCED
requested review from
Rudra-Sankha-Sinhamahapatra,
geekyharsh05 and
letstri
and removed request for
geekyharsh05 and
letstri
…t/table-row-detail-sidebar
There was a problem hiding this comment.
Pull request overview
Adds row-click support to the shared table package and uses it in the desktop table page to open a right-hand “row details” sidebar for inspecting all fields of the clicked row.
Changes:
- Extend
@conar/tablecontext/provider with optionalonRowClickand wire row click handling inTableBody. - Add
detailRowIndexto the table page store and reset logic. - Introduce
RowDetailSidebarUI and render it alongside the table when a row is selected.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/table/src/table-context.ts | Adds optional onRowClick callback to table context type. |
| packages/table/src/provider.tsx | Accepts onRowClick prop and exposes it via context. |
| packages/table/src/body.tsx | Hooks row click to call onRowClick and updates row styling/role. |
| apps/desktop/src/routes/_protected/database/$id/table/index.tsx | Resets detailRowIndex when table/schema changes. |
| apps/desktop/src/routes/_protected/database/$id/table/-store.ts | Adds detailRowIndex to store state and excludes it from persistence. |
| apps/desktop/src/routes/_protected/database/$id/table/-components/table/table.tsx | Toggles detailRowIndex on row click and lays out table + sidebar. |
| apps/desktop/src/routes/_protected/database/$id/table/-components/table/row-detail-sidebar.tsx | New sidebar component rendering column/value pairs for a row. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…t/table-row-detail-sidebar
…t/table-row-detail-sidebar
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { RiCloseLine, RiSidebarFoldLine, RiSideBarLine } from '@remixicon/react' | ||
| import { getEditableValue } from '~/entities/connection/components/table/utils' |
There was a problem hiding this comment.
RiSideBarLine looks like a mis-capitalized Remix Icon export (the library’s naming is typically RiSidebar*). If this named export doesn’t exist, the build will fail. Please verify the correct icon component name (likely RiSidebarLine) and update the import/usage accordingly.
| {column.maxLength && ( | ||
| <span className="ml-1 text-primary"> | ||
| (MAX | ||
| {' '} | ||
| {column.maxLength} | ||
| ) | ||
| </span> | ||
| )} |
There was a problem hiding this comment.
column.maxLength && ... will skip rendering when maxLength is 0 (since it’s a falsy value) even though 0 is a valid number per the type (number | null). Prefer an explicit null/undefined check (maxLength != null) to match the declared type and avoid edge-case omissions.
…nts/table/table.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <Panel | ||
| panelRef={rowDetailPanelRef} | ||
| defaultSize="25%" | ||
| minSize="5%" | ||
| maxSize="50%" | ||
| collapsible | ||
| collapsedSize={0} | ||
| onResize={() => { | ||
| const collapsed = rowDetailPanelRef.current?.isCollapsed() ?? false | ||
| setRowDetailCollapsed(collapsed) | ||
| if (collapsed) | ||
| store.setState(state => ({ ...state, detailRowIndex: null } satisfies typeof state)) | ||
| }} | ||
| className="flex flex-col overflow-hidden border-l border-border bg-background shrink-0" | ||
| data-slot="resizable-panel" | ||
| > | ||
| <RowDetailSidebar | ||
| row={rows[detailRowIndex]!} | ||
| columns={columns} | ||
| onClose={() => | ||
| store.setState(state => ({ | ||
| ...state, | ||
| detailRowIndex: null, | ||
| } satisfies typeof state))} | ||
| onExpand={() => rowDetailPanelRef.current?.expand()} | ||
| onCollapse={() => rowDetailPanelRef.current?.collapse()} | ||
| isCollapsed={rowDetailCollapsed} | ||
| /> | ||
| </Panel> |
There was a problem hiding this comment.
The code uses Panel imported directly from react-resizable-panels library instead of using ResizablePanel from the project's UI component library (@conar/ui/components/resizable). This is inconsistent with how other resizable panels are used throughout the codebase. All other files use ResizablePanel from the UI library wrapper. Using the raw Panel component bypasses any custom styling or behavior that the wrapper provides.
| import { useStore } from '@tanstack/react-store' | ||
| import { useCallback, useEffect, useMemo, useRef } from 'react' | ||
| import { useCallback, useEffect, useMemo, useRef, useState } from 'react' | ||
| import { Panel, useDefaultLayout } from 'react-resizable-panels' |
There was a problem hiding this comment.
When using the Panel component directly from react-resizable-panels, the import of Panel at line 11 should be removed since the convention is to use ResizablePanel from the UI library. However, if Panel needs to be used for access to imperative methods via panelRef, then the type import at line 2 (PanelImperativeHandle) should remain, but the component import should be changed to use ResizablePanel with a ref.
| import { Panel, useDefaultLayout } from 'react-resizable-panels' | |
| import { useDefaultLayout } from 'react-resizable-panels' |
| return ( | ||
| <aside | ||
| className={cn( | ||
| 'flex min-w-0 w-full shrink-0 flex-col border-l border-border bg-background', | ||
| className, | ||
| )} | ||
| aria-label="Row details" | ||
| > | ||
| <div className=" | ||
| flex shrink-0 items-center justify-between border-b border-border px-3 | ||
| py-2 | ||
| " | ||
| > | ||
| <span className="text-sm font-medium text-muted-foreground"> | ||
| Row details | ||
| </span> | ||
| <div className="flex items-center gap-1"> | ||
| {onCollapse && ( | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| className="size-7" | ||
| onClick={onCollapse} | ||
| aria-label="Collapse row details" | ||
| > | ||
| <RiSidebarFoldLine className="size-4" /> | ||
| </Button> | ||
| )} | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| className="size-7" | ||
| onClick={onClose} | ||
| aria-label="Close row details" | ||
| > | ||
| <RiCloseLine className="size-4" /> | ||
| </Button> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
The row detail sidebar lacks keyboard accessibility. Users should be able to close the sidebar using the Escape key, which is a common pattern for dismissible UI elements. Consider adding an onKeyDown handler to the sidebar (or its parent container) that closes the sidebar when the Escape key is pressed.
| if (detailRowIndex === null) | ||
| setRowDetailCollapsed(false) | ||
| }, [detailRowIndex]) | ||
|
|
There was a problem hiding this comment.
When filters or sorting are applied, the row at detailRowIndex may no longer exist or may be a different row. The sidebar should be closed when filters or orderBy change to prevent showing details for the wrong row. Consider adding a useEffect that resets detailRowIndex when filters or orderBy change, similar to how it's reset when table/schema changes.
| useEffect(() => { | |
| if (detailRowIndex !== null) { | |
| store.setState(state => ({ | |
| ...state, | |
| detailRowIndex: null, | |
| } satisfies typeof state)) | |
| setRowDetailCollapsed(true) | |
| } | |
| }, [filters, orderBy, store, detailRowIndex]) |
| <span className="ml-1 text-secondary-foreground">NULLABLE</span> | ||
| )} | ||
| {column.enum && ( | ||
| <span className="rounded-xl ml-1 text-secondary-foreground p-2 bg-secondary">Enum</span> |
There was a problem hiding this comment.
The Enum badge has inconsistent styling compared to other badges in the same row. It has p-2 padding which is much larger than needed for inline text, and uses rounded-xl which creates a pill shape. The other badges (PK, FK, UQ, etc.) use simpler inline text without background. For consistency, either remove the background styling from the Enum badge or apply similar badge styling to all indicators. The current p-2 padding (0.5rem = 8px) is especially excessive for inline text.
| <span className="rounded-xl ml-1 text-secondary-foreground p-2 bg-secondary">Enum</span> | |
| <span className="ml-1 text-secondary-foreground">Enum</span> |
…t/table-row-detail-sidebar
…t/table-row-detail-sidebar
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…t/table-row-detail-sidebar
ifeelBALANCED
force-pushed
the
feat/table-row-detail-sidebar
branch
from
a88f7bb to
89dc561
Compare
…l row index on filter change
Made-with: Cursor
Rudra-Sankha-Sinhamahapatra
force-pushed
the
feat/table-row-detail-sidebar
branch
from
0a6fb9f to
a88f7bb
Compare
…able-row-detail-sidebar
…t/table-row-detail-sidebar
…t/table-row-detail-sidebar Made-with: Cursor # Conflicts: # apps/desktop/src/routes/_protected/connection/$resourceId/table/-components/table/table.tsx
Made-with: Cursor
Description of Changes
onRowClickin the table package anddetailRowIndexin the table page store.Closes #60
Checklist
Screen Recording
Screen.Recording.2026-02-07.at.11.mp4