Skip to content

fix(Table): removed divider for expanded rows #11815

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
wants to merge 2 commits into
base: main
Choose a base branch
from
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
2 changes: 1 addition & 1 deletion packages/react-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
"tslib": "^2.8.1"
},
"devDependencies": {
"@patternfly/patternfly": "6.3.0-prerelease.6",
"@patternfly/patternfly": "6.3.0-prerelease.15",
"case-anything": "^3.1.2",
"css": "^3.0.0",
"fs-extra": "^11.3.0"
Expand Down
2 changes: 1 addition & 1 deletion packages/react-docs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"test:a11y": "patternfly-a11y --config patternfly-a11y.config"
},
"dependencies": {
"@patternfly/patternfly": "6.3.0-prerelease.6",
"@patternfly/patternfly": "6.3.0-prerelease.15",
"@patternfly/react-charts": "workspace:^",
"@patternfly/react-code-editor": "workspace:^",
"@patternfly/react-core": "workspace:^",
Expand Down
2 changes: 1 addition & 1 deletion packages/react-icons/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"@fortawesome/free-brands-svg-icons": "^5.15.4",
"@fortawesome/free-regular-svg-icons": "^5.15.4",
"@fortawesome/free-solid-svg-icons": "^5.15.4",
"@patternfly/patternfly": "6.3.0-prerelease.6",
"@patternfly/patternfly": "6.3.0-prerelease.15",
"fs-extra": "^11.3.0",
"tslib": "^2.8.1"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/react-styles/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"clean": "rimraf dist css"
},
"devDependencies": {
"@patternfly/patternfly": "6.3.0-prerelease.6",
"@patternfly/patternfly": "6.3.0-prerelease.15",
"change-case": "^5.4.4",
"fs-extra": "^11.3.0"
},
Expand Down
9 changes: 6 additions & 3 deletions packages/react-table/src/components/Table/Tr.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@ export interface TrProps extends Omit<React.HTMLProps<HTMLTableRowElement>, 'onR
innerRef?: React.Ref<any>;
/** Flag indicating the Tr is hidden */
isHidden?: boolean;
/** Only applicable to Tr within the Tbody: Makes the row expandable and determines if it's expanded or not.
/** Only applicable to Tr within the Tbody and determines if the expandable row content is expanded or not.
* To prevent column widths from responding automatically when expandable rows are toggled, the width prop must also be passed into either the th or td component
*/
isExpanded?: boolean;
/** Flag to indicate that a row is expandable. Only applicable to a tr that is intended to collapse or expand. */
isExpandable?: boolean;
/** Only applicable to Tr within the Tbody: Whether the row is editable */
isEditable?: boolean;
/** Flag which adds hover styles for the clickable table row */
Expand Down Expand Up @@ -46,6 +48,7 @@ const TrBase: React.FunctionComponent<TrProps> = ({
children,
className,
isExpanded,
isExpandable,
isEditable,
isHidden = false,
isClickable = false,
Expand Down Expand Up @@ -75,7 +78,7 @@ const TrBase: React.FunctionComponent<TrProps> = ({
};
}

const rowIsHidden = isHidden || (isExpanded !== undefined && !isExpanded);
const rowIsHidden = isHidden || (isExpanded !== undefined && !isExpanded && isExpandable);

const { registerSelectableRow } = useContext(TableContext);

Expand All @@ -96,7 +99,7 @@ const TrBase: React.FunctionComponent<TrProps> = ({
className={css(
styles.tableTr,
className,
isExpanded !== undefined && styles.tableExpandableRow,
isExpandable !== undefined && styles.tableExpandableRow,
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this break people who were relying on the isExpanded prop do apply the expandable row styles?

Copy link
Contributor

Choose a reason for hiding this comment

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

isExpanded still controls the expanded modifier so if anyone is detecting expansion based on a class presence, I think they should be testing for pf-m-expanded.

The issue is that the current block of logic isn't applying tableExpandableRow when a row is collapsed (because isExpanded = false is falsy), and it needs the class to be present on any row that can expand regardless of the expanded state. After thinking about it more, I think it might be possible to update the logic here to (isExpanded == true || isExpanded == false) && styles.tableExpandableRow though, to avoid needing the new modifier?

isExpanded && styles.modifiers.expanded,
isEditable && inlineStyles.modifiers.inlineEditable,
isClickable && styles.modifiers.clickable,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ exports[`RowWrapper renders expanded correctly 1`] = `
<table>
<tbody>
<tr
class="pf-v6-c-table__tr pf-v6-c-table__expandable-row pf-m-expanded"
class="pf-v6-c-table__tr pf-m-expanded"
data-ouia-component-id="OUIA-Generated-TableRow-2"
data-ouia-component-type="PF6/TableRow"
data-ouia-safe="true"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export const TableExpandable: React.FunctionComponent = () => {
id="toggle-compact"
name="toggle-compact"
/>
<Table aria-label="Expandable table" variant={isExampleCompact ? 'compact' : undefined}>
<Table isExpandable aria-label="Expandable table" variant={isExampleCompact ? 'compact' : undefined}>
<Thead>
<Tr>
<Th screenReaderText="Row expansion" />
Expand Down Expand Up @@ -152,7 +152,7 @@ export const TableExpandable: React.FunctionComponent = () => {
}
return (
<Tbody key={repo.name} isExpanded={isRepoExpanded(repo)}>
<Tr>
<Tr isExpanded={isRepoExpanded(repo)}>
<Td
expand={
repo.details
Expand All @@ -172,7 +172,7 @@ export const TableExpandable: React.FunctionComponent = () => {
<Td dataLabel={columnNames.lastCommit}>{repo.lastCommit}</Td>
</Tr>
{repo.details ? (
<Tr isExpanded={isRepoExpanded(repo)}>
<Tr isExpandable isExpanded={isRepoExpanded(repo)}>
{!childIsFullWidth ? <Td /> : null}
{repo.details.detail1 ? (
<Td dataLabel="Repo detail 1" noPadding={childHasNoPadding} colSpan={detail1Colspan}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ export const TableExpandable: React.FunctionComponent = () => {
];

return (
<Table aria-label="Simple table">
<Table isExpandable aria-label="Simple table">
<Thead>
<Tr>
<Th screenReaderText="Row expansion" />
Expand All @@ -141,7 +141,7 @@ export const TableExpandable: React.FunctionComponent = () => {
</Thead>
{repositories.map((repo, rowIndex) => (
<Tbody key={repo.name} isExpanded={isRepoExpanded(repo)}>
<Tr>
<Tr isExpanded={isRepoExpanded(repo)}>
<Td
expand={
repo.nestedComponent
Expand All @@ -163,7 +163,7 @@ export const TableExpandable: React.FunctionComponent = () => {
</Td>
</Tr>
{repo.nestedComponent ? (
<Tr isExpanded={isRepoExpanded(repo)}>
<Tr isExpandable isExpanded={isRepoExpanded(repo)}>
<Td
noPadding={repo.noPadding}
dataLabel={`${columnNames.name} expended`}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ export const TableExpandCollapseAll: React.FunctionComponent = () => {
aria-label="Collapsible table data"
>
<Card component="div">
<Table aria-label="Collapsible table">
<Table isExpandable aria-label="Collapsible table">
<Thead>
<Tr>
<Th
Expand All @@ -175,7 +175,7 @@ export const TableExpandCollapseAll: React.FunctionComponent = () => {

{serverData.map((server, serverIndex) => (
<Tbody key={server.name} isExpanded={isServerExpanded(server)}>
<Tr>
<Tr isExpanded={isServerExpanded(server)}>
<Td
expand={
server.details
Expand All @@ -195,7 +195,7 @@ export const TableExpandCollapseAll: React.FunctionComponent = () => {
<Td>{server?.workspaces}</Td>
<Td>{server?.status?.title}</Td>
</Tr>
<Tr isExpanded={isServerExpanded(server)}>
<Tr isExpandable isExpanded={isServerExpanded(server)}>
<Td></Td>
<Td colSpan={expandableColumns.length}>
<ExpandableRowContent>{server?.details}</ExpandableRowContent>
Expand Down
Loading
Loading