Skip to content

Commit 4c7688e

Browse files
nicolethoenclaude
nicolethoen
and
claude
authored
fix(SkeletonTableHead): prevent nested th elements when passing Th components (#821)
When React elements (like <Th>) were passed in the columns prop, they were being wrapped in an additional <Th> element, causing invalid DOM structure with nested <th> elements. This fix checks if a column is already a React element and renders it directly without wrapping it in an additional Th component. Fixes #764 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1c9449f commit 4c7688e

File tree

3 files changed

+149
-11
lines changed

3 files changed

+149
-11
lines changed

‎packages/module/src/SkeletonTableHead/SkeletonTableHead.test.tsx‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,35 @@
11
import { render } from '@testing-library/react';
22
import SkeletonTableHead from './SkeletonTableHead';
3-
import { Table } from '@patternfly/react-table';
3+
import { Table, Th } from '@patternfly/react-table';
44

55
describe('SkeletonTableHead component', () => {
66
it('should render correctly with count', () => {
77
expect(render(<Table><SkeletonTableHead columnsCount={2} isSelectable isExpandable /></Table>)).toMatchSnapshot();
88
});
99

10-
it('should render correctly with count', () => {
10+
it('should render correctly with string columns', () => {
1111
expect(render(<Table><SkeletonTableHead columns={[ 'First', 'Second' ]} isTreeTable isSelectable /></Table>)).toMatchSnapshot();
1212
});
13+
14+
it('should render correctly with Th element columns without nesting', () => {
15+
const { container } = render(
16+
<Table>
17+
<SkeletonTableHead
18+
columns={[
19+
<Th key="1" sort={{ columnIndex: 0, sortBy: {} }}>First</Th>,
20+
<Th key="2" sort={{ columnIndex: 1, sortBy: {} }}>Second</Th>
21+
]}
22+
/>
23+
</Table>
24+
);
25+
26+
// Ensure there are no nested <th> elements
27+
const theadCells = container.querySelectorAll('thead th');
28+
theadCells.forEach(th => {
29+
const nestedTh = th.querySelector('th');
30+
expect(nestedTh).toBeNull();
31+
});
32+
33+
expect(container).toMatchSnapshot();
34+
});
1335
});

‎packages/module/src/SkeletonTableHead/SkeletonTableHead.tsx‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import type { FunctionComponent } from 'react';
2-
import { ReactNode, useMemo } from 'react';
1+
import type { FunctionComponent, ReactElement } from 'react';
2+
import { ReactNode, useMemo, isValidElement } from 'react';
33
import {
44
Th,
55
ThProps,
@@ -10,6 +10,8 @@ import { Skeleton } from '@patternfly/react-core';
1010

1111
export const isThObject = (value: ReactNode | { cell: ReactNode; props?: ThProps }): value is { cell: ReactNode; props?: ThProps } => value != null && typeof value === 'object' && 'cell' in value;
1212

13+
export const isReactElement = (value: ReactNode): value is ReactElement => isValidElement(value);
14+
1315
export interface SkeletonTableHeadProps {
1416
/** Custom columns for the table */
1517
columns?: (ReactNode | { cell: ReactNode; props?: ThProps })[];
@@ -41,18 +43,33 @@ export const SkeletonTableHead: FunctionComponent<SkeletonTableHeadProps> = ({
4143
...(isExpandable ? [ <Th key="row-expand" screenReaderText='Data expansion table header cell' /> ] : []),
4244
...(isSelectable && !isTreeTable ? [ <Th key="row-select" screenReaderText='Data selection table header cell' /> ] : []),
4345
...(hasCustomColumns ? (
44-
columns.map((column, index) => (
45-
<Th key={index} {...(isThObject(column) && column?.props)} data-ouia-component-id={`${ouiaId}-th-${index}`}>
46-
{isThObject(column) ? column.cell : column}
47-
</Th>
48-
))
46+
columns.map((column, index) => {
47+
// If the column is an object with cell and props, wrap in Th
48+
if (isThObject(column)) {
49+
return (
50+
<Th key={index} {...column.props} data-ouia-component-id={`${ouiaId}-th-${index}`}>
51+
{column.cell}
52+
</Th>
53+
);
54+
}
55+
// If the column is already a React element (like <Th>), render it directly
56+
if (isReactElement(column)) {
57+
return column;
58+
}
59+
// Otherwise, wrap the content in Th
60+
return (
61+
<Th key={index} data-ouia-component-id={`${ouiaId}-th-${index}`}>
62+
{column}
63+
</Th>
64+
);
65+
})
4966
) : (
5067
[ ...Array(rowCellsCount) ].map((_, index) => (
5168
<Th key={index} data-ouia-component-id={`${ouiaId}-th-${index}`}>
5269
<Skeleton />
5370
</Th>
5471
))
55-
))
72+
))
5673
]
5774
, [ hasCustomColumns, isExpandable, isSelectable, isTreeTable, columns, rowCellsCount, ouiaId ]);
5875

‎packages/module/src/SkeletonTableHead/__snapshots__/SkeletonTableHead.test.tsx.snap‎

Lines changed: 100 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,104 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3+
exports[`SkeletonTableHead component should render correctly with Th element columns without nesting 1`] = `
4+
<div>
5+
<table
6+
class="pf-v6-c-table pf-m-grid-md"
7+
data-ouia-component-id="OUIA-Generated-Table-3"
8+
data-ouia-component-type="PF6/Table"
9+
data-ouia-safe="true"
10+
role="grid"
11+
>
12+
<thead
13+
class="pf-v6-c-table__thead"
14+
data-ouia-component-id="SkeletonTableHeader-thead"
15+
>
16+
<tr
17+
class="pf-v6-c-table__tr"
18+
data-ouia-component-id="SkeletonTableHeader-tr-head"
19+
data-ouia-component-type="PF6/TableRow"
20+
data-ouia-safe="true"
21+
>
22+
<th
23+
class="pf-v6-c-table__th pf-v6-c-table__sort"
24+
scope="col"
25+
tabindex="-1"
26+
>
27+
<button
28+
class="pf-v6-c-table__button"
29+
type="button"
30+
>
31+
<div
32+
class="pf-v6-c-table__button-content"
33+
>
34+
<span
35+
class="pf-v6-c-table__text"
36+
>
37+
First
38+
</span>
39+
<span
40+
class="pf-v6-c-table__sort-indicator"
41+
>
42+
<svg
43+
aria-hidden="true"
44+
class="pf-v6-svg"
45+
fill="currentColor"
46+
height="1em"
47+
role="img"
48+
viewBox="0 0 256 512"
49+
width="1em"
50+
>
51+
<path
52+
d="M214.059 377.941H168V134.059h46.059c21.382 0 32.09-25.851 16.971-40.971L144.971 7.029c-9.373-9.373-24.568-9.373-33.941 0L24.971 93.088c-15.119 15.119-4.411 40.971 16.971 40.971H88v243.882H41.941c-21.382 0-32.09 25.851-16.971 40.971l86.059 86.059c9.373 9.373 24.568 9.373 33.941 0l86.059-86.059c15.12-15.119 4.412-40.971-16.97-40.971z"
53+
/>
54+
</svg>
55+
</span>
56+
</div>
57+
</button>
58+
</th>
59+
<th
60+
class="pf-v6-c-table__th pf-v6-c-table__sort"
61+
scope="col"
62+
tabindex="-1"
63+
>
64+
<button
65+
class="pf-v6-c-table__button"
66+
type="button"
67+
>
68+
<div
69+
class="pf-v6-c-table__button-content"
70+
>
71+
<span
72+
class="pf-v6-c-table__text"
73+
>
74+
Second
75+
</span>
76+
<span
77+
class="pf-v6-c-table__sort-indicator"
78+
>
79+
<svg
80+
aria-hidden="true"
81+
class="pf-v6-svg"
82+
fill="currentColor"
83+
height="1em"
84+
role="img"
85+
viewBox="0 0 256 512"
86+
width="1em"
87+
>
88+
<path
89+
d="M214.059 377.941H168V134.059h46.059c21.382 0 32.09-25.851 16.971-40.971L144.971 7.029c-9.373-9.373-24.568-9.373-33.941 0L24.971 93.088c-15.119 15.119-4.411 40.971 16.971 40.971H88v243.882H41.941c-21.382 0-32.09 25.851-16.971 40.971l86.059 86.059c9.373 9.373 24.568 9.373 33.941 0l86.059-86.059c15.12-15.119 4.412-40.971-16.97-40.971z"
90+
/>
91+
</svg>
92+
</span>
93+
</div>
94+
</button>
95+
</th>
96+
</tr>
97+
</thead>
98+
</table>
99+
</div>
100+
`;
101+
3102
exports[`SkeletonTableHead component should render correctly with count 1`] = `
4103
{
5104
"asFragment": [Function],
@@ -203,7 +302,7 @@ exports[`SkeletonTableHead component should render correctly with count 1`] = `
203302
}
204303
`;
205304

206-
exports[`SkeletonTableHead component should render correctly with count 2`] = `
305+
exports[`SkeletonTableHead component should render correctly with string columns 1`] = `
207306
{
208307
"asFragment": [Function],
209308
"baseElement": <body>

0 commit comments

Comments
 (0)