Skip to content

Commit 19c0204

Browse files
committed
fix(app): cell-level hover hint placement and visual parity for failure rows
Addresses three rendering issues Drew flagged on #2321 after approval: - Failure rows now share a single .cellButton SCSS reset with the success-row <Link>, so the cell wrapper renders identically across branches. The user-agent button defaults (padding, font, color, text-align, line-height) were leaking through. - Hover hint is suppressed when action.url === null. The previous state showed an "Open in search" hint on a row whose click would only fire an error toast, which lied to the user. - Row-level HoverCard (position="top") replaced with Tooltip.Floating wrapping the <tr>. The hint now follows the cursor at the cell rather than anchoring at row-center-top. Tests updated: the failure-row hint test now asserts the hint is absent; a success-row positive test was added. Both branches get a data-shape attribute so the test assertions don't couple to the CSS-module hash.
1 parent 5def143 commit 19c0204

3 files changed

Lines changed: 79 additions & 26 deletions

File tree

packages/app/src/HDXMultiSeriesTableChart.tsx

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { useCallback, useMemo, useRef, useState } from 'react';
22
import Link from 'next/link';
33
import cx from 'classnames';
4-
import { HoverCard, Text, UnstyledButton } from '@mantine/core';
4+
import { Tooltip, UnstyledButton } from '@mantine/core';
55
import { IconDownload, IconTextWrap } from '@tabler/icons-react';
66
import {
77
flexRender,
@@ -187,6 +187,7 @@ export const Table = ({
187187
prefetch={false}
188188
className={interactiveClassName}
189189
data-testid="dashboard-table-row-action"
190+
data-shape="link"
190191
>
191192
{formattedValue}
192193
</Link>
@@ -197,15 +198,17 @@ export const Table = ({
197198
// New Tab" stay disabled (a # anchor would silently open
198199
// a meaningless new tab on auxclick before our onClick
199200
// handler runs). The button still surfaces the existing
200-
// notification toast on left-click; the proper "muted row
201-
// + warning icon" preempt state is tracked as AC8.
201+
// notification toast on left-click. focusStyles.cellButton
202+
// resets the user-agent button defaults (padding, font,
203+
// color, text-align, line-height) so the wrapper renders
204+
// identically to the success-row <Link>.
202205
return (
203206
<button
204207
type="button"
205-
className={interactiveClassName}
208+
className={cx(interactiveClassName, focusStyles.cellButton)}
206209
onClick={action.onClickError}
207-
style={{ background: 'none', border: 'none' }}
208210
data-testid="dashboard-table-row-action"
211+
data-shape="button"
209212
>
210213
{formattedValue}
211214
</button>
@@ -221,6 +224,7 @@ export const Table = ({
221224
prefetch={false}
222225
className={interactiveClassName}
223226
data-testid="dashboard-table-row-action"
227+
data-shape="link"
224228
>
225229
{formattedValue}
226230
</Link>
@@ -398,24 +402,24 @@ export const Table = ({
398402
})}
399403
</tr>
400404
);
401-
// Row-level HoverCard so the hint position stays stable as the
402-
// cursor moves between cells in the same row. Mantine's
403-
// HoverCard.Target clones the <tr> and merges its hover ref
404-
// with the virtualizer's measureElement ref.
405-
if (rowAction) {
405+
// Row-level Tooltip.Floating so the hint follows the cursor
406+
// and anchors near the cell the user is over, not at the row's
407+
// center-top. Tooltip.Floating tracks the cursor via floating-ui
408+
// and stays within the row's bounding box; one tooltip per row
409+
// means no flicker as the cursor moves between cells.
410+
//
411+
// The hint is suppressed when rowAction.url === null because
412+
// the click only fires an error toast on those rows, so showing
413+
// "Open in search" would mislead the user.
414+
if (rowAction && rowAction.url) {
406415
return (
407-
<HoverCard
416+
<Tooltip.Floating
408417
key={virtualRow.key}
418+
label={rowAction.description}
409419
withinPortal
410-
shadow="md"
411-
openDelay={250}
412-
position="top"
413420
>
414-
<HoverCard.Target>{tr}</HoverCard.Target>
415-
<HoverCard.Dropdown py="xs" px="sm">
416-
<Text size="xs">{rowAction.description}</Text>
417-
</HoverCard.Dropdown>
418-
</HoverCard>
421+
{tr}
422+
</Tooltip.Floating>
419423
);
420424
}
421425
return tr;

packages/app/src/__tests__/HDXMultiSeriesTableChart.test.tsx

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,37 @@ describe('HDXMultiSeriesTableChart <Table>', () => {
7171
expect(links.length).toBeGreaterThan(0);
7272
links.forEach(link => {
7373
expect(link.tagName).toBe('A');
74+
expect(link.getAttribute('data-shape')).toBe('link');
7475
expect(link.getAttribute('href')).toContain('/search?source=src_1');
7576
});
7677
});
78+
79+
it('reveals the hint at the cursor when hovering a success row', async () => {
80+
const getRowAction = jest.fn().mockReturnValue({
81+
url: '/search?source=src_1&where=',
82+
description: 'Search HyperDX Logs',
83+
});
84+
85+
renderWithMantine(
86+
<Table
87+
data={baseData}
88+
columns={baseColumns}
89+
getRowAction={getRowAction}
90+
sorting={[]}
91+
onSortingChange={() => {}}
92+
/>,
93+
);
94+
95+
const row = screen.getByText('web').closest('tr')!;
96+
fireEvent.mouseEnter(row);
97+
98+
await waitFor(
99+
() => {
100+
expect(screen.getByText('Search HyperDX Logs')).toBeInTheDocument();
101+
},
102+
{ timeout: 1000 },
103+
);
104+
});
77105
});
78106

79107
describe('getRowAction failure path', () => {
@@ -99,6 +127,7 @@ describe('HDXMultiSeriesTableChart <Table>', () => {
99127
expect(buttons.length).toBeGreaterThan(0);
100128
buttons.forEach(btn => {
101129
expect(btn.tagName).toBe('BUTTON');
130+
expect(btn.getAttribute('data-shape')).toBe('button');
102131
// No href anywhere, so cmd-click / middle-click / right-click
103132
// "Open in New Tab" can't silently open the page.
104133
expect(btn.hasAttribute('href')).toBe(false);
@@ -108,7 +137,7 @@ describe('HDXMultiSeriesTableChart <Table>', () => {
108137
expect(onClickError).toHaveBeenCalledTimes(1);
109138
});
110139

111-
it('reveals the row-level HoverCard hint when the cursor hovers the row', async () => {
140+
it('does not reveal a hint when the row action has no url', async () => {
112141
const getRowAction = jest.fn().mockReturnValue({
113142
url: null,
114143
description: 'Search HyperDX Logs',
@@ -128,12 +157,9 @@ describe('HDXMultiSeriesTableChart <Table>', () => {
128157
const row = screen.getByText('web').closest('tr')!;
129158
fireEvent.mouseEnter(row);
130159

131-
await waitFor(
132-
() => {
133-
expect(screen.getByText('Search HyperDX Logs')).toBeInTheDocument();
134-
},
135-
{ timeout: 1000 },
136-
);
160+
// Give the tooltip a chance to mount; assert it never does.
161+
await new Promise(resolve => setTimeout(resolve, 250));
162+
expect(screen.queryByText('Search HyperDX Logs')).not.toBeInTheDocument();
137163
});
138164
});
139165

@@ -156,6 +182,7 @@ describe('HDXMultiSeriesTableChart <Table>', () => {
156182
const links = screen.getAllByTestId('dashboard-table-row-action');
157183
links.forEach(link => {
158184
expect(link.tagName).toBe('A');
185+
expect(link.getAttribute('data-shape')).toBe('link');
159186
expect(link.getAttribute('href')).toContain('/search?source=legacy');
160187
});
161188
});

packages/app/styles/focus.module.scss

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,25 @@
33
outline-offset: 2px;
44
border-radius: var(--mantine-radius-xs);
55
}
6+
7+
// Used by the failure-row fallback <button> in the dashboard table
8+
// tile cell wrapper so it renders identically to the success-row
9+
// <Link>. Resets the user-agent button defaults (padding, font,
10+
// color, text-align, line-height) that the <a> doesn't carry and
11+
// reuses the same focus-visible ring as .focusRing.
12+
.cellButton {
13+
padding: 0;
14+
font: inherit;
15+
color: inherit;
16+
text-align: inherit;
17+
line-height: inherit;
18+
background: none;
19+
border: none;
20+
cursor: pointer;
21+
}
22+
23+
.cellButton:focus-visible {
24+
outline: 2px solid var(--color-outline-focus);
25+
outline-offset: 2px;
26+
border-radius: var(--mantine-radius-xs);
27+
}

0 commit comments

Comments
 (0)