Skip to content

DT-90: Visualize single row data in drawer view #91

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
abdotop wants to merge 1 commit into
base: master
Choose a base branch
from
Open

DT-90: Visualize single row data in drawer view #91

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
207 changes: 162 additions & 45 deletions web/pages/DeploymentPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ type AnyRecord = Record<string, unknown>
const schema = api['GET/api/deployment/schema'].signal()
// API signal for table data
export const tableData = api['POST/api/deployment/table/data'].signal()
export const rowDetailsData = api['POST/api/deployment/table/data'].signal()
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two separate signals (tableData and rowDetailsData) are created from the same API endpoint. This means they will have independent state and cache. If the same row exists in both datasets, they could become out of sync. Consider reusing the tableData signal or implementing a shared cache mechanism to avoid inconsistencies.

Suggested change
export const rowDetailsData = api['POST/api/deployment/table/data'].signal()
export const rowDetailsData = tableData

Copilot uses AI. Check for mistakes.

// Effect to fetch schema when deployment URL changes
effect(() => {
Expand Down Expand Up @@ -328,16 +329,28 @@ const EmptyRow = ({ colSpan }: { colSpan: number }) => (

const DataRow = (
{ row, columns, index }: { row: AnyRecord; columns: string[]; index: number },
) => (
<tr class='hover:bg-base-200/50'>
<RowNumberCell index={index} />
{columns.map((key, i) => (
<td key={i} class='align-top min-w-[8rem] max-w-[20rem]'>
<TableCell value={row[key]} />
</td>
))}
</tr>
)
) => {
const tableName = url.params.table || schema.data?.tables?.[0]?.table
const tableDef = schema.data?.tables?.find((t) => t.table === tableName)
const pk = tableDef?.columns?.[0]?.name
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assumption that the first column is the primary key may not always be correct. Many databases have primary keys defined by constraints rather than column order. Consider using schema metadata to identify the actual primary key column, or add a fallback mechanism to handle tables where the first column is not the primary key.

Suggested change
const pk = tableDef?.columns?.[0]?.name
const pkColumn =
// Prefer an explicit primary key flag if present in the column metadata.
(tableDef?.columns as any[])?.find(
(c) =>
('isPrimaryKey' in c && c.isPrimaryKey) ||
('primaryKey' in c && c.primaryKey) ||
('pk' in c && c.pk),
) ?? tableDef?.columns?.[0]
const pk = (pkColumn as any)?.name as string | undefined

Copilot uses AI. Check for mistakes.
const rowId = pk ? String(row[pk]) : undefined
Comment on lines +333 to +336
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic for determining the primary key and table name is duplicated in both the DataRow component (lines 333-336) and the effect that fetches row details (lines 1082-1084). Consider extracting this logic into a shared helper function to improve maintainability and ensure consistency.

Copilot uses AI. Check for mistakes.

Comment on lines +336 to +337
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the primary key is not found or is undefined, clicking the row will navigate with a 'row-id' parameter set to 'undefined'. This will trigger the effect at line 1077 but won't fetch valid data. Consider disabling the link or not rendering it when rowId is undefined to prevent invalid navigation.

Suggested change
const rowId = pk ? String(row[pk]) : undefined
const rowId = pk && row[pk] != null ? String(row[pk]) : undefined
if (!rowId) {
return (
<tr class='hover:bg-base-200/50'>
<RowNumberCell index={index} />
{columns.map((key, i) => (
<td key={i} class='align-top min-w-[8rem] max-w-[20rem]'>
<TableCell value={row[key]} />
</td>
))}
</tr>
)
}

Copilot uses AI. Check for mistakes.
return (
<A
params={{ drawer: 'view-row', 'row-id': rowId }}
class='contents'
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The clickable row lacks keyboard accessibility. Users navigating with keyboard cannot activate the row since the anchor tag is styled with 'contents' which makes it invisible to keyboard navigation. Add tabIndex={0} to the anchor or consider using a button element with proper keyboard event handlers to ensure the feature is accessible to keyboard users.

Suggested change
class='contents'
class='contents'
tabIndex={0}

Copilot uses AI. Check for mistakes.
>
<tr class='hover:bg-base-200/50 cursor-pointer'>
<RowNumberCell index={index} />
{columns.map((key, i) => (
<td key={i} class='align-top min-w-[8rem] max-w-[20rem]'>
<TableCell value={row[key]} />
</td>
))}
</tr>
</A>
Comment on lines +337 to +351
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrapping a table row in an anchor tag creates invalid HTML structure. The HTML specification requires that elements must be direct children of , , or elements. Placing an element between the table body and the row breaks this requirement. Consider adding an onClick handler to the element instead, or using a button with appropriate styling and ARIA attributes for better accessibility.

Suggested change
return (
<A
params={{ drawer: 'view-row', 'row-id': rowId }}
class='contents'
>
<tr class='hover:bg-base-200/50 cursor-pointer'>
<RowNumberCell index={index} />
{columns.map((key, i) => (
<td key={i} class='align-top min-w-[8rem] max-w-[20rem]'>
<TableCell value={row[key]} />
</td>
))}
</tr>
</A>
const rowUrl = url({
params: { drawer: 'view-row', 'row-id': rowId },
})
const handleActivate = () => {
navigate(rowUrl)
}
const handleKeyDown: JSX.KeyboardEventHandler<HTMLTableRowElement> = (event) => {
if (event.key === 'Enter' || event.key === ' ') {
event.preventDefault()
handleActivate()
}
}
return (
<tr
class='hover:bg-base-200/50 cursor-pointer'
role='link'
tabIndex={0}
onClick={handleActivate}
onKeyDown={handleKeyDown}
>
<RowNumberCell index={index} />
{columns.map((key, i) => (
<td key={i} class='align-top min-w-[8rem] max-w-[20rem]'>
<TableCell value={row[key]} />
</td>
))}
</tr>

Copilot uses AI. Check for mistakes.
)
}

const TableHeader = ({ columns }: { columns: string[] }) => (
<thead class='sticky top-0 bg-base-100 shadow-sm'>
Expand Down Expand Up @@ -1043,14 +1056,117 @@ const ComingSoon = ({ title }: { title: string }) => (
</div>
)

type DrawerTab = 'history' | 'insert'
const schemaPanel = <SchemaPanel />
const TabViews = {
tables: (
<div class='flex flex-1 h-full'>
{schemaPanel}
<DataTable />
</div>
),
queries: (
<div class='flex flex-1 h-full'>
{schemaPanel}
<QueryEditor />
</div>
),
logs: <LogsViewer />,
// Add other tab views here as needed
}
Comment on lines +1059 to +1075
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TabViews constant and its associated schemaPanel variable have been moved from after the Drawer component to before it. While this doesn't affect functionality due to hoisting, it breaks the logical flow of the code where components are typically defined before they're used. Consider keeping related component definitions together for better code organization.

Copilot uses AI. Check for mistakes.

effect(() => {
const rowId = url.params['row-id']
const dep = url.params.dep

if (dep && rowId) {
const tableName = url.params.table || schema.data?.tables?.[0]?.table
const tableDef = schema.data?.tables?.find((t) => t.table === tableName)
Comment on lines +1082 to +1083
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The effect fetches row details whenever dep and rowId are present, but it doesn't validate whether the schema data is loaded. If schema.data is not yet available, tableName and pk will be undefined, causing the fetch to be skipped silently. Consider adding a check for schema.data availability or showing a loading state while the schema is being fetched.

Suggested change
const tableName = url.params.table || schema.data?.tables?.[0]?.table
const tableDef = schema.data?.tables?.find((t) => t.table === tableName)
// Ensure schema data is loaded before attempting to derive table/primary key
if (!schema.data || !schema.data.tables || schema.data.tables.length === 0) {
return
}
const tableName = url.params.table || schema.data.tables[0]?.table
const tableDef = schema.data.tables.find((t) => t.table === tableName)

Copilot uses AI. Check for mistakes.
const pk = tableDef?.columns?.[0]?.name

if (tableName && pk) {
rowDetailsData.fetch({
deployment: dep,
table: tableName,
filter: [{
key: pk,
comparator: '=',
value: rowId,
}],
sort: [],
search: '',
limit: 1,
offset: 0,
})
}
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The effect doesn't clean up or reset rowDetailsData when the row-id parameter is removed. This means stale data from a previously viewed row may still be displayed if the drawer is reopened. Consider clearing rowDetailsData when rowId becomes null or undefined.

Suggested change
}
}
} else {
// Clear row details when there is no selected row to avoid stale data
rowDetailsData.data = undefined
if ('error' in rowDetailsData) {
rowDetailsData.error = undefined as typeof rowDetailsData.error
}

Copilot uses AI. Check for mistakes.
}
})

const RowDetails = () => {
const row = rowDetailsData.data?.rows?.[0]

if (rowDetailsData.pending) {
return (
<div class='flex items-center justify-center p-8'>
<span class='loading loading-spinner loading-md'></span>
</div>
)
}

if (rowDetailsData.error) {
return (
<div class='p-4 text-error'>
Error loading row: {rowDetailsData.error.message}
</div>
)
}

if (!row) {
return (
<div class='p-4 text-base-content/60'>
Row not found
</div>
)
}

return (
<div class='flex flex-col h-full bg-base-100'>
<div class='p-4 border-b border-base-300 flex items-center justify-between sticky top-0 bg-base-100 z-10'>
<h3 class='font-semibold text-lg'>Row Details</h3>
<A
params={{ drawer: null, 'row-id': null }}
replace
class='btn btn-ghost btn-sm btn-circle'
>
<XCircle class='h-5 w-5' />
</A>
</div>
<div class='flex-1 overflow-y-auto p-4 space-y-4'>
{Object.entries(row).map(([key, value]) => (
<div key={key} class='group'>
<div class='text-xs font-semibold text-base-content/50 uppercase tracking-wider mb-1 select-none'>
{key}
</div>
<div class='bg-base-200/50 rounded-lg p-3 font-mono text-sm break-all group-hover:bg-base-200 transition-colors'>
{typeof value === 'object' && value !== null
? JSON.stringify(value, null, 2)
: String(value ?? 'null')}
</div>
</div>
))}
</div>
</div>
)
}

type DrawerTab = 'history' | 'insert' | 'view-row'
const drawerViews: Record<DrawerTab, JSX.Element> = {
history: <QueryHistory />,
insert: <ComingSoon title='Insert Row' />,
'view-row': <RowDetails />,
} as const

const Drawer = () => (
<div class='drawer drawer-end'>
const Drawer = ({ children }: { children: JSX.Element }) => (
<div class='drawer h-full relative' dir='rtl'>
<input
id='drawer-right'
type='checkbox'
Expand All @@ -1062,46 +1178,46 @@ const Drawer = () => (
}
}}
/>
Comment on lines 1178 to 1180
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When closing the drawer via the checkbox onChange handler, only the 'drawer' parameter is cleared, but not the 'row-id' parameter. This leaves the row-id in the URL even after the drawer is closed, which could cause confusion or unexpected behavior if the user reopens the drawer. Ensure 'row-id' is also cleared when closing the drawer.

Copilot uses AI. Check for mistakes.
<div class='drawer-side'>
<div class='drawer-content flex flex-col h-full overflow-hidden' dir='ltr'>
{children}
</div>
<div class='drawer-side z-50 absolute h-full w-full pointer-events-none'>
<label
for='drawer-right'
aria-label='close sidebar'
class='drawer-overlay'
class='drawer-overlay absolute inset-0 pointer-events-auto'
dir='ltr'
>
</label>
<div class='bg-base-200 text-base-content min-h-full w-96 flex flex-col'>
<div class='flex-1 overflow-hidden'>
<div class='drawer-view' data-view='history'>
{drawerViews[url.params.drawer as DrawerTab] || (
<div class='p-4'>
<h3 class='text-lg font-semibold mb-4'>No view selected</h3>
<p>Please select a view from the sidebar.</p>
</div>
)}
<div
class='bg-base-200 text-base-content h-full flex flex-col resize-x overflow-auto pointer-events-auto'
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The resizable drawer implementation using CSS resize-x property may not provide a good user experience across all browsers. The resize handle is often subtle and difficult to discover. Consider implementing a custom resize handle with a visible drag indicator or using a dedicated resize library for better cross-browser compatibility and user experience.

Copilot uses AI. Check for mistakes.
style={{
direction: 'rtl',
width: '400px',
minWidth: '300px',
maxWidth: '80vw',
}}
>
<div
class='flex-1 flex flex-col h-full overflow-hidden'
style={{ direction: 'ltr' }}
>
<div class='flex-1 overflow-hidden'>
<div class='drawer-view h-full' data-view='history'>
Copy link

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The data-view attribute is hardcoded to 'history' but should reflect the actual drawer view being displayed. This could cause confusion when debugging or testing different drawer states. Update this to use the actual drawer parameter value.

Suggested change
<div class='drawer-view h-full' data-view='history'>
<div
class='drawer-view h-full'
data-view={url.params.drawer ?? 'none'}
>

Copilot uses AI. Check for mistakes.
{drawerViews[url.params.drawer as DrawerTab] || (
<div class='p-4'>
<h3 class='text-lg font-semibold mb-4'>No view selected</h3>
<p>Please select a view from the sidebar.</p>
</div>
)}
</div>
</div>
</div>
</div>
</div>
</div>
)

const schemaPanel = <SchemaPanel />
const TabViews = {
tables: (
<div class='flex flex-1 h-full'>
{schemaPanel}
<DataTable />
</div>
),
queries: (
<div class='flex flex-1 h-full'>
{schemaPanel}
<QueryEditor />
</div>
),
logs: <LogsViewer />,
}

export const DeploymentPage = () => {
const tab = (url.params.tab as 'tables' | 'queries' | 'logs') || 'tables'
const view = TabViews[tab]
Expand All @@ -1115,13 +1231,14 @@ export const DeploymentPage = () => {
<main class='flex-1 flex flex-col h-full'>
<TabNavigation activeTab={tab} />
<section class='flex-1 h-full overflow-hidden'>
<div class='h-full bg-base-100 border border-base-300 overflow-hidden flex flex-col lg:flex'>
{view}
</div>
<Drawer>
<div class='h-full bg-base-100 border border-base-300 overflow-hidden flex flex-col lg:flex'>
{view}
</div>
</Drawer>
</section>
</main>
</div>
<Drawer />
</div>
)
}
Loading