diff --git a/.changeset/witty-mangos-listen.md b/.changeset/witty-mangos-listen.md new file mode 100644 index 0000000000..c9ef18614f --- /dev/null +++ b/.changeset/witty-mangos-listen.md @@ -0,0 +1,10 @@ +--- +'@hyperdx/app': minor +--- + +Add a Preferences → Search results → Row click setting. Leave it at `Open side +panel` (the default) for the existing behavior, or set it to `Expand inline` to +have a row click expand the row in place — the chevron's 16px hit target is hard +to aim at while scanning logs. With inline expansion on, the side panel moves to +a hover button on the row, stays one click away from an expanded row, and keeps +receiving row clicks while it is open. diff --git a/packages/app/src/UserPreferencesModal.tsx b/packages/app/src/UserPreferencesModal.tsx index 578e784bbb..1bb8e79674 100644 --- a/packages/app/src/UserPreferencesModal.tsx +++ b/packages/app/src/UserPreferencesModal.tsx @@ -16,7 +16,12 @@ import { IconFlask } from '@tabler/icons-react'; import { OPTIONS_FONTS } from './config/fonts'; import { useAppTheme } from './theme/ThemeProvider'; import { isValidThemeName, themes } from './theme'; -import { UserPreferences, useUserPreferences } from './useUserPreferences'; +import { + DEFAULT_ROW_CLICK_ACTION, + RowClickAction, + UserPreferences, + useUserPreferences, +} from './useUserPreferences'; const OPTIONS_COLOR_MODE = [ { label: 'System', value: 'system' }, @@ -24,6 +29,11 @@ const OPTIONS_COLOR_MODE = [ { label: 'Dark', value: 'dark' }, ]; +const OPTIONS_ROW_CLICK_ACTION: { label: string; value: RowClickAction }[] = [ + { label: 'Open side panel', value: 'sidePanel' }, + { label: 'Expand inline', value: 'expand' }, +]; + // Brand theme options (generated from theme registry) const OPTIONS_BRAND_THEMES = Object.values(themes).map(t => ({ label: t.displayName, @@ -108,6 +118,27 @@ export const UserPreferencesModal = ({ /> + + + { + const option = OPTIONS_ROW_CLICK_ACTION.find( + o => o.value === value, + ); + if (option) { + setUserPreference({ rowClickAction: option.value }); + } + }} + data={OPTIONS_ROW_CLICK_ACTION} + allowDeselect={false} + /> + + diff --git a/packages/app/src/__tests__/UserPreferencesModal.test.tsx b/packages/app/src/__tests__/UserPreferencesModal.test.tsx new file mode 100644 index 0000000000..72868fb7e4 --- /dev/null +++ b/packages/app/src/__tests__/UserPreferencesModal.test.tsx @@ -0,0 +1,64 @@ +import { Provider } from 'jotai'; +import { screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import { UserPreferencesModal } from '@/UserPreferencesModal'; +import type { UserPreferences } from '@/useUserPreferences'; + +const STORAGE_KEY = 'hdx-user-preferences'; + +const renderModal = (stored?: Partial) => { + window.localStorage.clear(); + if (stored) { + window.localStorage.setItem( + STORAGE_KEY, + // `colorMode` is what marks stored preferences as already migrated. + JSON.stringify({ colorMode: 'dark', ...stored }), + ); + } + return renderWithMantine( + + {}} /> + , + ); +}; + +const readStoredPreferences = (): UserPreferences => + JSON.parse(window.localStorage.getItem(STORAGE_KEY) ?? '{}'); + +describe('UserPreferencesModal', () => { + describe('Row click', () => { + it('shows the default action for users with no stored preference', () => { + renderModal(); + + expect(screen.getByTestId('row-click-action-select')).toHaveValue( + 'Open side panel', + ); + }); + + it('reflects a stored preference', () => { + renderModal({ rowClickAction: 'expand' }); + + expect(screen.getByTestId('row-click-action-select')).toHaveValue( + 'Expand inline', + ); + }); + + it('stores the selected action', async () => { + renderModal(); + + await userEvent.click(screen.getByTestId('row-click-action-select')); + await userEvent.click( + await screen.findByRole('option', { + name: 'Expand inline', + hidden: true, + }), + ); + + expect(screen.getByTestId('row-click-action-select')).toHaveValue( + 'Expand inline', + ); + expect(readStoredPreferences().rowClickAction).toBe('expand'); + }); + }); +}); diff --git a/packages/app/src/components/DBRowTable.tsx b/packages/app/src/components/DBRowTable.tsx index 292ac9da58..e25d11bc5e 100644 --- a/packages/app/src/components/DBRowTable.tsx +++ b/packages/app/src/components/DBRowTable.tsx @@ -96,7 +96,10 @@ import { UNDEFINED_WIDTH, } from '@/tableUtils'; import { FormatTime } from '@/useFormatTime'; -import { useUserPreferences } from '@/useUserPreferences'; +import { + DEFAULT_ROW_CLICK_ACTION, + useUserPreferences, +} from '@/useUserPreferences'; import { getChartColorInfo, getLogLevelClass, @@ -385,7 +388,7 @@ export const RawLogTable = memo( rows: Record[]; isLoading?: boolean; fetchNextPage?: (options?: FetchNextPageOptions | undefined) => any; - onRowDetailsClick: (row: Record) => void; + onRowDetailsClick?: (row: Record) => void; generateRowId: (row: Record) => RowWhereResult; hasNextPage?: boolean; highlightedLineId?: string; @@ -453,9 +456,25 @@ export const RawLogTable = memo( ); const { - userPreferences: { isUTC }, + userPreferences: { isUTC, rowClickAction }, } = useUserPreferences(); + // The ClickHouse dashboard's slow-query list has no side panel to open, so + // inline expansion is the only thing a row click can do there. + const canOpenSidePanel = onRowDetailsClick != null; + + // Inline expansion is owned by the expand-button column, so tables that hide + // it (surrounding context, patterns) always fall back to the side panel. + const prefersInlineExpand = + showExpandButton && + (!canOpenSidePanel || + (rowClickAction ?? DEFAULT_ROW_CLICK_ACTION) === 'expand'); + + // Once the side panel is open it stays the active surface, so clicks keep + // moving it from row to row instead of expanding rows behind it. + const expandInlineOnRowClick = + prefersInlineExpand && (!canOpenSidePanel || highlightedLineId == null); + const [columnSizeStorage, setColumnSizeStorage] = useLocalStorage< Record >(`${tableId}-column-sizes`, {}); @@ -1195,9 +1214,23 @@ export const RawLogTable = memo( [styles.isTruncated]: !wrapLinesEnabled, })} onClick={() => { - _onRowExpandClick(row.original); + if (expandInlineOnRowClick) { + toggleRowExpansion(rowId); + } else { + _onRowExpandClick(row.original); + } }} - aria-label="View details for log entry" + aria-expanded={ + expandInlineOnRowClick ? isExpanded : undefined + } + // Deliberately distinct from the chevron's "Expand log + // details" so the two hit targets stay addressable + // apart in tests and assistive tech. + aria-label={ + expandInlineOnRowClick + ? `${isExpanded ? 'Collapse' : 'Expand'} log row` + : 'View details for log entry' + } > {row .getVisibleCells() @@ -1265,6 +1298,11 @@ export const RawLogTable = memo( onToggleWrap={() => setWrapLinesEnabled(!wrapLinesEnabled) } + onOpenSidePanel={ + prefersInlineExpand && canOpenSidePanel + ? () => _onRowExpandClick(row.original) + : undefined + } /> )} @@ -1278,6 +1316,7 @@ export const RawLogTable = memo( virtualKey={virtualRow.key.toString()} source={source} rowId={rowId} + canOpenSidePanel={canOpenSidePanel} > {renderRowDetails?.({ id: rowId, @@ -1909,7 +1948,9 @@ function DBSqlRowTableComponent({ fetchNextPage={fetchNextPage} // onPropertySearchClick={onPropertySearchClick} hasNextPage={hasNextPage} - onRowDetailsClick={_onRowDetailsClick} + // Passed through conditionally so tables with no side panel (the + // ClickHouse dashboard's slow-query list) don't offer to open one. + onRowDetailsClick={onRowDetailsClick ? _onRowDetailsClick : undefined} onScroll={onScroll} generateRowId={getRowWhere} isError={isError} diff --git a/packages/app/src/components/DBSqlRowTableWithSidebar.tsx b/packages/app/src/components/DBSqlRowTableWithSidebar.tsx index 342e87e2ff..c683786711 100644 --- a/packages/app/src/components/DBSqlRowTableWithSidebar.tsx +++ b/packages/app/src/components/DBSqlRowTableWithSidebar.tsx @@ -120,23 +120,30 @@ export default function DBSqlRowTableWithSideBar({ [sourceData], ); + // `rowWhere`/`rowSource` survive a source switch, so a stale `rowWhere` can + // outlive the panel it belongs to. Deriving the table's highlight from this + // element keeps it from highlighting a row — and from treating the panel as + // open, which suppresses inline expansion — while no panel is mounted. + const sidePanel = + sourceData && (rowSource === sourceId || !rowSource) ? ( + + ) : null; + return ( - {sourceData && (rowSource === sourceId || !rowSource) && ( - - )} + {sidePanel} = ({ className={baseClasses} tabIndex={tabIndex} role="button" + aria-label={title} > {isActive ? : children} diff --git a/packages/app/src/components/DBTable/DBRowTableRowButtons.tsx b/packages/app/src/components/DBTable/DBRowTableRowButtons.tsx index 8c93830b53..37a3849337 100644 --- a/packages/app/src/components/DBTable/DBRowTableRowButtons.tsx +++ b/packages/app/src/components/DBTable/DBRowTableRowButtons.tsx @@ -1,6 +1,11 @@ import React, { useState } from 'react'; import { notifications } from '@mantine/notifications'; -import { IconCopy, IconLink, IconTextWrap } from '@tabler/icons-react'; +import { + IconCopy, + IconLayoutSidebarRightExpand, + IconLink, + IconTextWrap, +} from '@tabler/icons-react'; import { RowWhereResult } from '@/hooks/useRowWhere'; import { @@ -19,6 +24,8 @@ interface DBRowTableRowButtonsProps { sourceId?: string; isWrapped: boolean; onToggleWrap: () => void; + /** Omitted when clicking the row already opens the side panel. */ + onOpenSidePanel?: () => void; } const DBRowTableRowButtons: React.FC = ({ @@ -27,6 +34,7 @@ const DBRowTableRowButtons: React.FC = ({ sourceId, isWrapped, onToggleWrap, + onOpenSidePanel, }) => { const [isCopied, setIsCopied] = useState(false); const [isUrlCopied, setIsUrlCopied] = useState(false); @@ -105,6 +113,15 @@ const DBRowTableRowButtons: React.FC = ({ > + {onOpenSidePanel && ( + + + + )} ); }; diff --git a/packages/app/src/components/DBTable/RowSelectionCells.tsx b/packages/app/src/components/DBTable/RowSelectionCells.tsx index 322ae2dd69..3995623dd6 100644 --- a/packages/app/src/components/DBTable/RowSelectionCells.tsx +++ b/packages/app/src/components/DBTable/RowSelectionCells.tsx @@ -40,7 +40,8 @@ export const RowSelectionCell = memo( aria-label="Select row" title="Select row, shift-click to select a range" data-testid="row-select-checkbox" - // The row body is a single button that opens the side panel. + // The row body is a single button that opens the side panel or expands + // the row, depending on the `rowClickAction` preference. onClick={e => e.stopPropagation()} onChange={e => onToggle(rowId, { diff --git a/packages/app/src/components/ExpandableRowTable.tsx b/packages/app/src/components/ExpandableRowTable.tsx index ab1251cd02..51f979035c 100644 --- a/packages/app/src/components/ExpandableRowTable.tsx +++ b/packages/app/src/components/ExpandableRowTable.tsx @@ -30,12 +30,15 @@ export const ExpandedLogRow = memo( virtualKey, source, rowId, + canOpenSidePanel, }: { children: React.ReactNode; columnsLength: number; virtualKey: string; source?: TSource; rowId: string; + /** False on tables that render no side panel, so the URL params go nowhere. */ + canOpenSidePanel?: boolean; }) => { const openSidebar = useSidebarOpener(); @@ -51,7 +54,7 @@ export const ExpandedLogRow = memo( - {openSidebar && ( + {canOpenSidePanel && ( openSidebar(rowId, source?.id)} - title="Open in sidebar" - aria-label="Open in sidebar" + title="Open in side panel" + aria-label="Open in side panel" style={{ zIndex: 1, fontSize: '12px', diff --git a/packages/app/src/components/__tests__/DBRowTable.test.tsx b/packages/app/src/components/__tests__/DBRowTable.test.tsx index 048e22c5c6..408fc50b2f 100644 --- a/packages/app/src/components/__tests__/DBRowTable.test.tsx +++ b/packages/app/src/components/__tests__/DBRowTable.test.tsx @@ -1,4 +1,6 @@ -import { screen, waitFor } from '@testing-library/react'; +import { Provider } from 'jotai'; +import type { ReactElement } from 'react'; +import { screen, waitFor, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { @@ -7,6 +9,20 @@ import { } from '@/components/DBRowTable'; import * as useChartConfigModule from '@/hooks/useChartConfig'; import { RowWhereResult } from '@/hooks/useRowWhere'; +import { RowClickAction, UserPreferences } from '@/useUserPreferences'; + +const mockSetQueryState = jest.fn(); + +// The inline expanded row reads the row query params through nuqs, which needs +// a mounted Next router. Routing every setter through one mock lets tests assert +// which param the expanded row's side panel button writes. +jest.mock('nuqs', () => ({ + ...jest.requireActual('nuqs'), + useQueryState: (key: string) => [ + null, + (value: unknown) => mockSetQueryState(key, value), + ], +})); const mockRowWhereResult: RowWhereResult = { where: '', aliasWith: [] }; @@ -238,6 +254,233 @@ describe('RawLogTable', () => { }); }); + describe('Row click behavior', () => { + const ROW_ID = 'Timestamp = 1'; + const VIEWPORT_HEIGHT = 900; + + // The table is virtualized and jsdom performs no layout, so every element + // measures 0px and the virtualizer renders no rows at all. Fake just enough + // geometry for one screenful. Same approach as the MetricExplorer tests. + let rectSpy: jest.SpyInstance; + let clientHeightSpy: jest.SpyInstance; + + beforeAll(() => { + rectSpy = jest + .spyOn(HTMLElement.prototype, 'getBoundingClientRect') + .mockImplementation(function (this: HTMLElement) { + const height = + this.dataset.testid === 'search-results-table' + ? VIEWPORT_HEIGHT + : 23; + return { + width: 580, + height, + top: 0, + left: 0, + right: 580, + bottom: height, + x: 0, + y: 0, + toJSON: () => ({}), + }; + }); + clientHeightSpy = jest + .spyOn(HTMLElement.prototype, 'clientHeight', 'get') + .mockReturnValue(VIEWPORT_HEIGHT); + }); + + afterAll(() => { + rectSpy.mockRestore(); + clientHeightSpy.mockRestore(); + }); + + const baseProps = { + displayedColumns: ['col1'], + rows: [{ col1: 'value1' }], + isLoading: false, + dedupRows: false, + hasNextPage: false, + generateRowId: () => ({ where: ROW_ID, aliasWith: [] }), + getRowWhere: () => ({ where: ROW_ID, aliasWith: [] }), + columnTypeMap: new Map(), + renderRowDetails: () => row details, + }; + + // A fresh jotai store per render keeps the stored preference from leaking + // into the next test through the module-level preferences atom. + const renderTable = (ui: ReactElement, rowClickAction?: RowClickAction) => { + window.localStorage.clear(); + if (rowClickAction) { + window.localStorage.setItem( + 'hdx-user-preferences', + JSON.stringify({ + colorMode: 'dark', + rowClickAction, + } satisfies Partial), + ); + } + return renderWithMantine({ui}); + }; + + it('opens the side panel on row click by default', async () => { + const onRowDetailsClick = jest.fn(); + + renderTable( + , + ); + + await userEvent.click( + await screen.findByRole('button', { + name: 'View details for log entry', + }), + ); + + expect(onRowDetailsClick).toHaveBeenCalledTimes(1); + expect( + screen.queryByTestId(`expanded-row-${ROW_ID}`), + ).not.toBeInTheDocument(); + // A row click already opens the panel, so the hover button would be a + // second way to do the same thing. + expect( + screen.queryByRole('button', { name: 'Open in side panel' }), + ).not.toBeInTheDocument(); + }); + + it('expands the row inline when its body is clicked', async () => { + const onRowDetailsClick = jest.fn(); + + renderTable( + , + 'expand', + ); + + await userEvent.click( + await screen.findByRole('button', { name: 'Expand log row' }), + ); + + const expandedRow = await screen.findByTestId(`expanded-row-${ROW_ID}`); + expect(expandedRow).toBeVisible(); + expect(onRowDetailsClick).not.toHaveBeenCalled(); + + // The expanded row keeps the side panel one click away, via URL params + // rather than `onRowDetailsClick`. + await userEvent.click( + within(expandedRow).getByRole('button', { + name: 'Open in side panel', + }), + ); + + expect(mockSetQueryState).toHaveBeenCalledWith('rowWhere', ROW_ID); + }); + + it('collapses an expanded row when its body is clicked again', async () => { + renderTable( + {}} />, + 'expand', + ); + + await userEvent.click( + await screen.findByRole('button', { name: 'Expand log row' }), + ); + await userEvent.click( + await screen.findByRole('button', { name: 'Collapse log row' }), + ); + + expect( + screen.queryByTestId(`expanded-row-${ROW_ID}`), + ).not.toBeInTheDocument(); + }); + + it('opens the side panel from the row hover button', async () => { + const onRowDetailsClick = jest.fn(); + + renderTable( + , + 'expand', + ); + + await userEvent.click( + await screen.findByRole('button', { name: 'Open in side panel' }), + ); + + expect(onRowDetailsClick).toHaveBeenCalledTimes(1); + expect( + screen.queryByTestId(`expanded-row-${ROW_ID}`), + ).not.toBeInTheDocument(); + }); + + // The expanded row is rendered by the expand-button column, so tables that + // hide it have nowhere to put inline details. + it('opens the side panel on row click when the expand button is hidden', async () => { + const onRowDetailsClick = jest.fn(); + + renderTable( + , + 'expand', + ); + + await userEvent.click( + await screen.findByRole('button', { + name: 'View details for log entry', + }), + ); + + expect(onRowDetailsClick).toHaveBeenCalledTimes(1); + }); + + // The ClickHouse dashboard's slow-query list renders inline details with no + // side panel behind them. + it('expands inline regardless of the preference when there is no side panel', async () => { + renderTable(, 'sidePanel'); + + const rowBody = await screen.findByRole('button', { + name: 'Expand log row', + }); + expect( + screen.queryByRole('button', { name: 'Open in side panel' }), + ).not.toBeInTheDocument(); + + await userEvent.click(rowBody); + + expect(await screen.findByTestId(`expanded-row-${ROW_ID}`)).toBeVisible(); + // The expanded row's maximize button would only write URL params nothing + // reads, so it stays hidden here too. + expect( + screen.queryByRole('button', { name: 'Open in side panel' }), + ).not.toBeInTheDocument(); + }); + + // With the panel open it stays the active surface, so a row click moves it + // rather than expanding rows behind it. + it('moves the open side panel on row click instead of expanding', async () => { + const onRowDetailsClick = jest.fn(); + + renderTable( + , + 'expand', + ); + + await userEvent.click( + await screen.findByRole('button', { + name: 'View details for log entry', + }), + ); + + expect(onRowDetailsClick).toHaveBeenCalledTimes(1); + expect( + screen.queryByTestId(`expanded-row-${ROW_ID}`), + ).not.toBeInTheDocument(); + }); + }); + describe('Error state', () => { const paginationProps = { displayedColumns: ['col1'], diff --git a/packages/app/src/useUserPreferences.tsx b/packages/app/src/useUserPreferences.tsx index 8d82240263..74ef219fbc 100644 --- a/packages/app/src/useUserPreferences.tsx +++ b/packages/app/src/useUserPreferences.tsx @@ -5,6 +5,11 @@ import { atomWithStorage } from 'jotai/utils'; type ColorModePreference = 'light' | 'dark' | 'system'; +/** What clicking the body of a search result row does. */ +export type RowClickAction = 'expand' | 'sidePanel'; + +export const DEFAULT_ROW_CLICK_ACTION: RowClickAction = 'sidePanel'; + export type UserPreferences = { isUTC: boolean; timeFormat: '12h' | '24h'; @@ -12,6 +17,12 @@ export type UserPreferences = { colorMode: ColorModePreference; font: 'IBM Plex Mono' | 'Roboto Mono' | 'Inter' | 'Roboto'; expandSidebarHeader?: boolean; + /** + * Unset until the user opts in. Stored preferences that predate this field are + * returned as-is rather than merged with `DEFAULT_PREFERENCES`, so readers must + * fall back to `DEFAULT_ROW_CLICK_ACTION`. + */ + rowClickAction?: RowClickAction; }; // Legacy type for migration diff --git a/packages/app/tests/e2e/components/TableComponent.ts b/packages/app/tests/e2e/components/TableComponent.ts index f854e4516e..2259bcfc2b 100644 --- a/packages/app/tests/e2e/components/TableComponent.ts +++ b/packages/app/tests/e2e/components/TableComponent.ts @@ -54,7 +54,7 @@ export class TableComponent { } /** - * Click on a specific row + * Click on a specific row, which opens it in the side panel. */ async clickRow(index: number) { await this.getRow(index).click(); @@ -67,6 +67,24 @@ export class TableComponent { await this.firstRow.click(); } + /** + * Open a row in the side panel via its hover button. The button only exists + * once the `expand` row-click preference is set — without it a plain + * {@link clickRow} already opens the panel. + */ + async openRowSidePanel(index: number) { + const row = this.getRow(index); + await row.hover(); + await row.getByRole('button', { name: 'Open in side panel' }).click(); + } + + /** + * Open the first row in the side panel via its hover button. + */ + async openFirstRowSidePanel() { + await this.openRowSidePanel(0); + } + /** * Expand a row in place via its chevron button, revealing the inline expanded * row underneath it. @@ -87,6 +105,25 @@ export class TableComponent { .click(); } + /** + * Click a row's body — the large hit target covering everything but the + * chevron. Requires the `expand` row-click preference, under which it toggles + * inline expansion, so calling it twice collapses the row. + */ + async clickRowBody(index: number) { + await this.getRow(index) + .getByRole('button', { name: /^(Expand|Collapse) log row$/ }) + .click(); + } + + /** + * Expand a row in place by clicking its body rather than the chevron. + */ + async expandRowByBodyClick(index: number) { + await this.clickRowBody(index); + await this.firstExpandedRow.waitFor({ state: 'visible', timeout: 10_000 }); + } + /** * Scroll the virtualized container to an absolute offset. */ diff --git a/packages/app/tests/e2e/features/search/search.spec.ts b/packages/app/tests/e2e/features/search/search.spec.ts index 0342d108e2..4d957f9aaa 100644 --- a/packages/app/tests/e2e/features/search/search.spec.ts +++ b/packages/app/tests/e2e/features/search/search.spec.ts @@ -88,6 +88,47 @@ test.describe('Search', { tag: '@search' }, () => { }); }); + // A row click opens the side panel by default; inline expansion is opt-in. + test.describe('Row click set to expand inline', () => { + test.beforeEach(async ({ page }) => { + await page.addInitScript(() => { + window.localStorage.setItem( + 'hdx-user-preferences', + // `colorMode` is what marks stored preferences as already migrated. + JSON.stringify({ colorMode: 'dark', rowClickAction: 'expand' }), + ); + }); + searchPage = new SearchPage(page); + await searchPage.goto(); + }); + + test('should expand a result row inline when its body is clicked', async () => { + await test.step('Perform search', async () => { + await searchPage.submitEmptySearch(); + await expect(searchPage.table.firstRow).toBeVisible(); + }); + + await test.step('Clicking the row body expands it in place', async () => { + await searchPage.table.expandRowByBodyClick(0); + + await expect(searchPage.table.firstExpandedRow).toBeVisible(); + await expect(searchPage.sidePanel.container).toBeHidden(); + }); + + await test.step('Clicking it again collapses the row', async () => { + await searchPage.table.clickRowBody(0); + + await expect(searchPage.table.expandedRows).toHaveCount(0); + }); + + await test.step('The row hover button opens the side panel', async () => { + await searchPage.table.openFirstRowSidePanel(); + + await expect(searchPage.sidePanel.container).toBeVisible(); + }); + }); + }); + test.describe('Advanced Workflows', () => { test.beforeEach(async ({ page }) => { searchPage = new SearchPage(page);