Skip to content

feat(Tabs): added animations #11767

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 9 commits into
base: main
Choose a base branch
from

Conversation

thatblindgeye
Copy link
Contributor

@thatblindgeye thatblindgeye commented Apr 16, 2025

What: Closes #11348

Looks like with the update to Tabs styling, we'll have to make updates in Org for the tabs being used for component pages. If any consumers are using tabs classes similar to how we are in Org (applying the tabs classes to elements rather than using the Tabs components themselves), they will probably run into a similar issue.

I updated and skipped some tests for now. Old tests didn't seem relevant with the unit tests for OverflowTab, and 1 test relies on a potential Core fix and the other was just acting up no matter what I had tried. Basically if there were 2 tests interacting with the OverflowTab, the 1st test would fail to find an item within that overflow menu while the 2nd test worked fine (using the same exact selectors/assertions). Remove the 1st test and suddenly the 2nd (now the 1st) would fail to find the specified overflow item.

Additional issues:

@patternfly-build
Copy link
Contributor

patternfly-build commented Apr 16, 2025

@thatblindgeye thatblindgeye requested review from kaylachumley, a team, rebeccaalpert, wise-king-sullyman and srambach and removed request for a team April 17, 2025 14:49
@thatblindgeye thatblindgeye force-pushed the iss11348_tabsAnimation branch 2 times, most recently from c9a3c66 to 0c323e3 Compare April 17, 2025 17:42
Copy link
Member

@rebeccaalpert rebeccaalpert left a comment

Choose a reason for hiding this comment

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

This looked good to me. Animations look awesome.

@thatblindgeye thatblindgeye force-pushed the iss11348_tabsAnimation branch 3 times, most recently from 0cbe199 to 7fc661c Compare April 18, 2025 15:04
Copy link

@kaylachumley kaylachumley left a comment

Choose a reason for hiding this comment

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

looks awesome! ty

@thatblindgeye thatblindgeye force-pushed the iss11348_tabsAnimation branch 2 times, most recently from 5b70362 to c10341f Compare April 21, 2025 15:13
Copy link
Contributor

@kmcfaul kmcfaul left a comment

Choose a reason for hiding this comment

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

lgtm

@kmcfaul
Copy link
Contributor

kmcfaul commented Apr 22, 2025

I do think we should add support for html-only tabs (disabling the animation via a class or setting automatic widths on those two style css), but I don't think we need to block this PR for that. We can do any adjustments in a follow up.

@srambach
Copy link
Member

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Animation: Tabs - Apply new tab states (React)
6 participants