feat(tabs): animate background, current border by mcoker · Pull Request #7336 · patternfly/patternfly

mcoker · 2025-02-12T03:42:53Z

patternfly-build · 2025-02-12T03:45:25Z

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

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

andrew-ronaldson · 2025-03-04T13:35:00Z

This looks good to me. Is there ongoing work or good for review?

thatblindgeye

From a React perspective this sounds feasible. Looking into the existing code for Tabs stuff, I'm thinking we'd just leverage existing things for the most part:

we're already using componenDidUpdate to check if the children length changes (covers adding/removing tabs to recalculate the value of these vars)
We could use that ^ lifecycle to also check if the active tab changes and likewise recalculate the value of the vars
We may need to add some logic within the Tab component to check if its content changes and recalculate the vars as well

Preventing the animation on initial render may be tricky depending if the animation would occur immediately, but as discussed if some modifier class were to be added to determine when to animate then we should be able to calculate the initial var values on the first render, then add whatever class to "enable" animations.

kaylachumley

i like it!

andrew-ronaldson · 2025-03-05T18:50:47Z

Do we need a separate issue for vertical tabs?

andrew-ronaldson · 2025-03-06T16:19:54Z

I can see a little blue nubbin' on the tabs now and the blue underline seems to be thicker with this last update.

Screen.Recording.2025-03-06.at.12.17.18.PM.mov

kaylachumley · 2025-03-06T16:40:04Z

I am also seeing the same nubbin' as well

mcoker · 2025-03-06T16:51:26Z

@andrew-ronaldson @kaylachumley sorry about that, it's not quite ready for review 😅 Aside from the nubbin' action, does the vertical tabs animation look as intended?

andrew-ronaldson · 2025-03-06T16:54:03Z

Aside from the nubbin' action, does the vertical tabs animation look as intended?

Absofruitly!
Except for the line thickness. Otherwise legendary

srambach · 2025-03-07T21:45:08Z

It looks like the old border is still showing here before the sliding one comes in on the examples from Tab Item Actions down.

2025-03-07_16-40-07.mp4

srambach · 2025-03-07T21:53:10Z

+    }
+  }
+
+  .#{$tabs}.pf-m-no-animate-current {


I'm not sure about this name 😄 This turns off the slide on a case by case basis? Do we need this?

For the time being, this is the class we'll add to tabs that keeps the animation from happening on initial render. Just for documentation purposes, here were the options from our slack thread about it

There are probably a few ways we can do that:

React can keep the tabs from rendering until they set the vars for the current item, then render the tabs component. Eric said that might be tricky though.

Core can add a class like .pf-m-accent-no-animate that disabled the animation, React can add that, set the vars, then remove the class. That's easy.

I looked into another way of doing it. I think I can expose 2 sets of vars the react component could set - one set of vars animates and the other doesn't. React could set the vars that don't animate first, then on subsequent clicks, they set the vars that do animate

Feel free to suggest a better name! .pf-m-no-animate is too broad IMO since we may need a class like this for other animations, so I threw on "current", but I'm thinking .pf-m-no-animate-accent would be more appropriate. wdyt?

I'm curious what scenario you can think of where we'd want to stop one animation but not others. My instinct is to use a more generic name like .pf-m-no-animation or .pf-m-pause-animation or even the .pf-m-no-motion that we already use.

Most of our animations wouldn't need to be stopped like this because they either load with the initial component styles or after some sort of interaction or event. React needs to trigger the "current" animation logic manually (set the 2 vars for accent start and width) when the component mounts just to get the initial border style to render. None of our other animations require anything like that AFAIK.

That said, with a "no-motion" class (as in the user is opting out of the animation), if the goal is to not use the animation at all, I think that will need to do something different because if you simply disable the animation (duration set to 0 or whatever), you won't have an accent at all since the accent is created in a whole separate way depending on whether it's animated or not. That type of class will need to trigger the accent to be created on individual links instead of the list.

Tried to think of what's bothering me about this one...

the grammar 😛

tokens use "motion" rather than "animate" to be more generic

it's affecting accent variables so I think I'd prefer it to be named for that rather than current

it's purpose is to freeze during load/initialization so I'm wondering if it could be named somehow for that state rather than no-animate

WDYT about something like pf-m-initializing-accent ?

Love it! 🚀

mcoker · 2025-03-07T23:24:53Z

It looks like the old border is still showing here before the sliding one comes in on the examples from Tab Item Actions down.

@srambach d'oh, thanks! I forgot @sg00dwin mentioned that bug in one of our meetings. Fixed 👍

andrew-ronaldson · 2025-03-11T13:31:32Z

The blue line has disappeared! Thanos snapped away! Ahhhhh

mcoker · 2025-03-11T19:12:56Z

@andrew-ronaldson yeah 2 things:

I commented out the JS snippets I added to trigger the blue border on click. I don't think we want to include that in the core workspace because we don't want to convey that the borders need that snippet to work.
Because the borders now rely on react in order to show, they don't show in core. I'm not particularly happy about that, but I'm also not sure how to best address that.

However, we won't be able to merge this PR in core before code freeze, because without the react work being done, the border would disappear for everyone. So I think we have some time to sort out how we should make the borders appear in core, or if we leave them off (similar to how we don't show expanded menus in core since that's react functionality)

andrew-ronaldson · 2025-03-11T19:45:35Z

Could we have a selected tab in the examples to show the states or is it entirely in react?

srambach

But I liked "tubtab" 😉

Also, wasn't sure if you were ready to remove the commented out JS or not.

srambach · 2025-03-11T19:29:20Z

        {{/if}}
      }
    )()"
+  --}}


did you want to remove this yet?

srambach · 2025-03-11T19:42:52Z

+      border-inline-start-width: var(--#{$tabs}--link-accent--BorderInlineStartWidth);
+      transition:
+        --#{$tabs}--link-accent--length var(--#{$tabs}--link-accent--TransitionDuration--length) var(--#{$tabs}--link-accent--TransitionTimingFunction--length),
+        --#{$tabs}--link-accent--start var(--#{$tabs}--link-accent--TransitionDuration--start) var(--#{$tabs}--link-accent--TransitionTimingFunction--start),


Suggested change

--#{$tabs}--link-accent--start var(--#{$tabs}--link-accent--TransitionDuration--start) var(--#{$tabs}--link-accent--TransitionTimingFunction--start),

--#{$tabs}--link-accent--start var(--#{$tabs}--link-accent--TransitionDuration--start) var(--#{$tabs}--link-accent--TransitionTimingFunction--start)

I'm wondering about breaking this up rather than using the shorthand. It is a little harder to make sure the timings are lined up - but also, do we need to have separate timing function and duration variables, or could we have one for the entire effect - that we'd always want them to be the same since it's one motion.

srambach · 2025-03-11T19:51:10Z

  text-decoration-line: none;
  background-color: var(--#{$tabs}__link--BackgroundColor);
  border-radius: var(--#{$tabs}__link--BorderRadius);
+  transition: background-color var(--#{$tabs}__link--TransitionDuration--background-color) var(--#{$tabs}__link--TransitionTimingFunction--background-color);


Do you want to separate this rather than using the shorthand?

Doh! Yep - fixed.

Mar-12-2025.09-09-01.mov

srambach · 2025-03-12T13:56:32Z

+    --#{$tabs}--link-accent--TransitionDuration--length: 0;
+    --#{$tabs}--link-accent--TransitionDuration--start: 0;


Should these two lines now just be --#{$tabs}--link-accent--TransitionDuration: 0;?

srambach

L 📁 TM

kaylachumley

oops, missed this PR! I went to review but the animated tabs isnt working for me... maybe user error. I tried a new browser and nada! is the animated tabs working for you? @mcoker

…click

mcoker · 2025-03-18T18:13:15Z

@kaylachumley since the border depends on javascript I have to manually enable it in this PR, but will want to take it out prior to merging this PR since we don't include javascript like this in the core examples. I've re-enabled it so you can see it on all of the examples now - https://patternfly-pr-7336.surge.sh/components/tabs

kaylachumley · 2025-03-18T18:38:04Z

looks awesome

andrew-ronaldson

🤩

patternfly-build · 2025-04-09T00:00:10Z

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

The release is available on:

Your semantic-release bot 📦🚀

This reverts commit 356de76.

mcoker requested review from andrew-ronaldson, kaylachumley, sg00dwin, srambach and thatblindgeye March 4, 2025 19:21

thatblindgeye approved these changes Mar 4, 2025

View reviewed changes

mcoker force-pushed the issue-7250 branch from 7030469 to dfa1cb7 Compare March 5, 2025 01:25

kaylachumley approved these changes Mar 5, 2025

View reviewed changes

mcoker force-pushed the issue-7250 branch from 2cd04f2 to e102d0b Compare March 7, 2025 19:38

mcoker requested a review from kaylachumley March 7, 2025 19:46

srambach reviewed Mar 7, 2025

View reviewed changes

mcoker force-pushed the issue-7250 branch from 68808ba to 0438990 Compare March 10, 2025 20:52

mcoker requested a review from srambach March 11, 2025 00:05

mcoker force-pushed the issue-7250 branch from 0438990 to bbdb148 Compare March 11, 2025 04:15

srambach reviewed Mar 11, 2025

View reviewed changes

mcoker requested a review from srambach March 12, 2025 01:56

srambach reviewed Mar 12, 2025

View reviewed changes

mcoker requested a review from srambach March 13, 2025 03:01

srambach approved these changes Mar 13, 2025

View reviewed changes

kaylachumley reviewed Mar 18, 2025

View reviewed changes

mcoker added 10 commits March 18, 2025 13:02

feat(tabs): animate background, current border

0610a21

chore(tabs): add no-animate class for react, prevent default on link …

11cc59c

…click

fix(tabs): add support for vertical tabs

99df727

chore(tabs): remove comments

d198b50

fix(tabs): fix nubble and width

5a07068

fix(tabs): fix border animation with tab actions

be9454a

chore(tabs): modifier name change, docs

3cfa414

chore(tabs): shorthand to longhand

cee21d2

chore(tabs): fix var

0451526

chore(tabs): enable click event styling

1121752

mcoker force-pushed the issue-7250 branch from 1a73fb5 to 1121752 Compare March 18, 2025 18:03

andrew-ronaldson approved these changes Mar 27, 2025

View reviewed changes

mcoker mentioned this pull request Apr 4, 2025

Tabs - animation and core styles, performance #7437

Closed

mcoker merged commit 8bfab8d into patternfly:main Apr 8, 2025

mcoker deleted the issue-7250 branch April 8, 2025 23:50

patternfly-build added the released on @prerelease label Apr 9, 2025

dlabaj pushed a commit that referenced this pull request Apr 16, 2025

feat(tabs): animate background, current border (#7336)

356de76

patternfly-build mentioned this pull request Apr 16, 2025

The automated release is failing 🚨 #7464

Closed

dlabaj added a commit that referenced this pull request Apr 22, 2025

revert "feat(tabs): animate background, current border (#7336)"

0c0997a

This reverts commit 356de76.

This was referenced Jul 22, 2026

The automated release is failing 🚨 #8519

Closed

The automated release is failing 🚨 #8533

Closed

	--#{$tabs}--link-accent--start var(--#{$tabs}--link-accent--TransitionDuration--start) var(--#{$tabs}--link-accent--TransitionTimingFunction--start),
	--#{$tabs}--link-accent--start var(--#{$tabs}--link-accent--TransitionDuration--start) var(--#{$tabs}--link-accent--TransitionTimingFunction--start)

		--#{$tabs}--link-accent--TransitionDuration--length: 0;
		--#{$tabs}--link-accent--TransitionDuration--start: 0;

Conversation

mcoker commented Feb 12, 2025

Uh oh!

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

Uh oh!

Uh oh!

andrew-ronaldson commented Mar 4, 2025

Uh oh!

thatblindgeye left a comment

Choose a reason for hiding this comment

Uh oh!

kaylachumley left a comment

Choose a reason for hiding this comment

Uh oh!

andrew-ronaldson commented Mar 5, 2025

Uh oh!

andrew-ronaldson commented Mar 6, 2025

Uh oh!

kaylachumley commented Mar 6, 2025

Uh oh!

mcoker commented Mar 6, 2025

Uh oh!

andrew-ronaldson commented Mar 6, 2025

Uh oh!

srambach commented Mar 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcoker commented Mar 7, 2025

Uh oh!

andrew-ronaldson commented Mar 11, 2025

Uh oh!

mcoker commented Mar 11, 2025

Uh oh!

andrew-ronaldson commented Mar 11, 2025

Uh oh!

srambach left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

srambach left a comment

Choose a reason for hiding this comment

Uh oh!

kaylachumley left a comment

Choose a reason for hiding this comment

Uh oh!

mcoker commented Mar 18, 2025

Uh oh!

kaylachumley commented Mar 18, 2025

Uh oh!

andrew-ronaldson left a comment

Choose a reason for hiding this comment

Uh oh!

patternfly-build commented Apr 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

patternfly-build commented Feb 12, 2025 •

edited

Loading