feat(app): add a row-click preference to expand result rows inline - #3117

🔵 minor — clickRow / clickFirstRow now have zero callers and silently mean the opposite of what specs used them for

After this diff every call site moved to openRowSidePanel / clickRowBody, so both helpers are unreferenced across the repo (grep for clickRow(/clickFirstRow( outside this file returns only MetricExplorer.test.tsx's unrelated local helper). They are now a trap: a future spec calling clickFirstRow to reach the side panel gets inline expansion instead — or the side panel, depending on whether rowWhere happens to be set. Delete both and let clickRowBody / openRowSidePanel be the API.

Agreed and done in 85271bb — both helpers are deleted. Confirmed the only remaining clickRow( hits are MetricExplorer.test.tsx's unrelated local helper, so clickRowBody / openRowSidePanel are now the whole API.

🔵 minor — highlightedLineId != null is a proxy for "the side panel is open", but it is non-null in states where no panel renders

highlightedLineId comes from DBSqlRowTableWithSidebar as rowId ?? undefined (the URL rowWhere param), while the panel itself only renders when sourceData && (rowSource === sourceId || !rowSource) (DBSqlRowTableWithSidebar.tsx:119). DBSearchPage never clears rowWhere/rowSource when the source changes, so landing on the search page with a stale ?rowWhere=…&rowSource=<other-source> shows no panel yet disables inline expansion — the first row click opens the panel instead of expanding. Derive the flag from whether the panel is actually mounted (e.g. pass an explicit isSidePanelOpen prop from DBSqlRowTableWithSidebar, which already computes that condition) rather than from highlightedLineId.

Confirmed and fixed in 85271bb. The reachable path is switching source with a row panel open: onCloseSidebar clears both params, but a source switch clears neither, so rowSource goes stale, the panel unmounts, and rowWhere survives.

I fixed it in DBSqlRowTableWithSidebar rather than by adding an isSidePanelOpen prop, since the condition only needs to exist once. The panel is hoisted into a sidePanel element and both the mount and the highlight read from it:

const sidePanel =
  sourceData && (rowSource === sourceId || !rowSource) ? (
    <DBRowSidePanel ... />
  ) : null;
...
{sidePanel}
<DBSqlRowTable highlightedLineId={sidePanel ? (rowId ?? undefined) : undefined} ... />

This keeps sourceData narrowed (a plain boolean would not), avoids duplicating the predicate, and needs no new prop on DBRowTable. It also fixes the pre-existing half of the bug you did not mention: the table was highlighting a row whose panel was not on screen.

🔵 minor — Expanded rows still offer "Open in side panel" on tables that have no side panel

This PR deliberately hides the row hover button when onRowDetailsClick is absent so the ClickHouse dashboard's slow-query list doesn't "offer a dead action" (DBRowTable.tsx:1888), but ExpandedLogRow unconditionally renders the maximize button — and ClickhousePage.tsx:800 renders no DBRowSidePanel at all (no DBRowSidePanel/rowWhere reference anywhere in that file), so clicking it just writes rowWhere/rowSource into the URL and nothing opens. Now that a plain row click reaches this expanded row there, gate the button on the same signal — e.g. thread the table's canOpenSidePanel into ExpandedLogRow — instead of the always-truthy {openSidebar && …} guard on line 58.

Correct, and fixed in 85271bb. {openSidebar && ...} guarded on the return value of a useCallback, so it was always truthy — ExpandedLogRow now takes canOpenSidePanel and DBRowTable passes the same flag that gates the row hover button, so the two agree.

Worth noting this was reachable before this PR too (the chevron already expanded rows on that table), just much harder to stumble into. Extended the no-side-panel unit test to assert the button is absent after expanding, not only before.

🔵 minor — The new preference control is untested and its data-testid has no consumer

The seven new unit tests seed hdx-user-preferences in localStorage directly, so nothing exercises the modal half of the feature: a regression here (wrong key, wrong option value, onChange dropped) would leave every test green. data-testid="row-click-action-select" is referenced nowhere in the repo — it looks like a hook for a test that wasn't written. Either add a test that drives this Select and asserts the stored preference, or drop the orphan test id.

Right on both counts — the testid was left over from an E2E spec I ended up deleting. Rather than drop it, 85271bb adds src/__tests__/UserPreferencesModal.test.tsx, which drives the Select and covers the wiring you flagged:

• shows Expand inline when nothing is stored (the DEFAULT_ROW_CLICK_ACTION fallthrough for existing users)
• reflects a stored sidePanel (the value binding)
• picking Open side panel writes rowClickAction: 'sidePanel' under hdx-user-preferences (the storage key, option values, and onChange)

A wrong key or option value now fails rather than passing green, and the testid has a consumer.

🟠 major — The side panel's only remaining direct entry point is unreachable by keyboard and hidden from screen readers

DBRowTableIconButton renders a div with role="button" and tabIndex={-1} (DBRowTableIconButton.tsx:56), and .rowButtons is opacity: 0 until hover (packages/app/styles/LogTable.module.scss:164). It is also nested inside the row-body <button> (DBRowTable.tsx:1155), whose descendants are presentational per ARIA, so the new aria-label is never exposed. Before this PR the row body itself — a real focusable button — opened the panel; now keyboard/AT users can only reach it via expand-then-maximize. Render the side-panel control outside the row <button> (e.g. alongside the chevron in the expand cell, or as its own <td>) as a focusable element, rather than as another tabIndex={-1} hover affordance.

🔵 minor — A preference labelled "Search results" silently changes row-click behavior for every row table in the app

prefersInlineExpand is computed in RawLogTable, so the new default applies to every DBSqlRowTableWithSideBar consumer — dashboard Search tiles (DBDashboardPage.tsx:1390), ServiceDashboardSlowestEventsTile.tsx:92, PodDetailsSidePanel.tsx:203, NodeDetailsSidePanel.tsx:194, NamespaceDetailsSidePanel.tsx:181, llm/dashboard/ErrorsTab.tsx:113 and ChartPreviewPanel.tsx:315 — several of which are fixed-height tiles where .expandedRowContent's 400px panel (LogTable.module.scss:61) fills the whole tile, and one of which is already inside a side panel. Either scope the default to the search page with an explicit prop on DBSqlRowTableWithSideBar (defaulting the tiles to sidePanel), or drop the "Search results" framing in UserPreferencesModal.tsx:120 so the setting reads as global.

🔵 minor — The stale-rowWhere guard that gates inline expansion has no test

The seven new unit tests all drive RawLogTable with highlightedLineId passed in directly, so nothing covers the wiring this PR actually changed here: that a rowWhere left over from another source (rowSource !== sourceId) no longer mounts a panel, no longer highlights a row, and no longer suppresses inline expansion. Add a DBSqlRowTableWithSidebar test that mounts with rowWhere set and rowSource pointing at a different source, and asserts the row body still toggles inline expansion.

Default preserves old behavior

Users without a stored rowClickAction, including existing users whose preferences predate this field, fall back to sidePanel. Because DBRowTable enables inline expansion only when this value is expand, row clicks keep opening the side panel unless users manually opt in. This contradicts the PR's stated behavior that row clicks expand inline by default and that existing users receive the new default.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

🔵 minor — Opening the side panel strands already-expanded rows: the body button stops collapsing them and drops aria-expanded

expandInlineOnRowClick is global to the table, but isExpanded is per row. With rowClickAction: 'expand', click row A's body (it expands, ExpandedLogRow renders because line 1256 gates only on showExpandButton && isExpanded), then click row B's "Open in side panel" hover button. highlightedLineId becomes non-null, so for row A the body button now reports aria-expanded={undefined} and is labelled "View details for log entry" while row A is still visibly expanded, and clicking it re-targets the panel instead of collapsing. Gate per row — e.g. keep toggling when isExpanded is true (expandInlineOnRowClick || isExpanded) so an expanded row can always be closed by the control that opened it. (Same state breaks the E2E clickRowBody regex in tests/e2e/components/TableComponent.ts:106.)

🔵 minor — DEFAULT_ROW_CLICK_ACTION is a second source of truth every reader has to remember

storageWithMigration.getItem returns the parsed object as-is (line 182–183) rather than {...DEFAULT_PREFERENCES, ...migrated}, which is why the new field needs ?? DEFAULT_ROW_CLICK_ACTION repeated at every read site (DBRowTable.tsx:458, UserPreferencesModal.tsx:127) — a third reader that forgets it silently gets undefined. Merging DEFAULT_PREFERENCES into the value returned by getItem would let rowClickAction: 'sidePanel' live in DEFAULT_PREFERENCES alone, and would also stop partially-stored preferences from yielding undefined timeFormat/font (which is exactly what the new tests and the new E2E seed do).

🔵 minor — The stale-rowWhere fix — the subtler of the two bug fixes — has no test

Nothing covers that a leftover rowWhere with a rowSource pointing at a different source now yields highlightedLineId={undefined} instead of highlighting a phantom row and suppressing inline expansion. The DBRowTable tests only cover the consumer side (highlightedLineId="some-other-row"). Add a test that renders DBSqlRowTableWithSideBar with sourceId="B" while rowWhere/rowSource are set for source A and asserts the child table receives no highlightedLineId — otherwise a future refactor of that predicate reintroduces the bug green.

🔵 minor — renderTable seeds localStorage but never clears it after the describe

window.localStorage.clear() runs only at the start of each render, so after the last "Row click behavior" test the key hdx-user-preferences is left at {colorMode:'dark',rowClickAction:'expand'} for the rest of the file; the "Error state" describe then mounts RawLogTable on jotai's module-level default store, whose atomWithStorage onMount reads that value (so isUTC/timeFormat/font are undefined there). The comment on line 262 says the fresh Provider prevents leakage, but the leak is via localStorage, not the store. Add an afterEach(() => window.localStorage.clear()) to this describe.

Virtual row selectors break

Moving data-index from each virtualized data row to its enclosing tbody breaks the dashboard and chart-editor E2E helpers, which still wait for tbody tr[data-index]. Those helpers can no longer find the data rows, so tests that read cells or interact with rows will time out. Update those locators alongside this DOM change.

🔵 minor — The new "Open in side panel" control is not keyboard reachable

DBRowTableIconButton defaults to tabIndex = -1 and renders component="div" (DBRowTableIconButton.tsx:23,53-56), and this call site doesn't override it. Under rowClickAction: 'expand' the row body no longer opens the side panel, so for a keyboard-only user the direct route to the panel disappears entirely — the only remaining path is Tab to the row body, Enter to expand inline, then Tab to the expanded row's maximize <button>. Pass tabIndex={0} on this one button (it's a primary action, unlike the sibling copy affordances).

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

🔵 minor — Hover "Open in side panel" button stays visible when a row click already opens the panel

onOpenSidePanel is gated on prefersInlineExpand && canOpenSidePanel, which ignores highlightedLineId. With the expand preference and the panel already open, expandInlineOnRowClick is false (line 475), so a row click opens/moves the panel — and the hover button does the same thing, which is exactly the duplication the default mode avoids and that DBRowTable.test.tsx:342-346 asserts against. Gate it on expandInlineOnRowClick instead, or accept the duplication deliberately and drop that assertion's rationale.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

🔵 minor — The new highlightedLineId derivation has no test; reverting it keeps every test green

This is the fix for the stale rowWhere/rowSource signal, and it is the only thing that returns row clicks to inline expansion after the panel closes — but the unit tests inject highlightedLineId into RawLogTable directly (DBRowTable.test.tsx:465), so changing this line back to highlightedLineId={rowId ?? undefined} still passes. Add a test that renders DBSqlRowTableWithSideBar with rowWhere set and rowSource pointing at a different source, and asserts no row is highlighted.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

🔵 minor — !canOpenSidePanel || in expandInlineOnRowClick is subsumed by prefersInlineExpand

When canOpenSidePanel is false, prefersInlineExpand (line 468) is already true whenever showExpandButton is, so the second !canOpenSidePanel branch can never change the result. Reduce to prefersInlineExpand && highlightedLineId == null and keep the comment about the panel staying the active surface.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

🔵 minor — Preference is scoped as "Search results" but applies to every row table in the app

rowClickAction is read in RawLogTable, which backs dashboard table tiles (DBDashboardPage.tsx:1390), the Kubernetes pod/node/namespace side panels (PodDetailsSidePanel.tsx:203, NodeDetailsSidePanel.tsx:194, NamespaceDetailsSidePanel.tsx:181), the service-dashboard slowest-events tile and the chart preview panel — all of which are short, fixed-height surfaces where a tall inline expansion is a worse fit than the panel. Either retitle the section to something accurate (e.g. "Result rows") or thread an opt-in prop from the search page so the preference only governs the full-height results table.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

🔵 minor — Expanded row's 'Open in side panel' skips onOpenSidebar and drops aliasWith, unlike the new hover button

The expanded row's button still opens the panel through useSidebarOpener, which only writes the rowWhere/rowSource URL params. The new hover button in DBRowTable goes through _onRowExpandClick → onOpenSidebar (DBSqlRowTableWithSidebar.tsx:93), which also sets aliasWith and fires onSidebarOpen. Opening a row from its expanded view therefore leaves aliasWith stale or empty, and with 'Expand inline' on this button is a main route to the panel. Fix: replace the canOpenSidePanel boolean with an onOpenSidePanel callback, pass () => _onRowExpandClick(row.original) as the hover button already does, and delete useSidebarOpener.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

🟠 major — The expanded row's 'Open in side panel' button skips aliasWith and live-tail pause that the hover button applies

With rowClickAction: 'expand', the PR offers two ways to open the side panel, and they behave differently. The hover button calls _onRowExpandClick, which reaches onOpenSidebar in DBSqlRowTableWithSidebar.tsx:93. That sets aliasWith and calls onSidebarOpen, which pauses live tail in DBSearchPage.tsx:1495. The maximize button in the expanded row goes through useSidebarOpener instead and only writes rowWhere/rowSource. So the panel opens with whatever aliasWith was left from the last open, and live tail keeps running. Fix: replace the new canOpenSidePanel?: boolean prop with onOpenSidePanel?: () => void, passed from DBRowTable.tsx:1319 as canOpenSidePanel ? () => _onRowExpandClick(row.original) : undefined, and delete useSidebarOpener. That leaves one code path for opening the panel.

_{Nothing blocks merge automatically, but a maintainer will expect this fixed if it is a real defect in code this PR changes. If it is about surrounding code, reply and say so instead of patching. Do not widen the PR. How to respond}

🔵 minor — The rowClickAction default is repeated at every reader instead of applied once when preferences load

rowClickAction is optional, so each reader has to add ?? DEFAULT_ROW_CLICK_ACTION itself (DBRowTable.tsx:471, UserPreferencesModal.tsx:58). Any new reader that forgets it gets undefined. Fix: add rowClickAction: 'sidePanel' to DEFAULT_PREFERENCES and return { ...DEFAULT_PREFERENCES, ...parsed } from the already-migrated branch of migrateUserPreferences (line 151). Then the field can be required and both fallbacks go away.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

🔵 minor — No test covers the new rule that hides highlightedLineId when the side panel isn't mounted

Inline expansion is disabled whenever highlightedLineId is set. That value now depends on the sidePanel element, which is null when rowSource points to another source. No test pins this, so dropping the ternary would pass CI while a stale rowWhere silently turns off inline expansion. Add a test for DBSqlRowTableWithSideBar that sets rowWhere plus a rowSource for a different source, then checks that a row click under expand expands inline instead of calling the side-panel opener.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}