feat: Add data table explorer blocks to Mosaic dashboards - #668

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

packages/mosaic/src/data-table-explorer/layout.ts (1)

7-12: ⚡ Quick win

Avoid duplicated width literals in class constants.

These class strings repeat pixel values already defined above, which can drift if one side is updated later. Build the classes from the numeric constants to keep a single source of truth.

♻️ Proposed refactor

 export const DATA_TABLE_EXPLORER_ROW_NUMBER_WIDTH_CLASS =
-  'min-w-[40px] w-[40px] max-w-[40px]';
+  `min-w-[${DATA_TABLE_EXPLORER_ROW_NUMBER_COLUMN_WIDTH_PX}px] w-[${DATA_TABLE_EXPLORER_ROW_NUMBER_COLUMN_WIDTH_PX}px] max-w-[${DATA_TABLE_EXPLORER_ROW_NUMBER_COLUMN_WIDTH_PX}px]`;
 export const DATA_TABLE_EXPLORER_DEFAULT_COLUMN_WIDTH_CLASS =
-  'min-w-[140px] w-[140px] max-w-[140px]';
+  `min-w-[${DATA_TABLE_EXPLORER_DEFAULT_COLUMN_WIDTH_PX}px] w-[${DATA_TABLE_EXPLORER_DEFAULT_COLUMN_WIDTH_PX}px] max-w-[${DATA_TABLE_EXPLORER_DEFAULT_COLUMN_WIDTH_PX}px]`;
 export const DATA_TABLE_EXPLORER_UNSUPPORTED_COLUMN_WIDTH_CLASS =
-  'min-w-[104px] w-[104px] max-w-[104px]';
+  `min-w-[${DATA_TABLE_EXPLORER_UNSUPPORTED_COLUMN_WIDTH_PX}px] w-[${DATA_TABLE_EXPLORER_UNSUPPORTED_COLUMN_WIDTH_PX}px] max-w-[${DATA_TABLE_EXPLORER_UNSUPPORTED_COLUMN_WIDTH_PX}px]`;

As per coding guidelines, “Prefer straightforward, maintainable code over clever code”.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/mosaic/src/data-table-explorer/layout.ts` around lines 7 - 12, The
three exported class constants DATA_TABLE_EXPLORER_ROW_NUMBER_WIDTH_CLASS,
DATA_TABLE_EXPLORER_DEFAULT_COLUMN_WIDTH_CLASS, and
DATA_TABLE_EXPLORER_UNSUPPORTED_COLUMN_WIDTH_CLASS duplicate hard-coded pixel
literals; introduce corresponding numeric width constants (e.g.,
ROW_NUMBER_WIDTH = 40, DEFAULT_COLUMN_WIDTH = 140, UNSUPPORTED_COLUMN_WIDTH =
104) and build each class string from those single-source numeric constants
(e.g., generate 'min-w-[{N}px] w-[{N}px] max-w-[{N}px]') so updates only change
the numeric value referenced by the class builders.

packages/mosaic/src/data-table-explorer/DataTableSelector.tsx (1)

18-176: ⚡ Quick win

Document the new public exports.

DataTableSelector, DataTableSelectorEmptyState, getDataTableSelectorReference, and DataTableSelectorProps are public exports of the mosaic package but carry no doc comments. Add brief JSDoc clarifying the contract — notably that onChange/value operate on the dotted table reference (database.schema.table) rather than the bare table name, which is non-obvious from the (tableName: string) signature.

As per coding guidelines, "Public exports in packages/ should be kept generic, composable, documented, and free of app/example-specific assumptions".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/mosaic/src/data-table-explorer/DataTableSelector.tsx` around lines
18 - 176, Add brief JSDoc comments for the public exports DataTableSelector,
DataTableSelectorEmptyState, getDataTableSelectorReference, and
DataTableSelectorProps: document the component purpose, prop contracts, and
especially clarify that DataTableSelectorProps.onChange and the value prop
expect and return the dotted table reference format "database.schema.table" (not
just the bare table name); mention that getDataTableSelectorReference returns
that dotted reference and that DataTableSelectorEmptyState is a presentational
wrapper rendering the same selector in an empty-state layout. Ensure the
comments are concise, placed immediately above each export
(DataTableSelectorProps type, DataTableSelector component,
DataTableSelectorEmptyState function, and getDataTableSelectorReference
function), and avoid app-specific assumptions.

packages/mosaic/src/data-table-explorer/DataTableBlockRenderer.tsx (1)

96-156: ⚡ Quick win

Extract the duplicated selector header.

The header (border-b row with {tableSelector} + optional caption) is repeated verbatim across the loading, not-ready, and ready branches. Extract it once to avoid drift.

♻️ Hoist a single header element

   const tableSelector = (
     <DataTableSelector
       className="max-w-full"
       disabled={readOnly || !onTitleChange}
       onChange={handleTableNameChange}
       tables={selectableTables}
       value={tableName}
     />
   );
+  const header = (
+    <div className="border-border flex shrink-0 items-center gap-2 border-b px-3 py-2">
+      {tableSelector}
+      {caption ? (
+        <div className="min-w-0 truncate text-sm font-medium">{caption}</div>
+      ) : null}
+    </div>
+  );

Then replace each duplicated block with {header}.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/mosaic/src/data-table-explorer/DataTableBlockRenderer.tsx` around
lines 96 - 156, There is duplicated JSX for the selector header across the
loading, not-ready, and ready branches; hoist that markup into a single const
(e.g., const header = (<div className="border-border flex shrink-0 items-center
gap-2 border-b px-3 py-2">{tableSelector}{caption ? <div className="min-w-0
truncate text-sm font-medium">{caption}</div> : null}</div>);) inside
DataTableBlockRenderer.tsx, then replace the three repeated header blocks in the
connection.status === 'loading', connection.status !== 'ready', and the final
return branch with {header} so the header is defined once and reused.