fix(dual-list-selector): animate tree expansion by mcoker · Pull Request #7493 · patternfly/patternfly

mcoker · 2025-04-30T01:21:08Z

patternfly-build · 2025-04-30T01:24:57Z

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

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

andrew-ronaldson

Crushing these issues bruh!

mcoker · 2025-05-09T19:32:14Z

@kmcfaul @thatblindgeye looks like dual list selector tree in react conditionally renders the nested items when they're expanded. I think that happens right here - https://github.com/patternfly/patternfly-react/blob/f8b21b594c90960305e3b58b723c1cff7a9decbb/packages/react-core/src/deprecated/components/DualListSelector/DualListSelectorTreeItem.tsx#L159

I think we'll need to either

To keep everything the way it is in the DOM now and continue to conditionally render
- On expand, render the nested tree as we do now, then use the setTimeout() method to apply .pf-m-expanded on the parent item.
- On collapse, wait for the transitionend event of the nested tree before unmounting
Let CSS manage hiding/showing the nested tree. That would probably mean the react component always renders the nested tree. Then the rest can be managed with CSS.

Do you have a preference? Or any other thoughts?

thatblindgeye · 2025-05-12T12:39:47Z

@mcoker gonna try fiddling around with React locally using this PR update, but at the moment I feel like personally I might lean towards making it more opt-in for React for now and updating in the next breaking change to be the default (similar to Alert).

Making it opt in means it won't be breaking for consumers to have the dual list selector content always rendered rather than dynamically rendered
Avoids any potential issues relying on the timeout to add that class to get animations working
Would be consistent with how the other expansion animations on ExpandableSection and Accordion are working right now

thatblindgeye · 2025-05-12T13:46:29Z

@mcoker @kmcfaul I think I have something locally that may not require a setTimeout but still work. There is an issue with nested tree view items in the DLS component, though, and seems like it's also in Core.

Toggling the pf-m-expanded class on a top most tree view item in the dual list selector with tree example animates fine (the "Colors" item). But trying to toggle an item nested in that one (e.g. "Green") doesn't have the animations I'd expect.

kmcfaul · 2025-05-12T15:35:39Z

I think I'd lean more towards a core solution with maybe an opt-in prop on react's side to allow the children to always be rendered, but I also think @thatblindgeye's solution avoiding the timeout and toggling the expanded class is good as well. The only issue I see is that the nested tree folders aren't being hidden when collapsed.

…et, collapse faster

mcoker · 2025-05-13T03:02:46Z

@kmcfaul @thatblindgeye fixed the nested item bug and converted it to transition display for the hide/show instead of relying on hidden

thatblindgeye

In React this looks great. The only thing is that by default (just the tree example as it curently is in React) the expand animation will still occur, but there's no collapse animation. Adding a hasAnimations prop and setting that up gets things working as expected, it's just the current behavior where the children are dynamically rendered rather than always rendered.

Would we be able to add a temporary class to disable animations for the current behavior, then in the next breaking change remove it (and the prop we'd need in React to get animations working)?

mcoker · 2025-05-13T15:22:44Z

@thatblindgeye forgot about that! Added, .pf-m-animate-expand that goes on the outer .pf-v6-c-dual-list-selector wrapper.

thatblindgeye

insert magneto-perfection.gif

Looks like this should work in React!

sg00dwin · 2025-05-15T13:19:12Z

src/patternfly/components/DualListSelector/dual-list-selector.scss

+      .#{$dual-list-selector}.pf-m-animate-expand & {
+        // stylelint-disable max-nesting-depth, selector-max-class
+        &:where([hidden]) {
+          display: flex;


I'm not following why this rule is needed. .#{$dual-list-selector}__list looks to be removed from the dom when in the unexpanded state.

@sg00dwin good catch, that was a copy/paste from another component.

sg00dwin · 2025-05-15T13:22:17Z

I'm not seeing it animate on collapse. Should it?

mcoker · 2025-05-19T18:39:06Z

@sg00dwin if you're testing in react, it won't collapse because react removes the nested menu from the DOM on collapse. But that's going to change so that react always renders the element - @thatblindgeye and I have tested it on the react side so it should be GTG now!

srambach

Just the one question about if we want to leave a breaking change comment for later.

srambach · 2025-05-19T20:30:58Z

src/patternfly/components/DualListSelector/dual-list-selector.scss

    --#{$dual-list-selector}__item--PaddingInlineStart: var(--#{$dual-list-selector}__item--m-expandable--PaddingInlineStart);
+
+    > .#{$dual-list-selector}__list {
+      .#{$dual-list-selector}.pf-m-animate-expand & {


Do we want to leave a TODO anywhere about removing the opt-in in a breaking change?

patternfly-build · 2025-05-20T21:50:49Z

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

The release is available on:

Your semantic-release bot 📦🚀

mcoker requested a review from andrew-ronaldson April 30, 2025 14:16

andrew-ronaldson approved these changes Apr 30, 2025

View reviewed changes

mcoker mentioned this pull request May 9, 2025

fix(tree-view): animate expansion #7495

Merged

mcoker added 4 commits May 12, 2025 21:34

fix(dual-list-selector): animate tree expansion

ab1d354

chore(dual-list-selector): reverse expand/collapse vars, shorten offs…

22f470f

…et, collapse faster

chore(dual-list-selector): remove unused var

c4a03e9

fix(dls): fix nested bug, animate display

f3af09a

mcoker force-pushed the issue-7325 branch from 4a6c642 to f3af09a Compare May 13, 2025 03:00

thatblindgeye reviewed May 13, 2025

View reviewed changes

chore(dls): add pf-m-animate-expand opt-in

43645ee

thatblindgeye approved these changes May 13, 2025

View reviewed changes

mcoker requested review from sg00dwin and srambach May 13, 2025 18:03

chore(dls): added example with animation

fa1e77a

sg00dwin reviewed May 15, 2025

View reviewed changes

mcoker added 3 commits May 15, 2025 21:38

chore(dls): transition visibility instead of display

2f1d661

fix(expandable-section): animate transition instead of display

4d013ed

chore(expandable-section): transition max-height instead of height

075c8c0

mcoker mentioned this pull request May 19, 2025

fix(expandable-section): animate transition instead of display #7531

Closed

mcoker requested a review from sg00dwin May 19, 2025 18:39

srambach approved these changes May 19, 2025

View reviewed changes

chore(dual-list-selector): todo to remove opt-in

d44f83a

mcoker requested a review from srambach May 20, 2025 20:32

mcoker merged commit 288e718 into patternfly:main May 20, 2025
4 checks passed

mcoker deleted the issue-7325 branch May 20, 2025 21:42

patternfly-build added the released on @prerelease label May 20, 2025

thatblindgeye mentioned this pull request May 21, 2025

DualListSelector/ExpandableSection - enable animations patternfly/patternfly-react#11836

Closed

Conversation

mcoker commented Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

andrew-ronaldson left a comment

Choose a reason for hiding this comment

Uh oh!

mcoker commented May 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thatblindgeye commented May 12, 2025

Uh oh!

thatblindgeye commented May 12, 2025

Uh oh!

kmcfaul commented May 12, 2025

Uh oh!

mcoker commented May 13, 2025

Uh oh!

thatblindgeye left a comment

Choose a reason for hiding this comment

Uh oh!

mcoker commented May 13, 2025

Uh oh!

thatblindgeye left a comment

Choose a reason for hiding this comment

Uh oh!

sg00dwin May 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcoker May 19, 2025

Choose a reason for hiding this comment

Uh oh!

sg00dwin commented May 15, 2025

Uh oh!

mcoker commented May 19, 2025

Uh oh!

srambach left a comment

Choose a reason for hiding this comment

Uh oh!

srambach May 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

patternfly-build commented May 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mcoker commented Apr 30, 2025 •

edited

Loading

patternfly-build commented Apr 30, 2025 •

edited

Loading

mcoker commented May 9, 2025 •

edited

Loading

sg00dwin May 15, 2025 •

edited

Loading