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.

Sign up for GitHub

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

Jump to bottom

[MS] Fixes animation for toast progress bar #6165

Merged
merged 1 commit into from
Jan 16, 2024
Merged

[MS] Fixes animation for toast progress bar #6165

merged 1 commit into from
Jan 16, 2024

Conversation

Max-7
Copy link
Contributor

@Max-7 Max-7 commented Jan 15, 2024

Closes #6163

It would probably be easier if toast where our own component, we could probably just bind the duration with Vue. Sadly, they're created by the toastController.

The idea behind this fix is to assigned a unique id to each toast, and dynamically add a <style> tag in the <head> of the page that contains the CSS with the appropriate duration. The style is removed once the toast is closed.

  • Keep changes in the pull request as small as possible
  • Ensure the commit history is sanitized
  • Give a meaningful title to your PR
  • Describe your changes
  • Link any related issue in the description

@NicoTuxx
Copy link
Contributor

Maybe use a ref instead of IDs? (you can look at how we modify css values in the sidebarMenu component for an example)

@Max-7 Max-7 force-pushed the ms-fix-toast-progress-duration branch from c264c17 to e8ca124 Compare January 16, 2024 09:37
@Max-7
Copy link
Contributor Author

Max-7 commented Jan 16, 2024 •
edited
Loading

So...
Tried to get the toast variable and add style directly to it to avoid adding/removing the style in the header. But since it's a shadow tree, it seems that I cannot update the element style because it's a ::after selector (mix of shadow tree being hard to access + read only + CSS only selector).

`#${toastId} {
  &.active {
    &::part(container) {
      &::after {
        width: 0%;
        transition: width ${duration}s ease-in-out;
      }
    }
  }
}`;

this does get applied to the toast but for some reason the transition doesn't work. If you remove the width: 0%; (or add a background color or whatever) it does change the toast, which shows that the style is indeed applied, but the transition does nothing.

@Max-7 Max-7 force-pushed the ms-fix-toast-progress-duration branch from e8ca124 to daa2442 Compare January 16, 2024 10:26
@Max-7 Max-7 marked this pull request as ready for review January 16, 2024 10:26
@Max-7 Max-7 requested a review from a team as a code owner January 16, 2024 10:26
@Max-7
Copy link
Contributor Author

Max-7 commented Jan 16, 2024

Instead of all the black magic, I've instead added a ms-toast-duration variable in CSS. The caveat is that we can only ever show one toast at a time because the variable will apply to all toasts, but it does work correctly.

I've also fixed a bug where the animation of a toast would play at its creation instead of when it's shown.

@Max-7 Max-7 force-pushed the ms-fix-toast-progress-duration branch from daa2442 to 352c16e Compare January 16, 2024 10:28
fabienSvtr
fabienSvtr requested changes Jan 16, 2024
View reviewed changes
client/src/theme/components/toasts.scss Outdated
@@ -85,7 +85,9 @@ ion-toast.notification-toast {
&::part(container) {
&::after {
width: 0%;
transition: width 5s ease-in-out;
transition-duration: var(--ms-toast-duration);
Copy link

@fabienSvtr fabienSvtr Jan 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

You can keep the condensed property:
transition: width var(--ms-toast-duration) ease-in-out;

@Max-7 Max-7 force-pushed the ms-fix-toast-progress-duration branch from 352c16e to 3a93660 Compare January 16, 2024 10:45
@Max-7 Max-7 force-pushed the ms-fix-toast-progress-duration branch from 3a93660 to f1b09c9 Compare January 16, 2024 10:45
@Max-7 Max-7 added this pull request to the merge queue Jan 16, 2024
Merged via the queue into master with commit e518b55 Jan 16, 2024
13 checks passed
@Max-7 Max-7 deleted the ms-fix-toast-progress-duration branch January 16, 2024 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabienSvtr fabienSvtr fabienSvtr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[🐛 | Bug]: Toast progress bar doesn't take the duration into account
3 participants
@Max-7 @NicoTuxx @fabienSvtr