-
Notifications
You must be signed in to change notification settings - Fork 130
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
Fix the NotificationToast transition issues #1355
Conversation
Previously, the JSdoc comment wasn't properly associated with the component.
🦋 Changeset detectedLatest commit: 5f692ac The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Hey @amelako, Thanks! |
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/sumup/oss-circuit-ui/7JDSH1hErW1N8rT8kc8bHeF3MEjF |
Codecov Report
@@ Coverage Diff @@
## main #1355 +/- ##
==========================================
- Coverage 92.24% 92.22% -0.02%
==========================================
Files 189 190 +1
Lines 3776 3780 +4
Branches 1175 1175
==========================================
+ Hits 3483 3486 +3
- Misses 275 276 +1
Partials 18 18
|
Co-authored-by: Connor Bär <connor-baer@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noice 🐛
border-radius: ${theme.borderRadius.byte}; | ||
border: ${theme.borderWidth.mega} solid ${theme.colors[colorMap[variant]]}; | ||
overflow: hidden; | ||
transition: opacity 200ms ease-in-out, height 200ms ease-in-out, | ||
visibility 200ms ease-in-out; | ||
will-change: height; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you notice a change when adding this? Curious
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really, I think this would only be noticeable on older hardware.
Purpose
Regarding the transitions when the toast appears, its a bit hard and jumps a bit in its height. There was an issue with disappear transitions too, where the toast is animated to half the height and then fades out.
Approach and changes
To fix the issues following changes were done:
div
around the toast content to fix the height transition issue. The padding needs to be on the innerdiv
.Definition of done