-
Notifications
You must be signed in to change notification settings - Fork 392
feat(Table): add composable sticky footer #12645
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.
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
Open
gabipodolnikova
wants to merge
2
commits into
patternfly:main
Choose a base branch
from
gabipodolnikova:fix/glass-sticky-table-sections
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import { forwardRef } from 'react'; | ||
| import { css } from '@patternfly/react-styles'; | ||
| import styles from '@patternfly/react-styles/css/components/Table/table'; | ||
|
|
||
| export interface TfootProps extends React.HTMLProps<HTMLTableSectionElement> { | ||
| /** Content rendered inside the <tfoot> row group */ | ||
| children?: React.ReactNode; | ||
| /** Additional classes added to the <tfoot> element */ | ||
| className?: string; | ||
| /** @hide Forwarded ref */ | ||
| innerRef?: React.Ref<any>; | ||
| } | ||
|
|
||
| const TfootBase: React.FunctionComponent<TfootProps> = ({ children, className, innerRef, ...props }: TfootProps) => ( | ||
| <tfoot className={css(styles.tableTfoot, className)} ref={innerRef} {...props}> | ||
| {children} | ||
| </tfoot> | ||
| ); | ||
|
|
||
| export const Tfoot = forwardRef((props: TfootProps, ref: React.Ref<HTMLTableSectionElement>) => ( | ||
| <TfootBase {...props} innerRef={ref} /> | ||
| )); | ||
| Tfoot.displayName = 'Tfoot'; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
65 changes: 65 additions & 0 deletions
65
packages/react-table/src/components/Table/__tests__/Tfoot.test.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| import { render, screen } from '@testing-library/react'; | ||
| import { Tfoot } from '../Tfoot'; | ||
| import styles from '@patternfly/react-styles/css/components/Table/table'; | ||
|
|
||
| test('Renders without children', () => { | ||
| render( | ||
| <table> | ||
| <Tfoot /> | ||
| </table> | ||
| ); | ||
|
|
||
| expect(screen.getByRole('rowgroup')).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| test('Renders with children', () => { | ||
| render( | ||
| <table> | ||
| <Tfoot>Footer content</Tfoot> | ||
| </table> | ||
| ); | ||
|
|
||
| expect(screen.getByRole('rowgroup')).toHaveTextContent('Footer content'); | ||
| }); | ||
|
|
||
| test(`Renders with class ${styles.tableTfoot} only by default`, () => { | ||
| render( | ||
| <table> | ||
| <Tfoot /> | ||
| </table> | ||
| ); | ||
|
|
||
| expect(screen.getByRole('rowgroup')).toHaveClass(styles.tableTfoot, { exact: true }); | ||
| }); | ||
|
|
||
| test('Forwards refs to the tfoot element', () => { | ||
| const ref = { current: null } as React.RefObject<HTMLTableSectionElement>; | ||
|
|
||
| render( | ||
| <table> | ||
| <Tfoot ref={ref} /> | ||
| </table> | ||
| ); | ||
|
|
||
| expect(ref.current).toBe(screen.getByRole('rowgroup')); | ||
| }); | ||
|
|
||
| test('Renders with custom class names provided via prop', () => { | ||
| render( | ||
| <table> | ||
| <Tfoot className="custom-footer" /> | ||
| </table> | ||
| ); | ||
|
|
||
| expect(screen.getByRole('rowgroup')).toHaveClass('custom-footer'); | ||
| }); | ||
|
|
||
| test('Spreads additional props', () => { | ||
| render( | ||
| <table> | ||
| <Tfoot data-custom="true" /> | ||
| </table> | ||
| ); | ||
|
|
||
| expect(screen.getByRole('rowgroup')).toHaveAttribute('data-custom', 'true'); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
33 changes: 33 additions & 0 deletions
33
packages/react-table/src/components/Table/examples/TableStickyFooter.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| import { Table, Thead, Tfoot, Tr, Th, Tbody, Td, InnerScrollContainer } from '@patternfly/react-table'; | ||
|
|
||
| export const TableStickyFooter: React.FunctionComponent = () => { | ||
| const rows = Array.from({ length: 12 }, (_, index) => index + 1); | ||
|
|
||
| return ( | ||
| <div style={{ height: '400px' }}> | ||
| <InnerScrollContainer> | ||
| <Table aria-label="Sticky footer table" isStickyFooter> | ||
| <Thead> | ||
| <Tr> | ||
| <Th>Item</Th> | ||
| <Th>Value</Th> | ||
| </Tr> | ||
| </Thead> | ||
| <Tbody> | ||
| {rows.map((row) => ( | ||
| <Tr key={row}> | ||
| <Td dataLabel="Item">Item {row}</Td> | ||
| <Td dataLabel="Value">Value {row}</Td> | ||
| </Tr> | ||
| ))} | ||
| </Tbody> | ||
| <Tfoot> | ||
| <Tr> | ||
| <Td colSpan={2}>Total: {rows.length} items</Td> | ||
| </Tr> | ||
| </Tfoot> | ||
| </Table> | ||
| </InnerScrollContainer> | ||
| </div> | ||
| ); | ||
| }; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🤖 get_repo_knowledge executed:
get_repo_knowledge patternfly/patternfly-react /tmp/coderabbit-repo-knowledge/patternfly-patternfly-react-228bfeca/architecture /tmp/coderabbit-repo-knowledge/patternfly-patternfly-react-228bfeca/learningsLength of output: 47118
🏁 Script executed:
Repository: patternfly/patternfly-react
Length of output: 36396
🏁 Script executed:
Repository: patternfly/patternfly-react
Length of output: 31490
🏁 Script executed:
Repository: patternfly/patternfly-react
Length of output: 50384
🤖 get_repo_knowledge executed:
get_repo_knowledge patternfly/patternfly-react /tmp/coderabbit-repo-knowledge/patternfly-patternfly-react-228bfeca/architecture /tmp/coderabbit-repo-knowledge/patternfly-patternfly-react-228bfeca/learningsLength of output: 45784
🌐 Web query:
PatternFly table-grid.scss role grid unexpected interactions CSS grid table gridBreakPoint💡 Result:
In PatternFly, the table component utilizes a CSS grid-based responsive layout triggered by specific grid breakpoints (e.g.,
pf-m-grid-md,pf-m-grid-lg) [1][2][3]. When these breakpoints are reached, the table's internal display properties switch from a standard tabular layout to CSS grid [4][5]. The following key interactions and behaviors have been identified in technical documentation and issue reports: Unexpected Layout Interactions: When the table switches to its mobile/grid-based layout, table cells (Td) transition todisplay: grid[4]. This can cause immediate children of the cell to become new grid items, often leading to unintended stacking or elongation of cell content [4]. To mitigate this, PatternFly developers recommend wrapping the cell contents in a container element (such as adiv) [4]. Grid Breakpoint Behavior: Responsive behavior for tables has evolved from viewport-based media queries to container queries (usingpf-v6-contain-table) to allow for more flexible scoping [5][6]. Issues have historically occurred when tablegridBreakPointmodifiers interact with nested components—such as expandable rows—where the internal grid structure of the nested component conflicts with the parent table's grid-column definitions [7][8]. Role Attribute: Therole="grid"attribute on the table element has been noted to potentially impact accessibility interactions, specifically affecting cell focus behavior on mobile viewports, which is a known architectural choice within the component's implementation [9]. Visibility Breakpoints: Users may encounter conflicts between "stacked" table states and column visibility modifiers (e.g.,hiddenOnMd), where a column is hidden based on visibility rules while the layout simultaneously triggers a stacked view [10]. These issues are typically addressed by adjusting breakpoint ranges to ensure CSS rules do not overlap in conflicting ways [10]. For the most stable implementation, ensure that table cell content is wrapped in a dedicated container and verify that any custom components within table cells (like charts or expandable content) are compatible with the table's container query-based responsive breakpoints [4][7][5].Citations:
🏁 Script executed:
Repository: patternfly/patternfly-react
Length of output: 10812
🌐 Web query:
site:github.com/patternfly/patternfly table-grid.scss pf-m-grid-md thead display none role grid💡 Result:
In PatternFly’s Table “grid responsive” implementation, the modifier you asked about is pf-m-grid-md, and the table widget uses role="grid" on the table container. In the examples, the table element is rendered with table--IsGrid=true and table--modifier="pf-m-grid-md", and the documented required accessibility attributes include role="grid" and aria-label when needed.[1] However, the specific snippet you referenced (“table-grid.scss pf-m-grid-md thead display none role grid”)—i.e., an exact rule in src/patternfly/components/Table/table-grid.scss that sets thead { display: none } for pf-m-grid-md while also relating to role="grid”—is not present in the sources returned by this search. What I can confirm from the authoritative repo evidence is: - pf-m-grid-md is a responsive grid layout modifier for the table component.[1] - The table examples and accessibility documentation for the v6 table grid pattern require role="grid" on the table container.[1] - Separately, PatternFly changed the table’s markup/accessibility rules (e.g., thead cells should be , not ) as of v6.0.0-alpha.227—this is about semantic correctness, not hiding the thead.[2] - src/patternfly/components/Table/table-grid.scss was updated to use container queries for responsive behavior of grid variants (including md/lg/xl/2xl), which affects when layout changes occur, but the exact “thead display:none” behavior for pf-m-grid-md was not surfaced in the returned search results.[3][4] If you share the exact PatternFly version (v6.x.y) or the expected CSS selector block (e.g., the parent selector around thead), I can do a more targeted lookup for the precise “display:none” rule in table-grid.scss for pf-m-grid-md.
Citations:
🏁 Script executed:
Repository: patternfly/patternfly-react
Length of output: 2798
Preserve the non-responsive layout for this sticky-footer example.
When the table container reaches the
grid-mdbreakpoint, the default responsive modifier switches to CSS-grid layout and hides<thead>.Tablestill rendersrole="grid", so the grid loses its exposed header row.Restore
gridBreakPoint="".Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents
Source: MCP tools