diff --git a/.changeset/hdx-4405-trailing-action-hint.md b/.changeset/hdx-4405-trailing-action-hint.md new file mode 100644 index 0000000000..2dba609e2b --- /dev/null +++ b/.changeset/hdx-4405-trailing-action-hint.md @@ -0,0 +1,5 @@ +--- +"@hyperdx/app": patch +--- + +Dashboard table tiles configured with a row-click action now show a trailing arrow-up-right icon at the right edge of each row, revealed on hover, with a small tooltip that names the destination. Actionable rows get a stronger background highlight on hover to reinforce interactivity before the user sees the arrow fade in. The icon click navigates to the same URL as a row click, with all the standard native browser behaviors (cmd-click new tab, middle-click new tab, right-click context menu). diff --git a/packages/app/src/HDXMultiSeriesTableChart.module.scss b/packages/app/src/HDXMultiSeriesTableChart.module.scss new file mode 100644 index 0000000000..416ffe0642 --- /dev/null +++ b/packages/app/src/HDXMultiSeriesTableChart.module.scss @@ -0,0 +1,68 @@ +// Trailing arrow hint for dashboard table tile rows with a +// configured onClick row action. The icon sits at the right edge of +// the last cell and is revealed on row hover. Mirrors the +// .rowButtons pattern from LogTable.module.scss (lines 156-172), +// scoped down to a single icon. See HDX-4405. + +.tableRow { + // Row hover is the visibility trigger for the trailing-icon hint. + // We rely on a descendant selector rather than wrapping a + // tooltip because anchored visibility is tied to the row's own + // lifecycle and never strands a popup after a virtual row + // unmounts. + &:hover .rowActionHint { + opacity: 1; + pointer-events: auto; + } +} + +.actionableRow { + // Stronger hover background on rows that resolve to a click + // destination. Non-actionable rows keep the default + // `bg-muted-hover` utility (`--color-bg-muted`), so the visual + // delta between the two reinforces interactivity before the + // user even sees the trailing arrow fade in. + &:hover { + background-color: var(--color-bg-highlighted); + } +} + +.lastCell { + // Containing block for the absolute-positioned .rowActionHint. + // does not reliably form a positioning context across + // browsers, so we attach `position: relative` to the last + // only. The icon then anchors to the cell's right edge, which + // visually maps to the row's right edge because the last data + // column claims UNDEFINED_WIDTH (see reactTableColumns above). + position: relative; +} + +.rowActionHint { + position: absolute; + top: 50%; + right: 6px; + transform: translateY(-50%); + display: inline-flex; + align-items: center; + justify-content: center; + width: 18px; + height: 18px; + border-radius: 4px; + color: var(--color-text-muted); + background: transparent; + opacity: 0; + pointer-events: none; + transition: opacity 0.15s ease-in-out; + text-decoration: none; + + &:hover, + &:focus-visible { + color: var(--color-text-primary); + background-color: var(--color-bg-muted); + } + + &:focus-visible { + outline: 2px solid var(--color-outline-focus); + outline-offset: 1px; + } +} diff --git a/packages/app/src/HDXMultiSeriesTableChart.tsx b/packages/app/src/HDXMultiSeriesTableChart.tsx index 604e0c3140..5cd3441470 100644 --- a/packages/app/src/HDXMultiSeriesTableChart.tsx +++ b/packages/app/src/HDXMultiSeriesTableChart.tsx @@ -2,7 +2,11 @@ import { useCallback, useMemo, useRef, useState } from 'react'; import Link from 'next/link'; import cx from 'classnames'; import { Tooltip, UnstyledButton } from '@mantine/core'; -import { IconDownload, IconTextWrap } from '@tabler/icons-react'; +import { + IconArrowUpRight, + IconDownload, + IconTextWrap, +} from '@tabler/icons-react'; import { flexRender, getCoreRowModel, @@ -28,6 +32,7 @@ import type { NumberFormat } from './types'; import { formatNumber } from './utils'; import focusStyles from '../styles/focus.module.scss'; +import styles from './HDXMultiSeriesTableChart.module.scss'; export type TableVariant = 'default' | 'muted'; @@ -307,33 +312,6 @@ export const Table = ({ ); const [wrapLinesEnabled, setWrapLinesEnabled] = useState(false); - // Store the virtual index of the hovered row (not its description string) - // so the label re-derives on every render. If the virtualiser replaces the - // row at that index (scroll re-virtualisation, auto-refetch) the label - // reflects the new row immediately rather than showing stale text. Storing - // the index also makes the safety-net `onMouseLeave` on correct: - // it sets null rather than the prior row's description. See HDX-4405. - const [hoveredVirtualIndex, setHoveredVirtualIndex] = useState( - null, - ); - - // Derive the label from whichever row currently occupies hoveredVirtualIndex. - // Returns null when no row is hovered, the index is out of range, or the - // row's action has no URL (error-toast rows show no hint). - const hoveredRowDescription = useMemo(() => { - if (hoveredVirtualIndex == null) return null; - const virtualRow = items.find(v => v.index === hoveredVirtualIndex); - if (!virtualRow) return null; - const row = rows[virtualRow.index] as TableRow | undefined; - if (!row) return null; - const rowAction = getRowAction ? getRowAction(row.original) : null; - return rowAction?.url != null && rowAction.description - ? rowAction.description - : null; - }, [hoveredVirtualIndex, items, rows, getRowAction]); - - const clearHovered = useCallback(() => setHoveredVirtualIndex(null), []); - const { csvData } = useCsvExport( truncatedData, columns.map(col => ({ @@ -398,62 +376,105 @@ export const Table = ({ ))} - {/* Single Tooltip.Floating wrapping the whole so the hint - follows the cursor without being tied to the lifecycle of any - individual virtual row. Per-row Tooltip.Floating instances get - stranded in the Portal when a row unmounts before onMouseLeave - fires (rapid mouse movement in a virtualised list). With this - approach the tooltip state lives on , which never unmounts, - and the label is re-derived from hoveredVirtualIndex each render so - scroll re-virtualisation never shows stale text. See HDX-4405. */} - {hoveredRowDescription} - } - withinPortal - disabled={!hoveredRowDescription} - > - {/* onMouseLeave on is a safety net: if a virtual row - unmounts before its own onMouseLeave fires (rapid cursor - movement or re-virtualisation), leaving the table body still - clears the hovered index. */} - - {paddingTop > 0 && ( - - - - )} - {items.map(virtualRow => { - const row = rows[virtualRow.index] as TableRow; - return ( - setHoveredVirtualIndex(virtualRow.index)} - onMouseLeave={clearHovered} - > - {row.getVisibleCells().map(cell => { - return ( - - {flexRender( - cell.column.columnDef.cell, - cell.getContext(), - )} - - ); - })} - - ); - })} - {paddingBottom > 0 && ( - - + + {paddingTop > 0 && ( + + + + )} + {items.map(virtualRow => { + const row = rows[virtualRow.index] as TableRow; + // Compute the action once per row so the trailing-icon hint + // shares the memoized result from useOnClickLinkBuilder with + // the per-cell renders. The hook keys its cache off the row + // reference via WeakMap (see useOnClickLinkBuilder), so this + // extra call is an O(1) lookup when the per-cell renders + // populate the entry first, and one extra compute per row + // only when the WeakMap entry is cold (e.g. fresh data). + const rowAction = getRowAction ? getRowAction(row.original) : null; + // Narrow `rowAction.url` to a non-null `string` once per row so + // the trailing-icon guard below (and the `` + // sink inside it) doesn't need an `as string` cast and stays + // type-safe under future changes to the `RowAction` shape. + const actionUrl = rowAction?.url ?? null; + const isActionable = actionUrl !== null; + const visibleCells = row.getVisibleCells(); + const lastCellIndex = visibleCells.length - 1; + return ( + + {visibleCells.map((cell, cellIndex) => { + const isLastCell = cellIndex === lastCellIndex; + return ( + + {flexRender( + cell.column.columnDef.cell, + cell.getContext(), + )} + {/* Trailing arrow hint: anchored Mantine Tooltip + wrapping a Next.js Link in the last cell of + rows that resolve to a URL. The icon is hidden + (opacity: 0) by default and revealed on row + hover via the .tableRow:hover .rowActionHint + rule. The Link inherits the same native + cmd-click / middle-click / right-click + semantics as the per-cell Link in the row body. + Suppressed when the row's templates failed + (rowAction.url === null) so the icon never + promises a destination the click won't open. + The arrow-up-right shape signals "navigate + elsewhere" without colliding with the + chevron-right used by sidebar group collapse + / expand affordances. See HDX-4405. */} + {isLastCell && actionUrl !== null && rowAction && ( + + + + + + )} + + ); + })} - )} - - + ); + })} + {paddingBottom > 0 && ( + + + + )} + {isTruncated && (
diff --git a/packages/app/src/__tests__/HDXMultiSeriesTableChart.test.tsx b/packages/app/src/__tests__/HDXMultiSeriesTableChart.test.tsx index 8947a731dc..13fc33b8a1 100644 --- a/packages/app/src/__tests__/HDXMultiSeriesTableChart.test.tsx +++ b/packages/app/src/__tests__/HDXMultiSeriesTableChart.test.tsx @@ -1,5 +1,6 @@ import React from 'react'; -import { fireEvent, screen, waitFor } from '@testing-library/react'; +import { screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import { Table } from '@/HDXMultiSeriesTableChart'; @@ -76,7 +77,59 @@ describe('HDXMultiSeriesTableChart ', () => { }); }); - it('reveals the hint at the cursor when hovering a success row', async () => { + it('marks actionable rows with the actionableRow class so they hover to --color-bg-highlighted', () => { + // Rows with a resolved click destination get a stronger hover + // background than non-actionable rows. The differentiation is + // applied via the `actionableRow` CSS module class on the ; + // non-actionable rows fall through to the global + // `bg-muted-hover` utility (a lighter muted hover). + const getRowAction = jest.fn().mockReturnValue({ + url: '/search?source=src_1&where=', + description: 'Search HyperDX Logs', + }); + + const { container } = renderWithMantine( +
{}} + />, + ); + + const rows = container.querySelectorAll('tbody tr[data-index]'); + expect(rows.length).toBeGreaterThan(0); + rows.forEach(row => { + expect(row.className).toContain('actionableRow'); + expect(row.className).not.toContain('bg-muted-hover'); + }); + }); + + it('renders a trailing arrow hint in the last cell of a success row', () => { + const getRowAction = jest.fn().mockReturnValue({ + url: '/search?source=src_1&where=', + description: 'Search HyperDX Logs', + }); + + renderWithMantine( +
{}} + />, + ); + + const hint = screen.getByTestId('row-action-hint'); + expect(hint.tagName).toBe('A'); + expect(hint.getAttribute('href')).toContain('/search?source=src_1'); + expect(hint.getAttribute('aria-hidden')).toBe('true'); + }); + + it('shows the description in an anchored tooltip when the arrow is hovered', async () => { + const user = userEvent.setup(); const getRowAction = jest.fn().mockReturnValue({ url: '/search?source=src_1&where=', description: 'Search HyperDX Logs', @@ -92,42 +145,31 @@ describe('HDXMultiSeriesTableChart
', () => { />, ); - const row = screen.getByText('web').closest('tr')!; - fireEvent.mouseEnter(row); + const hint = screen.getByTestId('row-action-hint'); + await user.hover(hint); + // Mantine renders the Tooltip label as an element with role="tooltip" + // (with the label text as its accessible name) once opened. await waitFor( () => { - expect(screen.getByText('Search HyperDX Logs')).toBeInTheDocument(); + expect( + screen.getByRole('tooltip', { name: 'Search HyperDX Logs' }), + ).toBeInTheDocument(); }, - { timeout: 1000 }, + { timeout: 2000 }, ); }); - it('hides the hint when the hovered virtual index maps to a no-URL row (HDX-4405)', async () => { - // Regression for the virtualiser race: the hovered can unmount - // before its onMouseLeave fires (rapid movement / data refresh). The - // old per-row Tooltip.Floating was stranded in the Portal; the fix - // stores a virtual index and re-derives the label via useMemo each - // render. When the row at that index no longer has a URL, the tooltip - // hides without a leave event from the (now-gone) element. - // - // We verify the key invariant structurally: hovering index 0 (URL row) - // shows the hint; hovering index 1 (no-URL row) hides it — no - // mouseLeave fires between the two enterevents, simulating the - // cursor jumping over the table faster than leave events dispatch. - const multiRowData = [ - { ServiceName: 'web', Count: 10 }, - { ServiceName: 'api', Count: 5 }, - ]; - const getRowAction = jest.fn((row: { ServiceName: string }) => - row.ServiceName === 'web' - ? { url: '/search?source=src_1&where=', description: 'Search Logs' } - : { url: null, description: '', onClickError: jest.fn() }, - ); + it('renders the dashboard variant description in the tooltip', async () => { + const user = userEvent.setup(); + const getRowAction = jest.fn().mockReturnValue({ + url: '/dashboards/dash_1?where=', + description: 'Open dashboard "API Latency"', + }); renderWithMantine(
', () => { />, ); - const webRow = screen.getByText('web').closest('tr')!; - const apiRow = screen.getByText('api').closest('tr')!; + const hint = screen.getByTestId('row-action-hint'); + await user.hover(hint); - // Hover the URL row — tooltip must appear - fireEvent.mouseEnter(webRow); - await waitFor(() => { - const hint = screen.getByTestId('row-action-hint'); - const tooltipBox = hint.closest('[style*="display"]'); - expect(tooltipBox?.style.display).toBe('block'); - }); - - // Hover the no-URL row WITHOUT firing mouseLeave on the first row. - // This simulates the cursor jumping faster than leave events dispatch. - fireEvent.mouseEnter(apiRow); - - // The label must derive to null (apiRow has url:null) so tooltip hides. - await waitFor(() => { - const hint = screen.getByTestId('row-action-hint'); - const tooltipBox = hint.closest('[style*="display"]'); - expect(tooltipBox?.style.display).toBe('none'); - }); + await waitFor( + () => { + expect( + screen.getByRole('tooltip', { + name: 'Open dashboard "API Latency"', + }), + ).toBeInTheDocument(); + }, + { timeout: 2000 }, + ); }); }); describe('getRowAction failure path', () => { - it('renders the cell as a
', () => { expect(btn.hasAttribute('href')).toBe(false); }); - fireEvent.click(buttons[0]); + await user.click(buttons[0]); expect(onClickError).toHaveBeenCalledTimes(1); }); - it('does not reveal a hint when the row action has no url', async () => { + it('leaves non-actionable rows on bg-muted-hover (no actionableRow class)', () => { + // Mirror of the success-path actionableRow test: rows whose + // action returns `url: null` keep the default muted hover + // utility and never gain the stronger `actionableRow` class. const getRowAction = jest.fn().mockReturnValue({ url: null, description: 'Search HyperDX Logs', onClickError: jest.fn(), }); - renderWithMantine( + const { container } = renderWithMantine(
', () => { />, ); - const row = screen.getByText('web').closest('tr')!; - fireEvent.mouseEnter(row); + const rows = container.querySelectorAll('tbody tr[data-index]'); + expect(rows.length).toBeGreaterThan(0); + rows.forEach(row => { + expect(row.className).not.toContain('actionableRow'); + expect(row.className).toContain('bg-muted-hover'); + }); + }); - // 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(); + it('does not render a trailing arrow hint when rowAction.url is null', () => { + const getRowAction = jest.fn().mockReturnValue({ + url: null, + description: 'Search HyperDX Logs', + onClickError: jest.fn(), + }); + + renderWithMantine( +
{}} + />, + ); + + // The icon must not be in the DOM at all on failure rows: showing + // an arrow would promise a destination the click can't deliver, + // because the click only fires an error toast. + expect(screen.queryByTestId('row-action-hint')).toBeNull(); }); }); describe('legacy getRowSearchLink fallback', () => { - it('renders the cell as a Link without a HoverCard when only getRowSearchLink is provided', () => { + it('renders the cell as a Link without a trailing arrow hint when only getRowSearchLink is provided', () => { const getRowSearchLink = jest .fn() .mockReturnValue('/search?source=legacy&where='); @@ -240,11 +301,15 @@ describe('HDXMultiSeriesTableChart
', () => { expect(link.getAttribute('data-shape')).toBe('link'); expect(link.getAttribute('href')).toContain('/search?source=legacy'); }); + + // No trailing arrow on the legacy path; it's an additive surface + // tied to the new getRowAction prop only. + expect(screen.queryByTestId('row-action-hint')).toBeNull(); }); }); describe('no action configured', () => { - it('renders plain cells with no Link, button, or HoverCard', () => { + it('renders plain cells with no Link, button, or trailing arrow', () => { renderWithMantine(
', () => { expect( screen.queryAllByTestId('dashboard-table-row-action'), ).toHaveLength(0); + expect(screen.queryByTestId('row-action-hint')).toBeNull(); }); }); }); diff --git a/packages/app/tests/e2e/features/dashboard-table-linking.spec.ts b/packages/app/tests/e2e/features/dashboard-table-linking.spec.ts index 2f96395a5c..689544080b 100644 --- a/packages/app/tests/e2e/features/dashboard-table-linking.spec.ts +++ b/packages/app/tests/e2e/features/dashboard-table-linking.spec.ts @@ -752,14 +752,15 @@ test.describe( }); }); - test('Tooltip hint appears on hover and disappears on mouse-leave — no stranded tooltip (HDX-4405)', async ({ + test('Trailing arrow hint appears on row hover and navigates to the action URL (HDX-4405)', async ({ page, }) => { - // Regression test for HDX-4405: Tooltip.Floating instances mounted - // per virtual row were getting stranded in their Portal when the row - // unmounted before onMouseLeave fired (rapid mouse movement). The fix - // moves a single shared Tooltip.Floating to so its state is - // never tied to a virtual row's lifecycle. + // Pivot of the original HDX-4405 regression test: the row-click hint + // is now an anchored Mantine Tooltip wrapping a trailing arrow-up-right + // icon in the last cell, rather than a Tooltip.Floating that tracks the + // cursor. The arrow is hidden until the row is hovered and the + // tooltip is tied to the icon's lifecycle, so no stranded popup is + // possible by construction. const ts = Date.now(); await test.step('Create a table tile with a Search row-click action', async () => { @@ -779,21 +780,39 @@ test.describe( await dashboardPage.waitForTableTileRows(0); - await test.step('Hover over first row — tooltip hint must appear', async () => { - await dashboardPage.hoverFirstTableRowAndGetTooltip(0); - // hoverFirstTableRowAndGetTooltip already asserts visibility before - // returning; reaching here means the tooltip appeared successfully. + await test.step('Trailing arrow is hidden until the row is hovered', async () => { + const hint = dashboardPage.getRowActionHint(0); + // The icon is in the DOM but its visibility is gated by the + // .tableRow:hover .rowActionHint CSS opacity transition. + await expect(hint).toHaveCSS('opacity', '0'); }); - await test.step('Move mouse away from the table — tooltip must disappear', async () => { - // Move to a neutral area well outside the table. The - // onMouseLeave safety net clears hoveredVirtualIndex, which makes - // hoveredRowDescription derive to null, disabling the shared - // Tooltip.Floating (display:none in the Portal). + await test.step('Hovering the row reveals the arrow and the tooltip describes the action', async () => { + const tooltip = await dashboardPage.hoverFirstTableRowAndGetTooltip(0); + await expect(tooltip).toContainText(/Search|Open/); + }); + + await test.step('Moving the cursor away closes the tooltip', async () => { + // Move to a neutral area; Mantine Tooltip closes on mouseleave + // (closeDelay=100ms in the component). Narrow the role match to + // the row-action tooltip text so the assertion does not flake on + // unrelated header / resize-handle tooltips that may also live in + // the portal at the moment of the check. await page.mouse.move(10, 10); - await expect(page.getByTestId('row-action-hint')).toBeHidden({ - timeout: 3000, - }); + await expect( + page.getByRole('tooltip', { name: /Search|Open/ }), + ).toBeHidden({ timeout: 3000 }); + }); + + await test.step('Clicking the trailing arrow navigates to the same destination as the row body', async () => { + // Re-hover the row, then click the arrow. The Link inside the + // tooltip-wrapped arrow points to the same href as the per-cell + // links, so click navigates exactly as a row click would. + const row = dashboardPage.getFirstTableRow(0); + await row.hover(); + const hint = dashboardPage.getRowActionHint(0); + await hint.click(); + await expect(page).toHaveURL(/\/search\?/, { timeout: 10000 }); }); }); }, diff --git a/packages/app/tests/e2e/page-objects/DashboardPage.ts b/packages/app/tests/e2e/page-objects/DashboardPage.ts index a07b9b2d0e..4bbf8b532b 100644 --- a/packages/app/tests/e2e/page-objects/DashboardPage.ts +++ b/packages/app/tests/e2e/page-objects/DashboardPage.ts @@ -1020,30 +1020,42 @@ export class DashboardPage { } /** - * Hover over the first data row of a table tile and wait for the - * floating tooltip to appear. Returns the tooltip locator so callers - * can make further assertions. + * Locator for the trailing arrow hint element rendered in the last + * cell of clickable rows. The icon (arrow-up-right) carries + * `data-testid="row-action-hint"` and is the trigger element for the + * anchored Mantine Tooltip describing the row's onClick destination. + */ + getRowActionHint(tileIndex = 0): Locator { + return this.getTile(tileIndex) + .locator('table tbody tr[data-index]') + .first() + .getByTestId('row-action-hint'); + } + + /** + * Hover the first data row of a table tile, then hover its trailing + * arrow hint so the anchored Mantine Tooltip opens. Returns the + * tooltip locator so callers can assert on the description text. * - * Tooltip.Floating renders its content inside a Portal at the document - * body level. Mantine uses CSS modules (hashed classes) so we locate the - * floating popup by text content that matches the known hint patterns used - * by describeOnClick (e.g. "Open in search", "Search ", - * "Open dashboard"). The tooltip is shown via `display: block` on the - * Portal div, so Playwright's `toBeVisible` checks that correctly. + * The arrow icon (`data-testid="row-action-hint"`) is hidden + * (`opacity: 0`) until the row is hovered. Hovering the row reveals + * the icon via the `.tableRow:hover .rowActionHint` CSS rule. The + * Mantine Tooltip wrapping the icon then opens when the cursor moves + * to the icon itself, rendering its label in a portal at the body. + * + * The returned locator narrows the role match by name so the assertion + * does not collide with header-cell or resize-handle tooltips that + * may also live in the portal at the moment of the check + * (Search-suggestion onClick wording, dashboard-open wording, etc.). */ async hoverFirstTableRowAndGetTooltip(tileIndex = 0): Promise { const row = this.getFirstTableRow(tileIndex); await row.hover(); - // Trigger a mousemove inside the row so Tooltip.Floating's internal - // mousemove handler has a coordinate to position against. - const box = await row.boundingBox(); - if (box) { - await this.page.mouse.move(box.x + box.width / 2, box.y + box.height / 2); - } - // The Tooltip.Floating label is a . - // The portal div uses display:none when disabled, so Playwright's - // toBeVisible() correctly reflects the open/closed state. - const tooltip = this.page.getByTestId('row-action-hint'); + const hint = this.getRowActionHint(tileIndex); + // Hover the icon directly so the anchored Tooltip's mouseEnter + // listener fires; row-hover alone only fades the icon in. + await hint.hover(); + const tooltip = this.page.getByRole('tooltip', { name: /Search|Open/ }); await tooltip.waitFor({ state: 'visible', timeout: 5000 }); return tooltip; }