Skip to content

fix(Alert): create alert group variables for toast animation#7305

Merged
mcoker merged 1 commit intopatternfly:mainfrom
sg00dwin:6812-alert-toast-animation
Feb 26, 2025
Merged

fix(Alert): create alert group variables for toast animation#7305
mcoker merged 1 commit intopatternfly:mainfrom
sg00dwin:6812-alert-toast-animation

Conversation

@sg00dwin
Copy link
Contributor

fixes #6812

@patternfly-build
Copy link
Collaborator

patternfly-build commented Jan 14, 2025

@mcoker mcoker requested review from mcoker and srambach January 15, 2025 01:54
@sg00dwin sg00dwin force-pushed the 6812-alert-toast-animation branch from 0ced7ff to fe7125b Compare January 24, 2025 17:22
Copy link
Member

@srambach srambach left a comment

Choose a reason for hiding this comment

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

I think I found a couple of little things keeping the default fade from working in reduced-motion.

I wonder if we could do away with some variables if we move the media query into the variables, but I'm not sure if it comes out any simpler or not, or if we'd want those variables for customization (my inclination is that customizing them is not necessary). Maybe it's worth chatting about since we are kind of breaking new ground using the reduced-motion preference. @mcoker

--#{$alert-group}--m-toast--ZIndex: var(--pf-t--global--z-index--2xl);

// Alert group item addition reduced motion
--#{$alert-group}--m-toast__item--TransitionDuration--opacity--default: var(--pf-t--global--motion--duration--fade--opacity--default);
Copy link
Member

Choose a reason for hiding this comment

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

I think this token should be --pf-t--global--motion--duration--fade--default, so then this variable isn't defined and that's ultimately causing the opacity change not to happen in reduced motion.

@sg00dwin sg00dwin force-pushed the 6812-alert-toast-animation branch 2 times, most recently from 62cece7 to 1bca732 Compare February 13, 2025 01:04
@sg00dwin
Copy link
Contributor Author

sg00dwin commented Feb 13, 2025

@srambach I've updated these changes, so that at reduced-motion, pf-m-offstage-right now correctly shows a transition to opacity: 0 then it will collapse with no slide. To prevent the alert from moving while it's fading out, I used timing delays on the grid-template-rows margin-block padding and border-width properties.

Here's a recording of it in real time and slowed down.

Copy link
Member

@srambach srambach left a comment

Choose a reason for hiding this comment

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

I was going to start #7341 to fix an issue we noticed in patternfly/patternfly-react#11495 but I realized it would be better to address here. Left 2 teensy changes that should fix the animation as intended. (i.e. on the transition in from the top, the item will not wait until the "hole" opens before sliding into the slot; it all happens at the same time)

@sg00dwin sg00dwin force-pushed the 6812-alert-toast-animation branch from 1bca732 to 8fca86e Compare February 17, 2025 22:19
@sg00dwin
Copy link
Contributor Author

Removed transition delays which addresses #7341

Copy link
Member

@srambach srambach left a comment

Choose a reason for hiding this comment

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

Hey, I noticed one more bug that would be good to fix here too if you don't mind - I think the pf-v6-c-alert-group__item needs to have a min-height: 0; Without it, you can see that there's still a bit of space when you take a toast alert offstage right.

2025-02-18_12-01-59.mp4

@sg00dwin sg00dwin force-pushed the 6812-alert-toast-animation branch from 8fca86e to 8901946 Compare February 20, 2025 15:28
@sg00dwin
Copy link
Contributor Author

@srambach Added the min-height: 0 to prevent the alert from adding space once it's removed.

pf-m-offstage-right.mp4

Copy link
Member

@srambach srambach left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you!

Copy link
Contributor

@mcoker mcoker left a comment

Choose a reason for hiding this comment

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

Sweet! Good job on these.

@mcoker mcoker merged commit f90244b into patternfly:main Feb 26, 2025
3 of 4 checks passed
@patternfly-build
Copy link
Collaborator

🎉 This PR is included in version 6.2.0-prerelease.12 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

Alert - add parameters for animation

4 participants