diff --git a/.changeset/onclick-hover-hint.md b/.changeset/onclick-hover-hint.md new file mode 100644 index 0000000000..c529280ac2 --- /dev/null +++ b/.changeset/onclick-hover-hint.md @@ -0,0 +1,6 @@ +--- +"@hyperdx/common-utils": patch +"@hyperdx/app": patch +--- + +Dashboard table tiles configured with a row-click action now show a hover hint describing where the click will go (for example, `Search HyperDX Logs` or `Open dashboard "API Latency Drilldown"`). The cell wrapper is now a real link, so cmd-click and middle-click open the destination in a new tab, right-click shows the browser context menu with "Open in New Tab" and "Copy Link Address", and the destination URL appears in the browser status bar on hover. Keyboard users can Tab to a cell and press Enter to navigate, with a visible focus ring. diff --git a/packages/app/src/HDXMultiSeriesTableChart.tsx b/packages/app/src/HDXMultiSeriesTableChart.tsx index 6d449c6c58..78641fe7e9 100644 --- a/packages/app/src/HDXMultiSeriesTableChart.tsx +++ b/packages/app/src/HDXMultiSeriesTableChart.tsx @@ -1,6 +1,7 @@ import { useCallback, useMemo, useRef, useState } from 'react'; +import Link from 'next/link'; import cx from 'classnames'; -import { UnstyledButton } from '@mantine/core'; +import { Tooltip, UnstyledButton } from '@mantine/core'; import { IconDownload, IconTextWrap } from '@tabler/icons-react'; import { flexRender, @@ -20,11 +21,14 @@ import { useVirtualizer } from '@tanstack/react-virtual'; import { CsvExportButton } from './components/CsvExportButton'; import TableHeader from './components/DBTable/TableHeader'; import { useCsvExport } from './hooks/useCsvExport'; +import type { RowAction } from './hooks/useOnClickLinkBuilder'; import { useBrandDisplayName } from './theme/ThemeProvider'; import { UNDEFINED_WIDTH } from './tableUtils'; import type { NumberFormat } from './types'; import { formatNumber } from './utils'; +import focusStyles from '../styles/focus.module.scss'; + export type TableVariant = 'default' | 'muted'; // Arbitrary limit to prevent OOM crashes for very large result sets. Most result sets should be paginated anyway. @@ -34,7 +38,8 @@ export const Table = ({ data, groupColumnName, columns, - onRowClick, + getRowAction, + getRowSearchLink, tableBottom, enableClientSideSorting = false, sorting, @@ -53,7 +58,18 @@ export const Table = ({ sortingFn?: SortingFnOption; }[]; groupColumnName?: string; - onRowClick?: (row: any, e?: React.MouseEvent) => void; + // Returns the row click destination + a hover-hint description. When + // set, the cell becomes an wrapped in a HoverCard. The resolved + // URL goes straight in the href so the browser handles cmd-click, + // middle-click, right-click, status bar preview, and keyboard + // activation natively. Rows whose templates fail (`url: null`) fall + // back to a click handler that fires a notification, preserving the + // pre-existing #2140 / #2141 / #2146 / #2148 behavior. + getRowAction?: (row: any) => RowAction; + // Legacy single-tile drilldown: bare URL, no hint, no HoverCard. + // Used outside the dashboard onClick path (event side panel, services + // dashboard, etc.). Only consulted when getRowAction is not provided. + getRowSearchLink?: (row: any) => string | null; tableBottom?: React.ReactNode; enableClientSideSorting?: boolean; sorting: SortingState; @@ -146,40 +162,76 @@ export const Table = ({ 'text-truncate': !wrapLinesEnabled, }); - if (onRowClick) { + // Native covers cmd-click (new tab), middle-click + // (new tab), right-click ("Open in New Tab" / "Copy Link + // Address"), Enter key activation, and the browser status + // bar URL preview. No manual handlers required. + const interactiveClassName = cx( + className, + 'd-block text-reset text-decoration-none w-100 text-start', + focusStyles.focusRing, + ); + + if (getRowAction) { + // The hook memoizes per-row results internally, so calling + // this once per cell is cheap and the row-level HoverCard + // in the loop below sees the same identity. + const action = getRowAction(row.original); + if (action.url) { + // Use prefetch={false} so virtualization scroll doesn't + // trigger an N-row prefetch storm against /search? and + // /dashboards/ routes the user usually never opens. + return ( + + {formattedValue} + + ); + } + // Row's templates failed to resolve. Use a real ); } + if (getRowSearchLink) { + const url = getRowSearchLink(row.original); + if (url) { + return ( + + {formattedValue} + + ); + } + } + return
{formattedValue}
; }, size: @@ -327,7 +379,11 @@ export const Table = ({ )} {items.map(virtualRow => { const row = rows[virtualRow.index] as TableRow; - return ( + // Compute the action once per row so the row-level HoverCard + // sees the same description and per-cell renders share the + // memoized result from useOnClickLinkBuilder. + const rowAction = getRowAction ? getRowAction(row.original) : null; + const tr = ( ); + // Row-level Tooltip.Floating so the hint follows the cursor + // and anchors near the cell the user is over, not at the row's + // center-top. Tooltip.Floating tracks the cursor via floating-ui + // and stays within the row's bounding box; one tooltip per row + // means no flicker as the cursor moves between cells. + // + // The hint is suppressed when rowAction.url === null because + // the click only fires an error toast on those rows, so showing + // "Open in search" would mislead the user. + if (rowAction && rowAction.url) { + return ( + + {tr} + + ); + } + return tr; })} {paddingBottom > 0 && ( diff --git a/packages/app/src/__tests__/HDXMultiSeriesTableChart.test.tsx b/packages/app/src/__tests__/HDXMultiSeriesTableChart.test.tsx new file mode 100644 index 0000000000..c63e72941b --- /dev/null +++ b/packages/app/src/__tests__/HDXMultiSeriesTableChart.test.tsx @@ -0,0 +1,207 @@ +import React from 'react'; +import { fireEvent, screen, waitFor } from '@testing-library/react'; + +import { Table } from '@/HDXMultiSeriesTableChart'; + +// Next.js Link needs a router context; the router isn't exercised +// because we never trigger client-side navigation in these tests. +jest.mock('next/router', () => ({ + useRouter: () => ({ + push: jest.fn(), + prefetch: jest.fn(), + pathname: '/', + query: {}, + }), +})); + +// JSDOM gives the table container 0×0 dimensions, so the real +// virtualizer renders no rows. Stub it to render every row as if +// visible. The hint / button / link assertions don't depend on +// virtualization correctness. +jest.mock('@tanstack/react-virtual', () => ({ + useVirtualizer: ({ count }: { count: number }) => ({ + getVirtualItems: () => + Array.from({ length: count }, (_, index) => ({ + index, + key: index, + start: index * 58, + end: (index + 1) * 58, + size: 58, + })), + getTotalSize: () => count * 58, + measureElement: () => {}, + options: { scrollMargin: 0 }, + }), +})); + +const baseColumns = [ + { + id: 'service', + dataKey: 'ServiceName', + displayName: 'Service', + }, + { + id: 'count', + dataKey: 'Count', + displayName: 'Count', + }, +]; + +const baseData = [{ ServiceName: 'web', Count: 10 }]; + +describe('HDXMultiSeriesTableChart ', () => { + describe('getRowAction success path', () => { + it('renders the row click cell as a real with the resolved URL', () => { + const getRowAction = jest.fn().mockReturnValue({ + url: '/search?source=src_1&where=', + description: 'Search HyperDX Logs', + }); + + renderWithMantine( +
{}} + />, + ); + + const links = screen.getAllByTestId('dashboard-table-row-action'); + expect(links.length).toBeGreaterThan(0); + links.forEach(link => { + expect(link.tagName).toBe('A'); + expect(link.getAttribute('data-shape')).toBe('link'); + expect(link.getAttribute('href')).toContain('/search?source=src_1'); + }); + }); + + it('reveals the hint at the cursor when hovering a success row', async () => { + const getRowAction = jest.fn().mockReturnValue({ + url: '/search?source=src_1&where=', + description: 'Search HyperDX Logs', + }); + + renderWithMantine( +
{}} + />, + ); + + const row = screen.getByText('web').closest('tr')!; + fireEvent.mouseEnter(row); + + await waitFor( + () => { + expect(screen.getByText('Search HyperDX Logs')).toBeInTheDocument(); + }, + { timeout: 1000 }, + ); + }); + }); + + describe('getRowAction failure path', () => { + it('renders the cell as a
{}} + />, + ); + + const buttons = screen.getAllByTestId('dashboard-table-row-action'); + expect(buttons.length).toBeGreaterThan(0); + buttons.forEach(btn => { + expect(btn.tagName).toBe('BUTTON'); + expect(btn.getAttribute('data-shape')).toBe('button'); + // No href anywhere, so cmd-click / middle-click / right-click + // "Open in New Tab" can't silently open the page. + expect(btn.hasAttribute('href')).toBe(false); + }); + + fireEvent.click(buttons[0]); + expect(onClickError).toHaveBeenCalledTimes(1); + }); + + it('does not reveal a hint when the row action has no url', async () => { + const getRowAction = jest.fn().mockReturnValue({ + url: null, + description: 'Search HyperDX Logs', + onClickError: jest.fn(), + }); + + renderWithMantine( +
{}} + />, + ); + + const row = screen.getByText('web').closest('tr')!; + fireEvent.mouseEnter(row); + + // Give the tooltip a chance to mount; assert it never does. + await new Promise(resolve => setTimeout(resolve, 250)); + expect(screen.queryByText('Search HyperDX Logs')).not.toBeInTheDocument(); + }); + }); + + describe('legacy getRowSearchLink fallback', () => { + it('renders the cell as a Link without a HoverCard when only getRowSearchLink is provided', () => { + const getRowSearchLink = jest + .fn() + .mockReturnValue('/search?source=legacy&where='); + + renderWithMantine( +
{}} + />, + ); + + const links = screen.getAllByTestId('dashboard-table-row-action'); + links.forEach(link => { + expect(link.tagName).toBe('A'); + expect(link.getAttribute('data-shape')).toBe('link'); + expect(link.getAttribute('href')).toContain('/search?source=legacy'); + }); + }); + }); + + describe('no action configured', () => { + it('renders plain cells with no Link, button, or HoverCard', () => { + renderWithMantine( +
{}} + />, + ); + + expect( + screen.queryAllByTestId('dashboard-table-row-action'), + ).toHaveLength(0); + }); + }); +}); diff --git a/packages/app/src/components/DBTableChart.tsx b/packages/app/src/components/DBTableChart.tsx index 3862424c20..708f63e1da 100644 --- a/packages/app/src/components/DBTableChart.tsx +++ b/packages/app/src/components/DBTableChart.tsx @@ -1,5 +1,4 @@ import { useCallback, useMemo, useState } from 'react'; -import { useRouter } from 'next/router'; import { ClickHouseQueryError } from '@hyperdx/common-utils/dist/clickhouse'; import { isRatioChartConfig } from '@hyperdx/common-utils/dist/core/renderChartConfig'; import { @@ -217,33 +216,11 @@ export default function DBTableChart({ queriedConfig, ]); - const getOnClickLink = useOnClickLinkBuilder({ + const getRowAction = useOnClickLinkBuilder({ onClick: config.onClick, dateRange: queriedConfig.dateRange, }); - const router = useRouter(); - const hasOnRowClick = !!getOnClickLink || !!getRowSearchLink; - const onRowClick = useCallback( - (row: Record, e?: React.MouseEvent) => { - const url = getOnClickLink - ? getOnClickLink(row) - : getRowSearchLink - ? getRowSearchLink(row) - : null; - - // getOnClickLink will surface any errors notifications - if (!url) return; - - if (e?.metaKey || e?.ctrlKey || e?.button === 1) { - window.open(url, '_blank'); - } else { - router.push(url); - } - }, - [getOnClickLink, getRowSearchLink, router], - ); - return ( {isLoading && !data ? ( @@ -285,7 +262,8 @@ export default function DBTableChart({
({ + useSources: jest.fn(), +})); + +jest.mock('@/dashboard', () => ({ + useDashboards: jest.fn(), +})); + +jest.mock('@mantine/notifications', () => ({ + notifications: { show: jest.fn() }, +})); + +const dateRange: [Date, Date] = [ + new Date('2026-01-01T00:00:00Z'), + new Date('2026-01-01T01:00:00Z'), +]; + +describe('useOnClickLinkBuilder', () => { + beforeEach(() => { + jest.clearAllMocks(); + jest.mocked(useSources).mockReturnValue({ + data: [ + { + id: 'src_1', + name: 'HyperDX Logs', + kind: 'log', + connection: 'c', + from: { databaseName: 'd', tableName: 't' }, + timestampValueExpression: 'Timestamp', + }, + ], + } as any); + jest.mocked(useDashboards).mockReturnValue({ + data: [{ id: 'dash_1', name: 'API Latency Drilldown', tiles: [] }], + } as any); + }); + + it('returns null when no onClick is configured', () => { + const { result } = renderHook(() => + useOnClickLinkBuilder({ onClick: undefined, dateRange }), + ); + expect(result.current).toBeNull(); + }); + + it('returns a row resolver that includes the resolved source name in the description', () => { + const onClick: OnClickSearch = { + type: 'search', + target: { mode: 'id', id: 'src_1' }, + whereLanguage: 'sql', + }; + const { result } = renderHook(() => + useOnClickLinkBuilder({ onClick, dateRange }), + ); + expect(result.current).not.toBeNull(); + const action = result.current!({}); + expect(action.description).toBe('Search HyperDX Logs'); + const params = new URLSearchParams(action.url!.split('?')[1]); + expect(action.url!.startsWith('/search?')).toBe(true); + expect(params.get('source')).toBe('src_1'); + expect(action.onClickError).toBeUndefined(); + }); + + it('returns a row resolver that includes the resolved dashboard name in the description', () => { + const onClick: OnClickDashboard = { + type: 'dashboard', + target: { mode: 'id', id: 'dash_1' }, + whereLanguage: 'sql', + }; + const { result } = renderHook(() => + useOnClickLinkBuilder({ onClick, dateRange }), + ); + const action = result.current!({}); + expect(action.description).toBe('Open dashboard "API Latency Drilldown"'); + expect(action.url!.startsWith('/dashboards/dash_1?')).toBe(true); + }); + + it('caches results per row reference so repeated calls share the same RowAction', () => { + const onClick: OnClickSearch = { + type: 'search', + target: { mode: 'id', id: 'src_1' }, + whereLanguage: 'sql', + }; + const { result } = renderHook(() => + useOnClickLinkBuilder({ onClick, dateRange }), + ); + const row = { ServiceName: 'web' }; + const first = result.current!(row); + const second = result.current!(row); + expect(first).toBe(second); + }); + + it('encodes a row resolution failure as url: null with a click handler that fires a notification', () => { + const onClick: OnClickSearch = { + type: 'search', + target: { mode: 'template', template: '{{MissingColumn}}' }, + whereLanguage: 'sql', + }; + const { result } = renderHook(() => + useOnClickLinkBuilder({ onClick, dateRange }), + ); + const action = result.current!({ Src: 'Logs' }); + expect(action.url).toBeNull(); + expect(action.description).toBe('Open in search'); + expect(action.onClickError).toBeInstanceOf(Function); + + // Render time is silent: no toast fires while computing the URL. + expect(jest.mocked(notifications.show)).not.toHaveBeenCalled(); + + // Clicking the failing row fires the toast. + const preventDefault = jest.fn(); + action.onClickError!({ preventDefault } as any); + expect(preventDefault).toHaveBeenCalledTimes(1); + expect(jest.mocked(notifications.show)).toHaveBeenCalledWith( + expect.objectContaining({ + color: 'red', + title: 'Link error', + message: "Row has no column 'MissingColumn'", + }), + ); + }); +}); diff --git a/packages/app/src/hooks/useOnClickLinkBuilder.ts b/packages/app/src/hooks/useOnClickLinkBuilder.ts index 6ce705621d..6b4841ffbd 100644 --- a/packages/app/src/hooks/useOnClickLinkBuilder.ts +++ b/packages/app/src/hooks/useOnClickLinkBuilder.ts @@ -1,5 +1,6 @@ import { useMemo } from 'react'; import { + describeOnClick, renderOnClickDashboard, renderOnClickSearch, } from '@hyperdx/common-utils/dist/core/linkUrlBuilder'; @@ -10,10 +11,36 @@ import { useDashboards } from '@/dashboard'; import { useSources } from '@/source'; /** - * Returns a function that, given some row data, produces a URL for the - * configured onClick action. Errors (unresolved names, malformed templates, - * unknown sources) surface as Mantine toast notifications; the function - * returns null in error cases. + * The action a row click should take. `url` is set when the row's + * templates resolved successfully; the cell wrapper renders a real + * `` so the browser handles cmd-click, middle-click, + * right-click, status bar preview, and keyboard activation natively. + * + * When `url` is null the row's templates failed (missing column, stale + * source ID, etc.). The cell still renders as a clickable anchor so the + * hover hint and click-shows-error UX from the original onClick work + * (#2140 / #2141 / #2146 / #2148) keep working. `onClickError` is the + * click handler that fires the notification. + */ +export type RowAction = { + url: string | null; + description: string; + onClickError?: (e: React.MouseEvent) => void; +}; + +/** + * Returns a function that, given some row data, produces a `RowAction` + * describing the configured onClick: the destination URL (when the row + * resolves), a one-line description for the hover hint, and an + * on-click error handler (when the row's templates failed). + * + * Returns null when no `onClick` is configured on the chart, so callers + * can fall back to the legacy `getRowSearchLink` drilldown. + * + * Render-time calls to the returned function are silent: a failing row + * produces `url: null` without raising a Mantine notification. The + * notification fires only when the user clicks the failing row (via + * `onClickError`), mirroring the pre-existing behavior. */ export function useOnClickLinkBuilder({ onClick, @@ -21,49 +48,56 @@ export function useOnClickLinkBuilder({ }: { onClick: OnClick | undefined; dateRange: [Date, Date]; -}): ((row: Record) => string | null) | null { +}): ((row: Record) => RowAction) | null { const { data: sources } = useSources(); const { data: dashboards } = useDashboards(); - const [sourceIdsByName, sourceIds] = useMemo(() => { - const map = new Map(); - const set = new Set(); + const [sourceIdsByName, sourceIds, sourceNamesById] = useMemo(() => { + const idsByName = new Map(); + const ids = new Set(); + const namesById = new Map(); for (const s of sources?.filter(isSearchableSource) ?? []) { - set.add(s.id); + ids.add(s.id); + namesById.set(s.id, s.name); - const existing = map.get(s.name); + const existing = idsByName.get(s.name); if (existing) existing.push(s.id); - else map.set(s.name, [s.id]); + else idsByName.set(s.name, [s.id]); } - return [map, set]; + return [idsByName, ids, namesById]; }, [sources]); - const [dashboardIdsByName, dashboardIds] = useMemo(() => { - const map = new Map(); - const set = new Set(); + const [dashboardIdsByName, dashboardIds, dashboardNamesById] = useMemo(() => { + const idsByName = new Map(); + const ids = new Set(); + const namesById = new Map(); for (const d of dashboards ?? []) { - set.add(d.id); + ids.add(d.id); + namesById.set(d.id, d.name); - const existing = map.get(d.name); + const existing = idsByName.get(d.name); if (existing) existing.push(d.id); - else map.set(d.name, [d.id]); + else idsByName.set(d.name, [d.id]); } - return [map, set]; + return [idsByName, ids, namesById]; }, [dashboards]); return useMemo(() => { if (!onClick) return null; - return (row: Record) => { - const showError = (message: string) => { - notifications.show({ - id: message, - color: 'red', - title: 'Link error', - message, - }); - }; + const description = describeOnClick({ + onClick, + sourceNamesById, + dashboardNamesById, + }); + // Cache results by row reference so the same row producing N cells + // doesn't re-run handlebars rendering + URLSearchParams construction + // N times per render. The WeakMap is scoped to this closure, so a new + // memo (different onClick / sources / dashboards) starts fresh. + const cache = new WeakMap, RowAction>(); + + const compute = (row: Record): RowAction => { const renderResult = onClick.type === 'search' ? renderOnClickSearch({ @@ -81,19 +115,41 @@ export function useOnClickLinkBuilder({ dateRange, }); - if (!renderResult.ok) { - showError(renderResult.error); - return null; + if (renderResult.ok) { + return { url: renderResult.url, description }; } - return renderResult.url; + const errorMessage = renderResult.error; + return { + url: null, + description, + onClickError: (e: React.MouseEvent) => { + e.preventDefault(); + notifications.show({ + id: errorMessage, + color: 'red', + title: 'Link error', + message: errorMessage, + }); + }, + }; + }; + + return (row: Record): RowAction => { + const cached = cache.get(row); + if (cached) return cached; + const result = compute(row); + cache.set(row, result); + return result; }; }, [ onClick, sourceIds, sourceIdsByName, + sourceNamesById, dateRange, dashboardIds, dashboardIdsByName, + dashboardNamesById, ]); } diff --git a/packages/app/styles/focus.module.scss b/packages/app/styles/focus.module.scss index 0f21db49cb..a7501701ca 100644 --- a/packages/app/styles/focus.module.scss +++ b/packages/app/styles/focus.module.scss @@ -3,3 +3,25 @@ outline-offset: 2px; border-radius: var(--mantine-radius-xs); } + +// Used by the failure-row fallback