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

[Skeleton] Apply the wave animation to the correct element #43474

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

mnajdova
Copy link
Member

@mnajdova mnajdova commented Aug 27, 2024

Fixes #43470. While migrating to the variants API, we forgot to add the selector for the ::after element when applying the wave animation, check v5 and instead we had it on the root element.

Tested that it works with both Emotion & Pigment CSS.

Broken behavior: https://mui.com/material-ui/react-skeleton/#animations
Fixed behavior: https://deploy-preview-43474--material-ui.netlify.app/material-ui/react-skeleton/#animations

@mnajdova mnajdova added component: skeleton This is the name of the generic UI component, not the React module! regression A bug, but worse labels Aug 27, 2024
@mui-bot
Copy link

mui-bot commented Aug 27, 2024

Netlify deploy preview

https://deploy-preview-43474--material-ui.netlify.app/

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 884346a

@mnajdova mnajdova marked this pull request as ready for review August 27, 2024 13:28
Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

I wonder how this got through Argos 😅

Probably worth it releasing a patch with this one.

@mnajdova
Copy link
Member Author

I wonder how this got through Argos 😅

It's an animation.

Probably worth it releasing a patch with this one.

I am keeping an eye on the issues, let's see if there is something else reported in the first 24 hours that we should fix, and decide tomorrow if it's worth doing a release :)

@mnajdova mnajdova merged commit d2e0762 into mui:master Aug 27, 2024
21 checks passed
@trungutt
Copy link

@mnajdova @DiegoAndai FWIW we are using Chromatic for our visual testing (we don't have Argos but it's interesting alternative). Chromatic indicated difference in several of our stories which were making use of Skeleton component, hence the issue

@mnajdova
Copy link
Member Author

Chromatic indicated difference in several of our stories which were making use of Skeleton component, hence the issue

Interesting, we'll look into it. We never really tested animations so far (at least not visually).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: skeleton This is the name of the generic UI component, not the React module! regression A bug, but worse
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[material-ui][Skeleton] animation="wave" not displaying correctly in MUI v6
4 participants