Skip to content

Commit 1c6b96f

Browse files
authored
fix(core): don't apply hasHover highlight to Table header row (#3244)
Closes #2734. hasHover applied the hover highlight to every <tr>, including the header row, because the header is rendered with the same RowComponent (TableRow) as body rows and TableRow pulled hover/striped styling from context unconditionally. Add an isHeaderRow flag (set on the header row in BaseTable) so the header opts out of striped/hover styling, which is meant for body rows only. +regression test. Co-authored-by: Durvesh Pilankar <durveshpilankar@meta.com>
1 parent e839b09 commit 1c6b96f

5 files changed

Lines changed: 58 additions & 37 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@astryxdesign/core': patch
3+
---
4+
5+
[fix] Table: the header row no longer picks up the `hasHover` row highlight — hover (and striped) styling now applies to body rows only. Adds an internal `isHeaderRow` flag on the row component so the header row in `<thead>` opts out (#2734)
6+
@durvesh1992

packages/core/src/Table/BaseTable.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,7 @@ function BaseTableInner<T extends Record<string, unknown>>({
477477
<TableHeader>
478478
<RowComponent
479479
{...headerRowRenderProps.htmlProps}
480+
isHeaderRow
480481
xstyle={headerRowRenderProps.styles}>
481482
{headerRowRenderProps.children}
482483
</RowComponent>

packages/core/src/Table/Table.test.tsx

Lines changed: 29 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,27 @@ describe('BaseTable', () => {
245245
expect(screen.getAllByRole('row')).toHaveLength(4);
246246
});
247247

248+
it('does not apply hover styling to the header row when hasHover is set', () => {
249+
// Use the public Table (provides TableContext); BaseTable alone has no
250+
// context, so rows wouldn't pick up hover styling at all.
251+
const {container} = render(
252+
<Table data={users} columns={columns} hasHover />,
253+
);
254+
const headerRow = container.querySelector('thead tr');
255+
const bodyRow = container.querySelector('tbody tr');
256+
expect(headerRow).not.toBeNull();
257+
expect(bodyRow).not.toBeNull();
258+
259+
const headerClasses = new Set(
260+
(headerRow?.className ?? '').split(/\s+/).filter(Boolean),
261+
);
262+
// The body row carries hover-styling class(es) that the header row does not.
263+
const bodyOnlyClasses = (bodyRow?.className ?? '')
264+
.split(/\s+/)
265+
.filter(c => c && !headerClasses.has(c));
266+
expect(bodyOnlyClasses.length).toBeGreaterThan(0);
267+
});
268+
248269
it('auto-generates columns from data keys when columns omitted', () => {
249270
render(<BaseTable data={users} />);
250271
const headers = screen.getAllByRole('columnheader');
@@ -276,11 +297,7 @@ describe('BaseTable', () => {
276297

277298
it('uses idKey function to key rows', () => {
278299
render(
279-
<BaseTable
280-
data={users}
281-
columns={columns}
282-
idKey={item => item.email}
283-
/>,
300+
<BaseTable data={users} columns={columns} idKey={item => item.email} />,
284301
);
285302
expect(screen.getAllByRole('row')).toHaveLength(4);
286303
});
@@ -374,9 +391,7 @@ describe('BaseTable', () => {
374391
htmlProps: {...props.htmlProps, 'data-testid': 'plugin-table'},
375392
}),
376393
};
377-
render(
378-
<BaseTable data={users} columns={columns} plugins={[plugin]} />,
379-
);
394+
render(<BaseTable data={users} columns={columns} plugins={[plugin]} />);
380395
expect(screen.getByTestId('plugin-table')).toBeInTheDocument();
381396
});
382397

@@ -387,9 +402,7 @@ describe('BaseTable', () => {
387402
htmlProps: {...props.htmlProps, 'data-testid': 'plugin-header-row'},
388403
}),
389404
};
390-
render(
391-
<BaseTable data={users} columns={columns} plugins={[plugin]} />,
392-
);
405+
render(<BaseTable data={users} columns={columns} plugins={[plugin]} />);
393406
expect(screen.getByTestId('plugin-header-row')).toBeInTheDocument();
394407
});
395408

@@ -404,9 +417,7 @@ describe('BaseTable', () => {
404417
};
405418
},
406419
};
407-
render(
408-
<BaseTable data={users} columns={columns} plugins={[plugin]} />,
409-
);
420+
render(<BaseTable data={users} columns={columns} plugins={[plugin]} />);
410421
expect(receivedKeys).toEqual(['name', 'age', 'email']);
411422
const headers = screen.getAllByRole('columnheader');
412423
expect(headers[0]).toHaveAttribute('data-column', 'name');
@@ -423,9 +434,7 @@ describe('BaseTable', () => {
423434
};
424435
},
425436
};
426-
render(
427-
<BaseTable data={users} columns={columns} plugins={[plugin]} />,
428-
);
437+
render(<BaseTable data={users} columns={columns} plugins={[plugin]} />);
429438
expect(receivedItems).toEqual(['Alice', 'Bob', 'Charlie']);
430439
});
431440

@@ -437,9 +446,7 @@ describe('BaseTable', () => {
437446
return props;
438447
},
439448
};
440-
render(
441-
<BaseTable data={users} columns={columns} plugins={[plugin]} />,
442-
);
449+
render(<BaseTable data={users} columns={columns} plugins={[plugin]} />);
443450
// 3 rows * 3 columns = 9 calls
444451
expect(calls).toHaveLength(9);
445452
expect(calls[0]).toEqual({col: 'name', name: 'Alice'});
@@ -746,11 +753,7 @@ describe('Table', () => {
746753
}),
747754
};
748755
render(
749-
<Table
750-
data={users}
751-
columns={columns}
752-
plugins={{custom: userPlugin}}
753-
/>,
756+
<Table data={users} columns={columns} plugins={{custom: userPlugin}} />,
754757
);
755758
expect(screen.getByTestId('custom-plugin')).toBeInTheDocument();
756759
});
@@ -767,11 +770,7 @@ describe('Table', () => {
767770
},
768771
};
769772
render(
770-
<Table
771-
data={users}
772-
columns={columns}
773-
plugins={{custom: userPlugin}}
774-
/>,
773+
<Table data={users} columns={columns} plugins={{custom: userPlugin}} />,
775774
);
776775
expect(screen.getByTestId('after-xds')).toBeInTheDocument();
777776
});

packages/core/src/Table/TableRow.tsx

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,11 @@ export interface TableRowProps extends BaseProps<HTMLTableRowElement> {
3434
* Must be a `stylex.create()` value — not an inline style object.
3535
*/
3636
xstyle?: StyleXStyles[];
37+
/**
38+
* Whether this row is the header row. Header rows skip the striped/hover
39+
* row styling (which is only meant for body rows).
40+
*/
41+
isHeaderRow?: boolean;
3742
}
3843

3944
const stripedRowStyles = stylex.create({
@@ -114,6 +119,7 @@ export function TableRow({
114119
children,
115120
xstyle,
116121
ref,
122+
isHeaderRow = false,
117123
...props
118124
}: TableRowProps) {
119125
const ctx = use(TableContext);
@@ -134,13 +140,17 @@ export function TableRow({
134140

135141
const rowStyles: StyleXStyles[] = [];
136142

137-
// Handle striped + hover combination to avoid backgroundColor conflicts
138-
if (ctx.isStriped && ctx.hasHover) {
139-
rowStyles.push(stripedHoverRowStyles.row);
140-
} else if (ctx.isStriped) {
141-
rowStyles.push(stripedRowStyles.row);
142-
} else if (ctx.hasHover) {
143-
rowStyles.push(hoverRowStyles.row);
143+
// Striped + hover styling is for body rows only — the header row must not
144+
// pick up the hover highlight (or striping) even when hasHover/isStriped.
145+
if (!isHeaderRow) {
146+
// Handle striped + hover combination to avoid backgroundColor conflicts
147+
if (ctx.isStriped && ctx.hasHover) {
148+
rowStyles.push(stripedHoverRowStyles.row);
149+
} else if (ctx.isStriped) {
150+
rowStyles.push(stripedRowStyles.row);
151+
} else if (ctx.hasHover) {
152+
rowStyles.push(hoverRowStyles.row);
153+
}
144154
}
145155

146156
if (ctx.dividers === 'rows' || ctx.dividers === 'grid') {

packages/core/src/Table/types.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,11 @@ export interface TableRowComponentProps extends HTMLAttributes<HTMLTableRowEleme
421421
ref?: Ref<HTMLTableRowElement>;
422422
children: ReactNode;
423423
xstyle?: StyleXStyles[];
424+
/**
425+
* Whether this row is the header row. Header rows skip the striped/hover
426+
* row styling, which is only meant for body rows.
427+
*/
428+
isHeaderRow?: boolean;
424429
}
425430

426431
/** Props for cell components used in the components prop */

0 commit comments

Comments
 (0)