Skip to content

[code-infra] Skip charts animation for visual regression test#42530

Merged
alexfauquette merged 4 commits intomui:nextfrom
alexfauquette:fix-screenshots
Jun 5, 2024
Merged

[code-infra] Skip charts animation for visual regression test#42530
alexfauquette merged 4 commits intomui:nextfrom
alexfauquette:fix-screenshots

Conversation

@alexfauquette
Copy link
Member

@alexfauquette alexfauquette commented Jun 5, 2024

Should prevent that type of false positive

image

I also noticed we do screenshot of the dashboard and each component individually. Which seems like waisting resources. So I only kept the dashboard screenshot and remove all the subcomponents

See: #41757 (comment)

@mui-bot
Copy link

mui-bot commented Jun 5, 2024

Netlify deploy preview

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

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 07b9fed


const blacklist = [
'docs-getting-started-templates-dashboard-internals-components/CustomIcons.png', // No public components
// Next components are tested by docs-getting-started-templates-dashboard-components/MainGrid.png
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Next components are tested by docs-getting-started-templates-dashboard-components/MainGrid.png
// The following components are tested by docs-getting-started-templates-dashboard-components/MainGrid.png

The downside is that this list must be manually synchronized with the demos. But it's still better than running all demos unnecessarily, I guess.

Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@alexfauquette alexfauquette merged commit f0026ad into mui:next Jun 5, 2024
@zannager zannager added the internal Behind-the-scenes enhancement. Formerly called “core”. label Jun 5, 2024
@Janpot
Copy link
Member

Janpot commented Jun 5, 2024

Have you tried disabling animations in the playwright screenshot function here? This is how we do it in Toolpad, I believe it's a relatively new playwright API. The method would apply to all screenshots (including that flaky dashboard one 🙂 and doesn't require messing with the internal implementation details)

@michaldudak
Copy link
Member

If the animation is handled by React Spring, I'm afraid configuring it in Playwright may not be enough.

@alexfauquette
Copy link
Member Author

If the browser settings ask to avoid animation (mostly for a11y issues) the charts will skip them https://mui.com/x/react-charts/pie/#animation

maybe that's what make it works

@Janpot
Copy link
Member

Janpot commented Jun 5, 2024

Right, I'm also setting reducedMotion then in #42537

@oliviertassinari oliviertassinari added test scope: code-infra Involves the code-infra product (https://www.notion.so/mui-org/5562c14178aa42af97bc1fa5114000cd). labels Jun 9, 2024
@oliviertassinari oliviertassinari changed the title [core] Skip charts animation for visual regression test [code-infra] Skip charts animation for visual regression test Jun 9, 2024
joserodolfofreitas pushed a commit to joserodolfofreitas/material-ui that referenced this pull request Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Behind-the-scenes enhancement. Formerly called “core”. scope: code-infra Involves the code-infra product (https://www.notion.so/mui-org/5562c14178aa42af97bc1fa5114000cd). test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants