Skip to content
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

feat(Modal): add pop from center/anchor animations #2489

Conversation

YossiSaadi
Copy link
Contributor

@YossiSaadi YossiSaadi commented Oct 10, 2024

@YossiSaadi YossiSaadi force-pushed the feat/yossi/new-modal-building-blocks-7359960492 branch from 153d94f to 23b868f Compare October 14, 2024 10:30
@YossiSaadi YossiSaadi force-pushed the feat/yossi/new-modal-modal-animation-add-animation-for-modal-opening-7586333722 branch from 2db54e9 to 9acf136 Compare October 20, 2024 08:47
@YossiSaadi YossiSaadi changed the title feat(Modal): add pop from center animation feat(Modal): add pop from center/anchor animations Oct 20, 2024
@YossiSaadi YossiSaadi force-pushed the feat/yossi/new-modal-modal-animation-add-animation-for-modal-opening-7586333722 branch from ba0fb26 to b4c0e00 Compare October 20, 2024 10:36
@YossiSaadi YossiSaadi marked this pull request as ready for review October 20, 2024 10:36
@YossiSaadi YossiSaadi requested a review from a team as a code owner October 20, 2024 10:36
opacity: [0, 1, 1],
scale: [0.65, 0.65, 1],
top: "50%",
left: "50%",
Copy link
Contributor

Choose a reason for hiding this comment

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

Are the top & left needed here as they are defined already in the css?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, the css determines their first positioning and then I need to reflect it also here, otherwise its positioning is messed up

initial="exit"
animate="enter"
exit="exit"
custom={anchorElementRef}
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need a fallback in case it is undefined, so it won't mess up the animation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

checked it, if it is undefined then the pop from center variation would be applied and it doesn't use it
if it is being used from custom, then the pop from anchor variation would be applied, and it means that the anchor element ref isn't undefined

Copy link
Contributor

@rivka-ungar rivka-ungar left a comment

Choose a reason for hiding this comment

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

Very cool! Left 2 small questions

@YossiSaadi YossiSaadi merged commit 9b53547 into feat/yossi/new-modal-building-blocks-7359960492 Oct 30, 2024
9 of 10 checks passed
@YossiSaadi YossiSaadi deleted the feat/yossi/new-modal-modal-animation-add-animation-for-modal-opening-7586333722 branch October 30, 2024 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants