fix(table): updated expandable to animate visibility by mcoker · Pull Request #7537 · patternfly/patternfly

mcoker · 2025-05-20T02:47:08Z

patternfly-build · 2025-05-20T02:51:17Z

Preview: https://patternfly-pr-7537.surge.sh

A11y report: https://patternfly-pr-7537-a11y.surge.sh

thatblindgeye

Tested this against the React PR that resolves the divider issue patternfly/patternfly-react#11815 and it mostly looks good. The one thing I'm noticing is when you click to collapse a table row, the divider line appears as the expandable row is collapsing (causing the table to look like it has an empty row momentarily). I think it's really only noticeable at 25 or 10% animation speed, though.

My mac isn't currently letting my screen record so pictures instead:

srambach · 2025-05-20T18:22:01Z

Tested this against the React PR that resolves the divider issue patternfly/patternfly-react#11815 and it mostly looks good. The one thing I'm noticing is when you click to collapse a table row, the divider line appears as the expandable row is collapsing (causing the table to look like it has an empty row momentarily). I think it's really only noticeable at 25 or 10% animation speed, though.

Does it have to do with the border-block-end: revert on line 1051 and/or not everything is getting marked as expanded as it should?
There are 3 elements with .pf-m-expanded and if you toggle the table__tr that's not the expandable row, it creates an extra border

srambach · 2025-05-20T19:30:55Z

There's a problem with the compound expansion example - looks like maybe changing them to visibility instead of display:none means they are taking space.

thatblindgeye

Other than ^ lgtm, animations work nicely when pulled into React (with the fixes for the divider bug that got merged in react)

mcoker · 2025-05-21T21:59:47Z

@srambach can you check again?

I'm assuming we should animate compound expandable, too? At least the initial click on a compound expandable item, but also possibly a fade between items once one is expanded when clicking on a different item. cc @andrew-ronaldson wdyt? Here's a link to compound expandable in react - https://www.patternfly.org/components/table/react/compound-expandable/

sg00dwin

LGTM

…ntent

srambach

It looks like compound expandable shows all the non-expanded rows unless .pf-m-animate-expand is on the table. Are we keeping display: none in that case so we don't break core users?

Also, I think we need to add .pf-m-animate-expand in the docs at the appropriate spot.

srambach · 2025-06-10T00:39:04Z

src/patternfly/components/Table/examples/Table.md

 | `.pf-v6-c-table__control-row` | `.pf-v6-c-table__expandable > <tr>` | Modifies a compound expandable table control row. |
-| `.pf-m-expanded` | `<tbody>`, `.pf-v6-c-table__compound-expansion-toggle` > `.pf-v6-c-button` | Modifies a tbody with a row and an expandable row. |
 | `.pf-v6-c-table__compound-expansion-toggle` | `<td>` | Modifies a `<td>` on active/focus. |
+| `.pf-m-expanded` | `<tbody>`, `.pf-v6-c-table__compound-expansion-toggle` > `.pf-v6-c-button` | Modifies a tbody with a row and an expandable row. |


This wording is confusing - modifies a tbody that contains ... and an expanded toggle (?)

That was already there (I didn't add it) but I took a stab. It looks like the selectors were wrong, too.

srambach · 2025-06-10T00:39:09Z

src/patternfly/components/Table/examples/Table.md

-| `.pf-m-expanded` | `<tbody>`, `.pf-v6-c-table__compound-expansion-toggle` > `.pf-v6-c-button` | Modifies a tbody with a row and an expandable row. |
 | `.pf-v6-c-table__compound-expansion-toggle` | `<td>` | Modifies a `<td>` on active/focus. |
+| `.pf-m-expanded` | `<tbody>`, `.pf-v6-c-table__compound-expansion-toggle` > `.pf-v6-c-button` | Modifies a tbody with a row and an expandable row. |
+| `.pf-m-no-background` | `.pf-v6-c-table__expandable-row-content` | Modifies the expandable row content to have a transparent background. For in compound expandable when the parent expandable row has no padding with `.pf-m-no-padding`. |


Suggested change

| `.pf-m-no-background` | `.pf-v6-c-table__expandable-row-content` | Modifies the expandable row content to have a transparent background. For in compound expandable when the parent expandable row has no padding with `.pf-m-no-padding`. |

| `.pf-m-no-background` | `.pf-v6-c-table__expandable-row-content` | Modifies the expandable row content to have a transparent background. For use in compound expandable when the parent expandable row has no padding with `.pf-m-no-padding`. |

srambach · 2025-06-10T00:59:36Z

src/patternfly/components/Table/table.scss

-  &:not(.pf-m-expanded) {
-    display: none;


Does this need to remain or use another method to hide rows that aren't expanded?

Added back, missed that completely. That hides the non-expanded rows in a non-expandable table.

mcoker · 2025-06-10T02:14:45Z

@srambach thanks! I should have caught the non-animated rows showing!

Updated and added new examples with the animated class for regular and compound expandable.

thatblindgeye

Repeat from Slack convo below, but a couple things I'm noticing when pulling these updates into React with updates made there:

In Core we need to apply the pf-m-animate-expand class to the outer table element. Doing that will show an issue seen in React where when anything but the 1st compound expandable section is expanded, there's a border above the expandable content. I assume it's due to some tr + tr selector not taking into account this animate-expand modifier? The core compound expandable looks fine when updating the classes so the second compound expansion button+content is expanded, it's not until you add the missing pf-m-animate-expand to the outer table where you can reproduce the issue
If content within the pf-v6-c-table__expandable-row-content element has a margin (as it does in React examples/demos for compound expansion), that margin continues to take up space when collapsed. Updating the React examples/demos to use the padding utility class instead of the margin one should resolve the issue, but just wondering if we want to handle a case of margin being used.

Not from Slack, but also examples will need to be updates so that hidden and inert are added to the collapsed rows.

srambach · 2025-06-11T14:28:44Z

I think now the top padding is missing on tabs that aren't the first one? You can see in the inspector that padding-block-start is 0 if it's not immediately following a control row

2025-06-11_10-24-34.mp4

thatblindgeye

Looks good when linked to my React draft!

srambach

I'm good with a follow-on later to see if we can improve the bounce when switching from one compound expandable to another.

patternfly-build · 2025-06-11T17:11:43Z

🎉 This PR is included in version 6.3.0-prerelease.28 🎉

The release is available on:

Your semantic-release bot 📦🚀

…fly#7537)" This reverts commit a8e2447.

mcoker requested review from sg00dwin, srambach and thatblindgeye May 20, 2025 14:26

thatblindgeye reviewed May 20, 2025

View reviewed changes

thatblindgeye approved these changes May 21, 2025

View reviewed changes

mcoker force-pushed the issue-7536 branch from 7091946 to 898d5da Compare May 21, 2025 21:20

mcoker requested a review from thatblindgeye May 21, 2025 21:57

sg00dwin approved these changes May 22, 2025

View reviewed changes

kmcfaul mentioned this pull request Jun 4, 2025

Table - expandable section animation support patternfly/patternfly-react#11856

Closed

mcoker added 2 commits June 5, 2025 14:30

fix(table): updated expandable to animate visibility

a3d99ac

chore(table): fix hidden issue

0abbba1

mcoker force-pushed the issue-7536 branch from adfeb3b to 0abbba1 Compare June 5, 2025 19:31

mcoker added 4 commits June 5, 2025 14:54

fix(table): make sure all expandable tables are wrapped in exp-row-co…

388de1e

…ntent

fix(table): fix compound expandable examples

3addc59

fix(table): remove comp exp control row expanded border per drewski

fb7b51b

chore(table): move animations to opt-in

014a126

kmcfaul approved these changes Jun 9, 2025

View reviewed changes

chore(motion): bump max-height for all components

a4fc264

srambach reviewed Jun 10, 2025

View reviewed changes

mcoker added 2 commits June 9, 2025 20:47

chore(table): hide collapsed rows for non-exp table

e369f02

chore(table): docs update, css fix

c9c3f35

mcoker requested a review from srambach June 10, 2025 02:13

thatblindgeye requested changes Jun 10, 2025

View reviewed changes

mcoker added 2 commits June 10, 2025 10:32

fix(table): fix compound expandable border

30097be

fix(table): hide collapsed row overflow

15859b9

fix(table): fix border issue

156004e

thatblindgeye mentioned this pull request Jun 11, 2025

feat(Table): added optin animations for expansion patternfly/patternfly-react#11865

Merged

fix(table): fix expandable top padding

50c8554

thatblindgeye approved these changes Jun 11, 2025

View reviewed changes

srambach approved these changes Jun 11, 2025

View reviewed changes

mcoker merged commit a8e2447 into patternfly:main Jun 11, 2025
4 checks passed

mcoker deleted the issue-7536 branch June 11, 2025 17:02

patternfly-build added the released on @prerelease label Jun 11, 2025

mcoker added a commit to mcoker/patternfly that referenced this pull request Jul 16, 2025

Revert "fix(table): updated expandable to animate visibility (pattern…

2749fb3

…fly#7537)" This reverts commit a8e2447.

mcoker mentioned this pull request Jul 16, 2025

fix(table): fix animation style conflicts #7663

Merged

	\| `.pf-m-no-background` \| `.pf-v6-c-table__expandable-row-content` \| Modifies the expandable row content to have a transparent background. For in compound expandable when the parent expandable row has no padding with `.pf-m-no-padding`. \|
	\| `.pf-m-no-background` \| `.pf-v6-c-table__expandable-row-content` \| Modifies the expandable row content to have a transparent background. For use in compound expandable when the parent expandable row has no padding with `.pf-m-no-padding`. \|

Conversation

mcoker commented May 20, 2025

Uh oh!

patternfly-build commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thatblindgeye left a comment

Choose a reason for hiding this comment

Uh oh!

srambach commented May 20, 2025

Uh oh!

srambach commented May 20, 2025

Uh oh!

thatblindgeye left a comment

Choose a reason for hiding this comment

Uh oh!

mcoker commented May 21, 2025

Uh oh!

sg00dwin left a comment

Choose a reason for hiding this comment

Uh oh!

srambach left a comment

Choose a reason for hiding this comment

Uh oh!

srambach Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

mcoker Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

srambach Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

srambach Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

mcoker Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

mcoker commented Jun 10, 2025

Uh oh!

thatblindgeye left a comment

Choose a reason for hiding this comment

Uh oh!

srambach commented Jun 11, 2025

Uh oh!

thatblindgeye left a comment

Choose a reason for hiding this comment

Uh oh!

srambach left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

patternfly-build commented Jun 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

patternfly-build commented May 20, 2025 •

edited

Loading