Skip to content

feat(app): reveal search row-selection checkboxes on hover #3164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MikeShi42 merged 2 commits into from
Sep 21, 2026
Merged

feat(app): reveal search row-selection checkboxes on hover #3164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3a77dae
feat(app): reveal search row-selection checkboxes on hover
cursoragent Sep 20, 2026
31cf6df
fix(app): stop a clicked row checkbox from staying visible, and slim it
cursoragent Sep 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .changeset/search-row-select-hover-reveal.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
'@hyperdx/app': patch
---

feat: reveal search row-selection checkboxes on hover

The multi-select checkbox now fades in when a row is hovered or the checkbox
takes keyboard focus, instead of sitting on every row all the time, and is a
little smaller so its column costs less horizontal room. Selecting
any row reveals every checkbox so shift-click ranges stay aimable, and touch
devices (no hover) keep them visible. The cell keeps its width in every state,
so nothing reflows on hover.
4 changes: 3 additions & 1 deletion packages/app/src/components/DBRowTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1009,7 +1009,9 @@ export const RawLogTable = memo(
/>
)}
<table
className={styles.table}
className={cx(styles.table, {
[styles.selectionActive]: rowSelection.selectedCount > 0,
})}
style={{ minWidth: tableMinWidth }}
id={tableId}
>
Expand Down
16 changes: 13 additions & 3 deletions packages/app/src/components/DBTable/RowSelectionCells.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@ import { Checkbox } from '@mantine/core';

import styles from '@styles/LogTable.module.scss';

export const ROW_SELECTION_COLUMN_WIDTH = 24;
// Slimmer than Mantine's xs (16px): the column is pure overhead on a table
// that is already short of horizontal room. Plus the 4px edge gap the
// checkbox carries in `.selectCheckbox`.
const CHECKBOX_SIZE = 14;

export const ROW_SELECTION_COLUMN_WIDTH = CHECKBOX_SIZE + 4;

@github-actions github-actions Bot Sep 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 minor — Column width hand-syncs a 4px gap that actually lives in SCSS

ROW_SELECTION_COLUMN_WIDTH = CHECKBOX_SIZE + 4 encodes in TS the padding-left: 4px declared on .selectCheckbox in packages/app/styles/LogTable.module.scss:100-104; editing either one alone clips the glyph or leaves dead space, and nothing pins them together. It also means the 18px cell's content box is exactly 14px, so justify-content: center no longer centers anything and the checkbox now sits flush against the expand-chevron cell (previously ~2px of right gap). Drive the gap from one place — drop the padding-left and let the flex centering do the work, or expose it as a CSS custom property the TS constant reads.

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


const CELL_WIDTH_STYLE = {
width: ROW_SELECTION_COLUMN_WIDTH,
Expand All @@ -23,12 +28,17 @@ export const RowSelectionCell = memo(
isSelected: boolean;
onToggle: (rowId: string, options: { extendRange: boolean }) => void;
}) => (
<td className={styles.selectCell} style={CELL_WIDTH_STYLE}>
<td
className={styles.selectCell}
style={CELL_WIDTH_STYLE}
data-testid="row-select-cell"
>
<Checkbox
className={styles.selectCheckbox}
size="xs"
size={`${CHECKBOX_SIZE}px`}

@github-actions github-actions Bot Sep 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 minor — Pointer target shrinks to 14×14 while the surrounding cell stays inert

Mantine renders the Checkbox root as a plain div (no wrapping <label> when label is undefined), so the only clickable element is the <input> sized by --checkbox-size — the 18px td around it does nothing. The target therefore goes from 16×16 to 14×14 on a control that is now also invisible until hovered, which compounds aiming difficulty. Keep the 14px glyph but make .selectCell itself the hit area (a <label htmlFor> filling the cell, or styles={{ input: { margin: 2 } }}-style padding) so the clickable region matches the reserved column.

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

@jordan-simonovski jordan-simonovski Sep 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ I feel as though this one is worth addressing as it seems we've lost 2px in the clickable area of the checkbox.

Worth updating:

const CHECKBOX_SIZE = 16; // column: 20px, down from 24px

checked={isSelected}
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.
onClick={e => e.stopPropagation()}
Expand Down
29 changes: 29 additions & 0 deletions packages/app/styles/LogTable.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ $button-height: 18px;
opacity: 1;
}

&:hover .selectCell {
opacity: 1;
}

&:hover .rowContentButton {
background-color: var(--log-table-muted-bg);
}
Expand All @@ -63,9 +67,34 @@ $button-height: 18px;
}
}

// Search is a read-and-navigate surface, so the checkbox stays out of the way
// until the row is hovered. The cell keeps its width whatever the state —
// revealing the control must never reflow the data columns.
.selectCell {
vertical-align: middle;
user-select: none;
opacity: 0;
transition: opacity 0.15s ease-in-out;

// Tab still reaches the checkbox while it is transparent. Deliberately not
// :focus-within — a mouse click leaves focus on the checkbox, which would
// strand it on screen after the row is unchecked.
&:has(:focus-visible) {
opacity: 1;
}
}

// One checked row puts the table in selection mode and reveals every checkbox,
// so a shift-click range has visible endpoints and checked rows stay marked.
.selectionActive .selectCell {
opacity: 1;
}

// Touch has no hover to reveal with.
@media (hover: none) {
.selectCell {
opacity: 1;
}
}

.selectCheckbox {
Expand Down
13 changes: 13 additions & 0 deletions packages/app/tests/e2e/components/TableComponent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,19 @@ export class TableComponent {
return this.getRow(index).getByTestId('row-select-checkbox');
}

/**
* The cell holding a row's multi-select checkbox. It is the element that
* fades the checkbox in and out, so assert visibility on it rather than on
* the checkbox itself.
*/
getRowCheckboxCell(index: number) {
return this.getRow(index).getByTestId('row-select-cell');
}

async hoverRow(index: number) {
await this.getRow(index).hover();
}

/**
* Select multiple rows by indices
*/
Expand Down
56 changes: 56 additions & 0 deletions packages/app/tests/e2e/features/search/row-multi-select.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,62 @@ test.describe('Search row multi-select', { tag: '@search' }, () => {
await searchPage.goto();
});

test(
'reveals a row checkbox only while the row is hovered',
{ tag: ['@local-mode'] },
async () => {
const checkbox = searchPage.table.getRowCheckboxCell(0);
await expect(checkbox).toHaveCSS('opacity', '0');

await searchPage.table.hoverRow(0);
await expect(checkbox).toHaveCSS('opacity', '1');

await searchPage.table.hoverRow(3);
await expect(checkbox).toHaveCSS('opacity', '0');
},
);

test(
'hides the checkbox again once the row is unchecked',
{ tag: ['@local-mode'] },
async ({ page }) => {
const checkbox = searchPage.table.getRowCheckboxCell(0);

await searchPage.table.selectRows([0]);
await expect(searchPage.selectionCount).toBeVisible();
// Clicking again unchecks it, and leaves focus on the checkbox.
await searchPage.table.selectRows([0]);
await expect(searchPage.selectionCount).toBeHidden();

await searchPage.table.hoverRow(5);
await expect(checkbox).toHaveCSS('opacity', '0');

// That focus must still reveal the checkbox once it is keyboard focus:
// shift-tab walks back from the row's expand button to its checkbox.
await searchPage.table.expandRow(0);
await searchPage.table.hoverRow(5);
await page.keyboard.press('Shift+Tab');
await expect(checkbox).toHaveCSS('opacity', '1');
},
);

test(
'shows every checkbox while a selection exists',
{ tag: ['@local-mode'] },
async () => {
// Row 3 is never hovered here, so it can only be visible because the
// table is in selection mode — which is what keeps shift-click aimable.
const unhovered = searchPage.table.getRowCheckboxCell(3);
await expect(unhovered).toHaveCSS('opacity', '0');

await searchPage.table.selectRows([0]);
await expect(unhovered).toHaveCSS('opacity', '1');

await searchPage.clearRowSelection();
await expect(unhovered).toHaveCSS('opacity', '0');
Comment on lines +12 to +64

@greptile-apps greptile-apps Bot Sep 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Accessibility fallbacks lack coverage

The new tests cover mouse hover and active selection, but not the keyboard :focus-within or touch (hover: none) fallbacks added by this change. A regression in either path could leave row-selection controls invisible for keyboard or touch users while this suite remains green. Add coverage that focuses a checkbox by keyboard and checks initial visibility in a touch-capable browser context.

Knowledge Base Used: Development and test workflows

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!

Fix in Claude Code Fix in Conductor Fix in Cursor Fix in Codex

@github-actions github-actions Bot Sep 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 minor — Final assertion in 'shows every checkbox while a selection exists' depends on where the menu leaves the mouse

clearRowSelection() (SearchPage.ts:233) opens the header menu and clicks row-selection-clear — the 4th Menu.Item in RowSelectionMenu.tsx, in a position="bottom-end" dropdown that overlays the row list. When the menu unmounts the pointer is left resting on whichever row sits at that y, so expect(unhovered).toHaveCSS('opacity', '0') for row 3 passes only because the item happens to land near row 5 rather than row 3; a row-height or menu-padding change silently flips it into a failure. Add await page.mouse.move(0, 0) after clearRowSelection() so the assertion tests selection mode rather than menu geometry.

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

},
);

test(
'selects rows and reports the count',
{ tag: ['@local-mode'] },
Expand Down
Loading