Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { forwardRef } from 'react';
import { css } from '@patternfly/react-styles';
import styles from '@patternfly/react-styles/css/components/Table/table-scrollable';

Expand All @@ -6,16 +7,22 @@ export interface InnerScrollContainerProps extends React.HTMLProps<HTMLDivElemen
children?: React.ReactNode;
/** Additional classes added to the container */
className?: string;
/** @hide Forwarded ref */
innerRef?: React.Ref<HTMLDivElement>;
}

export const InnerScrollContainer: React.FunctionComponent<InnerScrollContainerProps> = ({
const InnerScrollContainerBase: React.FunctionComponent<InnerScrollContainerProps> = ({
children,
className,
innerRef,
...props
}: InnerScrollContainerProps) => (
<div className={css(className, styles.scrollInnerWrapper)} {...props}>
<div ref={innerRef} className={css(className, styles.scrollInnerWrapper)} {...props}>
{children}
</div>
);

export const InnerScrollContainer = forwardRef((props: InnerScrollContainerProps, ref: React.Ref<HTMLDivElement>) => (
<InnerScrollContainerBase innerRef={ref} {...props} />
));
InnerScrollContainer.displayName = 'InnerScrollContainer';
10 changes: 9 additions & 1 deletion packages/react-table/src/components/Table/Table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,12 @@ export interface TableProps extends React.HTMLProps<HTMLTableElement>, OUIAProps
isPlain?: boolean;
/** @beta Flag indicating if the table should not have plain styling when in the glass theme */
isNoPlainOnGlass?: boolean;
/** If set to true, the table header sticks to the top of its container */
/** If set to true, the table header sticks to the top of its container. This property applies both the sticky position and styling. */
isStickyHeader?: boolean;
/** @beta Flag indicating the table header should have sticky positioning to the top of the parent InnerScrollContainer. */
isStickyHeaderBase?: boolean;
/** @beta Flag indicating the table header should have stuck styling, when the header is not at the top of the scroll container. */
isStickyHeaderStuck?: boolean;
/** @hide Forwarded ref */
innerRef?: React.RefObject<any>;
/** Flag indicating table is a tree table */
Expand Down Expand Up @@ -98,6 +102,8 @@ const TableBase: React.FunctionComponent<TableProps> = ({
variant,
borders = true,
isStickyHeader = false,
isStickyHeaderBase = false,
isStickyHeaderStuck = false,
isPlain = false,
isNoPlainOnGlass = false,
gridBreakPoint = TableGridBreakpoint.gridMd,
Expand Down Expand Up @@ -225,6 +231,8 @@ const TableBase: React.FunctionComponent<TableProps> = ({
styles.modifiers[variant],
!borders && styles.modifiers.noBorderRows,
isStickyHeader && styles.modifiers.stickyHeader,
isStickyHeaderBase && styles.modifiers.stickyHeaderBase,
isStickyHeaderStuck && styles.modifiers.stickyHeaderStuck,
isTreeTable && stylesTreeView.modifiers.treeView,
isStriped && styles.modifiers.striped,
isExpandable && styles.modifiers.expandable,
Expand Down
36 changes: 36 additions & 0 deletions packages/react-table/src/components/Table/__tests__/Table.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -186,3 +186,39 @@ test(`Does not render with class ${styles.modifiers.noPlainOnGlass} when isNoPla

expect(screen.getByRole('grid', { name: 'Test table' })).not.toHaveClass(styles.modifiers.noPlainOnGlass);
});

test(`Renders with class ${styles.modifiers.stickyHeaderBase} when isStickyHeaderBase is true`, () => {
render(<Table isStickyHeaderBase aria-label="Test table" />);

expect(screen.getByRole('grid', { name: 'Test table' })).toHaveClass(styles.modifiers.stickyHeaderBase);
});

test(`Does not render with class ${styles.modifiers.stickyHeaderBase} when isStickyHeaderBase is false`, () => {
render(<Table isStickyHeaderBase={false} aria-label="Test table" />);

expect(screen.getByRole('grid', { name: 'Test table' })).not.toHaveClass(styles.modifiers.stickyHeaderBase);
});

test(`Does not render with class ${styles.modifiers.stickyHeaderBase} when isStickyHeaderBase is undefined`, () => {
render(<Table aria-label="Test table" />);

expect(screen.getByRole('grid', { name: 'Test table' })).not.toHaveClass(styles.modifiers.stickyHeaderBase);
});

test(`Renders with class ${styles.modifiers.stickyHeaderStuck} when isStickyHeaderStuck is true`, () => {
render(<Table isStickyHeaderStuck aria-label="Test table" />);

expect(screen.getByRole('grid', { name: 'Test table' })).toHaveClass(styles.modifiers.stickyHeaderStuck);
});

test(`Does not render with class ${styles.modifiers.stickyHeaderStuck} when isStickyHeaderStuck is false`, () => {
render(<Table isStickyHeaderStuck={false} aria-label="Test table" />);

expect(screen.getByRole('grid', { name: 'Test table' })).not.toHaveClass(styles.modifiers.stickyHeaderStuck);
});

test(`Does not render with class ${styles.modifiers.stickyHeaderStuck} when isStickyHeaderStuck is undefined`, () => {
render(<Table aria-label="Test table" />);

expect(screen.getByRole('grid', { name: 'Test table' })).not.toHaveClass(styles.modifiers.stickyHeaderStuck);
});
13 changes: 11 additions & 2 deletions packages/react-table/src/components/Table/examples/Table.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ The `Table` component takes an explicit and declarative approach, and its implem

The documentation for the deprecated table implementation can be found under the [React deprecated](/components/table/react-deprecated) tab. It is configuration based and takes a less declarative and more implicit approach to laying out the table structure, such as the rows and cells within it.

import { Fragment, isValidElement, useCallback, useEffect, useRef, useState } from 'react';
import { Fragment, isValidElement, useCallback, useEffect, useRef, useState, useLayoutEffect } from 'react';
import SearchIcon from '@patternfly/react-icons/dist/esm/icons/search-icon';
import CodeBranchIcon from '@patternfly/react-icons/dist/esm/icons/code-branch-icon';
import CodeIcon from '@patternfly/react-icons/dist/esm/icons/code-icon';
Expand Down Expand Up @@ -327,7 +327,6 @@ To enable a tree table:
- `checkAriaLabel` - (optional) accessible label for the checkbox
- `showDetailsAriaLabel` - (optional) accessible label for the show row details button in the responsive view
4. The first `Td` in each row will pass the following to the `treeRow` prop:

- `onCollapse` - Callback when user expands/collapses a row to reveal/hide the row's children.
- `onCheckChange` - (optional) Callback when user changes the checkbox on a row.
- `onToggleRowDetails` - (optional) Callback when user shows/hides the row details in responsive view.
Expand Down Expand Up @@ -419,6 +418,16 @@ To prevent the default text wrapping behavior and allow horizontal scrolling, al

```

### Dynamic sticky header

A sticky header may alternatively be implemented with two properties: `isStickyHeaderBase` and `isStickyHeaderStuck` - which allows separate control of the sticky position and sticky styling. `isStickyHeaderBase` should always be applied to make the header position sticky, and `isStickyHeaderStuck` may be applied dynamically to enable the sticky styling, such as when the sticky header is not at the top of the scroll parent as shown in the example.

`isStickyHeader` acts as if both properties are present and true when applied, and is useful when dynamic sticky styling is not necessary.
Comment on lines +423 to +425
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.

@edonehoo could you give this a quick look?


```ts file="TableStickyHeaderDynamic.tsx"

```

### Sticky columns and header

To maintain proper sticky behavior across sticky columns and header, `Table` must be wrapped with `OuterScrollContainer` and `InnerScrollContainer`.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
import { useLayoutEffect, useRef, useState } from 'react';
import { Table, Thead, Tr, Th, Tbody, Td, InnerScrollContainer } from '@patternfly/react-table';
import BlueprintIcon from '@patternfly/react-icons/dist/esm/icons/blueprint-icon';

interface Fact {
name: string;
state: string;
detail1: string;
detail2: string;
detail3: string;
detail4: string;
detail5: string;
detail6: string;
detail7: string;
}

const useIsStuckFromScrollParent = ({
shouldTrack,
scrollParentRef
}: {
/** Indicates whether to track the scroll top position of the scroll parent element */
shouldTrack: boolean;
/** Reference to the scroll parent element */
scrollParentRef: React.RefObject<any>;
}): boolean => {
Comment on lines +17 to +25
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify the ref type exposed/expected by InnerScrollContainer and align this example type.
fd -i "InnerScrollContainer.tsx"
rg -n -C3 "forwardRef|InnerScrollContainer|RefObject|HTMLDivElement|HTMLElement" packages/react-table/src/components/Table/InnerScrollContainer.tsx
rg -n -C2 "useIsStuckFromScrollParent|scrollParentRef" packages/react-table/src/components/Table/examples/TableStickyHeaderDynamic.tsx

Repository: patternfly/patternfly-react

Length of output: 2319


Replace any on scrollParentRef with concrete DOM type for type safety.

Line 24 uses React.RefObject<any>, losing compile-time checks for DOM APIs like addEventListener. The ref is only used with HTMLDivElement instances.

Proposed diff
-  scrollParentRef: React.RefObject<any>;
+  scrollParentRef: React.RefObject<HTMLDivElement>;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@packages/react-table/src/components/Table/examples/TableStickyHeaderDynamic.tsx`
around lines 17 - 25, The parameter type for scrollParentRef in
useIsStuckFromScrollParent is currently React.RefObject<any>, which loses DOM
type safety; change it to React.RefObject<HTMLDivElement> (or an appropriate
HTMLElement subtype) in the function signature and any related usages so callers
and TypeScript know the ref points to a div and you can safely call DOM methods
like addEventListener and scrollTop on scrollParentRef.current.

const [isStuck, setIsStuck] = useState(false);

useLayoutEffect(() => {
if (!shouldTrack) {
setIsStuck(false);
return;
}

const scrollElement = scrollParentRef.current;
if (!scrollElement) {
setIsStuck(false);
return;
}

const syncFromScroll = () => {
setIsStuck(scrollElement.scrollTop > 0);
};
syncFromScroll();
scrollElement.addEventListener('scroll', syncFromScroll, { passive: true });
return () => scrollElement.removeEventListener('scroll', syncFromScroll);
}, [shouldTrack, scrollParentRef]);

return isStuck;
};

export const TableStickyHeaderDynamic: React.FunctionComponent = () => {
const scrollContainerRef = useRef<HTMLDivElement>(null);
const isStuck = useIsStuckFromScrollParent({ shouldTrack: true, scrollParentRef: scrollContainerRef });

// In real usage, this data would come from some external source like an API via props.
const facts: Fact[] = Array.from({ length: 9 }, (_, index) => ({
name: `Fact ${index + 1}`,
state: `State ${index + 1}`,
detail1: `Test cell ${index + 1}-3`,
detail2: `Test cell ${index + 1}-4`,
detail3: `Test cell ${index + 1}-5`,
detail4: `Test cell ${index + 1}-6`,
detail5: `Test cell ${index + 1}-7`,
detail6: `Test cell ${index + 1}-8`,
detail7: `Test cell ${index + 1}-9`
}));

const columnNames = {
name: 'Fact',
state: 'State',
header3: 'Header 3',
header4: 'Header 4',
header5: 'Header 5',
header6: 'Header 6',
header7: 'Header 7',
header8: 'Header 8',
header9: 'Header 9'
};

return (
<div style={{ height: '400px' }}>
<InnerScrollContainer ref={scrollContainerRef}>
<Table
aria-label="Dynamic sticky header table"
gridBreakPoint=""
isStickyHeaderBase
isStickyHeaderStuck={isStuck}
>
<Thead>
<Tr>
<Th modifier="truncate">{columnNames.name}</Th>
<Th modifier="truncate">{columnNames.state}</Th>
<Th modifier="truncate">{columnNames.header3}</Th>
<Th modifier="truncate">{columnNames.header4}</Th>
<Th modifier="truncate">{columnNames.header5}</Th>
<Th modifier="truncate">{columnNames.header6}</Th>
<Th modifier="truncate">{columnNames.header7}</Th>
<Th modifier="truncate">{columnNames.header8}</Th>
<Th modifier="truncate">{columnNames.header9}</Th>
</Tr>
</Thead>
<Tbody>
{facts.map((fact) => (
<Tr key={fact.name}>
<Td modifier="nowrap" dataLabel={columnNames.name}>
{fact.name}
</Td>
<Td modifier="nowrap" dataLabel={columnNames.state}>
<BlueprintIcon />
{` ${fact.state}`}
</Td>
<Td modifier="nowrap" dataLabel={columnNames.header3}>
{fact.detail1}
</Td>
<Td modifier="nowrap" dataLabel={columnNames.header4}>
{fact.detail2}
</Td>
<Td modifier="nowrap" dataLabel={columnNames.header5}>
{fact.detail3}
</Td>
<Td modifier="nowrap" dataLabel={columnNames.header6}>
{fact.detail4}
</Td>
<Td modifier="nowrap" dataLabel={columnNames.header7}>
{fact.detail5}
</Td>
<Td modifier="nowrap" dataLabel={columnNames.header8}>
{fact.detail6}
</Td>
<Td modifier="nowrap" dataLabel={columnNames.header9}>
{fact.detail7}
</Td>
</Tr>
))}
</Tbody>
</Table>
</InnerScrollContainer>
</div>
);
};
Loading