Skip to content

Extract the renderGrid local-state wiring (sort/widths + repaint) — four hand-rolled copies #167

Description

@BorisTyshkevich

Found during #164's code review (D9). The complex surface (sortable/resizable grid) is already shared — renderGrid — but its state wiring ({sort, widths} holder + repaint-on-sort closure) is now hand-rolled at four sites, each slightly different:

  • renderTable (src/ui/results.js:~670) — global app.state.resultSort + full renderResults repaint
  • openRowsViewer (src/ui/results.js:~356) — entry-persisted sort/widths + local paint
  • expandDataPane (src/ui/results.js:~709) — local sort/widths inside a 3-view paint
  • dashboard table tile (src/ui/dashboard.js:~172) — slot-persisted, with a schemaKey reset (the only copy that has one)

Goal

Extract the repeated state-and-repaint wiring around renderGrid() into a small shared adapter, and move the grid renderer out of results.js into its own module.

This is a behavior-preserving preparation step for #166. The new abstraction must support the current three grid consumers and become the table-panel foundation for #166 without knowing anything about panels, dashboards, or saved-query configuration.

Why now

renderGrid() already centralizes the complex table DOM and interactions:

  • row sorting;
  • sortable headers;
  • column resize handles;
  • fixed-width application;
  • row-number column;
  • numeric alignment;
  • cell click handling;
  • display caps.

However, every consumer still hand-wires the same controller responsibilities:

  1. create or locate sort state;
  2. create or locate column-width state;
  3. mutate sort state on header click;
  4. repaint the correct UI scope;
  5. preserve width state across repaint;
  6. decide how long state should live;
  7. decide when stale state should reset.

There are currently three consumers on main:

  • main results table;
  • script-result rows viewer;
  • detached Data pane.

PR #168 will not be merged. Its dashboard table implementation is not a fourth source to preserve, but #166 will add a real fourth consumer through the table panel type.

The shape is now sufficiently clear to extract before #166 rather than duplicating the wiring again inside the panel registry.

Current duplication

Main results table

State ownership is split:

r.colWidths = r.colWidths || {};

renderGrid({
  columns: r.columns,
  rows: r.rows,
  sort: app.state.resultSort,
  onSort: (col, dir) => {
    app.state.resultSort = { col, dir };
    renderResults(app);
  },
  widths: r.colWidths,
  onCell: (...args) => openCellDetail(app, ...args),
  cap: visCap(r),
});

Characteristics:

  • sort state belongs to global application state;
  • widths belong to the result object;
  • sorting repaints the whole results region;
  • streaming and view chrome share that repaint path.

Script-result rows viewer

const sort = entry.viewerSort || (entry.viewerSort = { col: null, dir: 'asc' });
const widths = entry.viewerWidths || (entry.viewerWidths = {});

const paint = () => body.replaceChildren(renderGrid({
  columns: entry.columns || [],
  rows: entry.rows,
  sort,
  onSort: (col, dir) => {
    sort.col = col;
    sort.dir = dir;
    paint();
  },
  widths,
  onCell: (...args) => openCellDetail(app, ...args),
}));

Characteristics:

  • sort and widths persist on the script entry;
  • repaint is local to the drawer body;
  • state survives reopening the same entry;
  • cell details open in the main document.

Detached Data pane

const sort = { col: null, dir: 'asc' };
const widths = {};

const paint = () => {
  // also switches Table / JSON / Chart and destroys Chart.js
  inner.replaceChildren(renderGrid({
    columns: r.columns,
    rows: r.rows,
    sort,
    onSort: (col, dir) => {
      sort.col = col;
      sort.dir = dir;
      paint();
    },
    widths,
    onCell: (...args) => openCellDetail(app, ...args, doc),
    cap: visCap(r),
  }));
};

Characteristics:

  • state lives only for the opened snapshot;
  • repaint is shared with Table / JSON / Chart switching;
  • rendering may happen in another document;
  • cell details must open in that document;
  • chart cleanup is caller-owned and must remain caller-owned.

Problem

The duplicated code is small, but it encodes important behavior. A sort/resize change currently requires separate verification across every consumer.

Likely future changes affected by this duplication include:

A local fix can easily create inconsistent behavior:

  • widths reset after sorting in one surface but not another;
  • stale widths are applied by column position to a different schema;
  • a sort points to a removed column;
  • a detached document opens cell detail in the wrong document;
  • a broad repaint runs where only a local repaint was intended.

Design principles

Keep renderGrid() stateless

renderGrid() should remain a pure DOM renderer driven by explicit inputs.

It must not own:

  • application state;
  • panel state;
  • dashboard slots;
  • saved-query config;
  • schema-change policy;
  • repaint scope;
  • document lifecycle.

Extract a thin wiring adapter

The new helper should centralize only the repeated grid controller behavior:

  • passing sort and widths into renderGrid();
  • updating sort state;
  • invoking the caller-supplied repaint;
  • forwarding caller-specific options.

Suggested shape:

renderGridView({
  columns,
  rows,
  sort,
  setSort,
  widths,
  rerender,
  onCell,
  cap,
})

Equivalent naming is acceptable, but the state seams must remain explicit.

Example behavior:

export function renderGridView({
  columns,
  rows,
  sort,
  setSort,
  widths,
  rerender,
  onCell,
  cap,
}) {
  return renderGrid({
    columns,
    rows,
    sort,
    onSort: (col, dir) => {
      setSort({ col, dir });
      rerender();
    },
    widths,
    onCell,
    cap,
  });
}

The adapter may support mutable state holders where that produces cleaner call sites, but it must not silently choose state ownership.

Caller owns state lifetime

Each surface must continue deciding where sort and widths live:

Caller owns repaint scope

The helper receives a rerender callback. It must not call renderResults() or manipulate a specific container directly.

This preserves:

Schema reset is separate

The renderer should not calculate schemaKey() or reset state automatically.

A small pure helper is allowed if useful:

ensureGridState(holder, schemaKey)

or:

gridStateForSchema(previous, key)

Possible contract:

{
  key,
  sort: { col: null, dir: 'asc' },
  widths: {},
}

But schema-reset policy belongs to the state owner.

For the current three consumers, preserve existing behavior unless a stale-state bug is explicitly demonstrated. #166 can use schema-keyed state for table panels.

Module extraction

Move grid-specific code out of src/ui/results.js into a dedicated module, for example:

src/ui/grid-render.js

Move together:

  • renderGrid;
  • column resize logic;
  • fixed-width application;
  • width-key mapping used by the data grid;
  • colResizeWidth;
  • the new state-wiring adapter.

Do not move unrelated script-grid behavior unless sharing the same low-level resize primitive is clearly cleaner.

The extraction must avoid a dependency from the grid module back into results.js.

This prepares #166's required renderer split and prevents:

results.js → panels.js → grid/logs renderer → results.js

circular dependencies.

Proposed API

Exact names may differ, but the module should expose responsibilities approximately like this:

export function renderGrid(options) {}
export function renderGridView(options) {}
export function colResizeWidth(startWidth, deltaX, scale) {}

Optional schema-state helper:

export function gridStateForSchema(current, key) {}

renderGridView() inputs:

{
  columns,
  rows,
  sort,
  setSort,
  widths,
  rerender,
  onCell,
  cap,
}

Required behavior:

  • setSort() is called exactly once per sort click;
  • rerender() is called after sort state is updated;
  • the same widths object is reused;
  • missing optional handlers degrade safely;
  • no network request or application action occurs inside the helper.

Consumer migration

Main results table

Preserve:

  • app.state.resultSort;
  • r.colWidths;
  • full renderResults(app) repaint;
  • current visCap(r);
  • current cell-detail behavior.

Expected shape:

return renderGridView({
  columns: r.columns,
  rows: r.rows,
  sort: app.state.resultSort,
  setSort: (next) => {
    app.state.resultSort = next;
  },
  widths: r.colWidths,
  rerender: () => renderResults(app),
  onCell: (name, type, value) => openCellDetail(app, name, type, value),
  cap: visCap(r),
});

Script-result rows viewer

Preserve:

  • state stored on entry;
  • local body repaint;
  • current cell-detail behavior;
  • default cap.

Expected shape:

renderGridView({
  columns: entry.columns || [],
  rows: entry.rows,
  sort: entry.viewerSort,
  setSort: (next) => {
    entry.viewerSort = next;
  },
  widths: entry.viewerWidths,
  rerender: paint,
  onCell: (name, type, value) => openCellDetail(app, name, type, value),
});

The implementation may keep a stable mutable sort object if existing tests depend on object identity, but this should be deliberate and documented.

Detached Data pane

Preserve:

  • local sort and width state;
  • caller-owned Table / JSON / Chart repaint;
  • Chart.js cleanup;
  • detached-document cell detail;
  • current cap.

The helper must not know that the repaint also handles charts.

#166 integration contract

This issue should land before #166.

The table panel arm in #166 should be able to call the shared adapter directly:

renderPanel({
  result,
  state,
  rerender,
  cap,
  ...
}) {
  return {
    node: renderGridView({
      columns: result.columns,
      rows: result.rows,
      sort: state.sort,
      setSort: (next) => {
        state.sort = next;
      },
      widths: state.widths,
      rerender,
      onCell: ...,
      cap,
    }),
  };
}

This issue must not add panel-specific code, registry knowledge, or saved-query migration.

Acceptance criteria

  • Grid rendering and resize helpers are moved out of results.js into a dedicated UI module.
  • renderGrid() remains stateless and behavior-equivalent.
  • A shared adapter centralizes sort-update plus repaint wiring.
  • Main results table uses the shared adapter.
  • Script-result rows viewer uses the shared adapter.
  • Detached Data pane uses the shared adapter.
  • Each consumer preserves its existing state lifetime.
  • Each consumer preserves its existing repaint scope.
  • Column widths survive sorting exactly as before.
  • Detached-document cell details still open in the correct document.
  • No network behavior changes.
  • No panel, dashboard, or saved-query knowledge is introduced.
  • The exported API is sufficient for Panels: visualization registry + Panel drawer tab + Library panel field #166's table panel without another copy of the wiring.
  • No new runtime dependency.
  • Changed modules retain the repository coverage gate.

Tests

Shared adapter

  • Sort click calls setSort({col, dir}).
  • Repaint occurs after state update.
  • Width object is forwarded unchanged.
  • columns, rows, onCell, and cap are forwarded unchanged.
  • Ascending/descending values supplied by renderGrid() reach setSort() correctly.
  • Missing optional inputs degrade safely if the API allows them.

Main results table

  • Sorting still updates app.state.resultSort.
  • Sorting still repaints the results region.
  • Resized widths remain after sorting.
  • Current row cap is preserved.
  • Cell detail still opens.

Script rows viewer

  • Sorting repaints only the viewer body.
  • Sort and widths remain associated with the entry.
  • Reopening the same entry preserves current behavior.
  • Cell detail stacking and Escape behavior remain unchanged.

Detached Data pane

  • Sorting repaints the current table view.
  • Switching Table / JSON / Chart still works.
  • Chart cleanup remains intact.
  • Sort and widths remain local to the snapshot.
  • Cell detail uses the detached document.

Regression

  • Existing renderGrid sorting tests remain valid.
  • Existing column-resize tests remain valid.
  • Existing result, script-viewer, and detached-pane tests pass without changed user-visible behavior.

Non-goals

Sequencing

  1. Land this issue as a small behavior-preserving preparation PR.
  2. Start Panels: visualization registry + Panel drawer tab + Library panel field #166 from the extracted grid module and adapter.
  3. Add the table panel arm as the next real consumer.
  4. Keep chart/footer extraction either in the same preparation series or as isolated early commits in Panels: visualization registry + Panel drawer tab + Library panel field #166, but do not duplicate grid wiring inside Panels: visualization registry + Panel drawer tab + Library panel field #166.

Suggested title

Extract renderGrid state/repaint wiring before Panels (#166)

Metadata

Metadata

Assignees

No one assigned

    Labels

    inboxFiled mid-task; not yet triaged into the roadmap

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions