mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.17k stars 1.3k forks source link

[charts] browser reduce motion does not disable the initial animation in production #13477

Closed alexfauquette closed 2 weeks ago

alexfauquette commented 4 months ago

Steps to reproduce

See https://github.com/mui/material-ui/pull/42537#issuecomment-2152780974

Current behavior

No response

Expected behavior

No response

Context

No response

Your environment

npx @mui/envinfo ``` Don't forget to mention which browser you used. Output from `npx @mui/envinfo` goes here. ```

Search keywords: a11y animation charts

JCQuintas commented 2 months ago

Btw, I believe this happens because the Global change is triggered in about the same render step or after the animation starts. If we skipAnimation statically, the issue disappears.

// docs/pages/_app.js
import { Globals } from '@react-spring/web';

Globals.assign({
  skipAnimation: true,
});
Janpot commented 1 month ago

Btw, I believe this happens because the Global change is triggered in about the same render step or after the animation starts. If we skipAnimation statically, the issue disappears.

I was taking another look at the flaky regression test in core and when I set CPU throttling to 6x, I'm still seeing some sort of initial animation happening on docs-getting-started-templates-dashboard-components/MainGrid:

https://github.com/user-attachments/assets/28d6f03d-7dd8-4706-b9ce-fbf43ecef2c0

Even with global animations turned off it seems to be rendering something narrower on the initial render. Just saying because these tests do

Globals.assign({
  skipAnimation: true,
});

Maybe this is a separate issue?

alexfauquette commented 1 month ago

This motion does not correspond to one of our animations. It's much closer to what happened when the responsive container resize 🤔

image image

Janpot commented 1 month ago

This motion does not correspond to one of our animations. It's much closer to what happened when the responsive container resize 🤔

By "responsive container" you still mean "of the chart", right? Because the direct parent element does not resize.

JCQuintas commented 1 month ago

Would it be possible that it is rendered in a size on ssr and then hydrated into another size? 🤔

It does indeed look like the chart just changed size. Are you able to check the svg properties when this is happening?

Janpot commented 1 month ago

It's the core regression testing fixture. It's not SSRed, it's rendered in a nested React root created in an effect. To reproduce:

  1. On the core repository
  2. Run pnpm test:regressions:dev
  3. Set CPU trhottling in the browser to 6x
  4. visit http://localhost:5001/docs-getting-started-templates-dashboard-components/MainGrid.

Are you able to check the svg properties when this is happening?

In the devtool elements panel you mean? No, not really, it always resets on reload.

JCQuintas commented 1 month ago

Any specific browser? I didn't see this behaviour on chrome, tested both 6x and 20x

Janpot commented 1 month ago

That was on Chrome. This is firefox:

https://github.com/user-attachments/assets/8a1466e1-a8fa-47d2-a5a8-f960d9ee2aad

Seeing the same on Safari, Edge and mobile chrome. But only when running locally, latest master. Not seeing it on the deployed version. Maybe it's something poisoning the build locally 🤔

JCQuintas commented 1 month ago

Oddly enough, was seeing it happening when the CPU is not slowed down, but not after I open the devtools. Turns out the devtools made it withing the width range where the issue doesn't appear.

Sweet spot is between ~912 - ~1590 where no shuflling happens

I would assume this specific issue is due to some layout changes after css/js is loaded?

Janpot commented 1 month ago

I reduced the test fixture down to:

import * as React from 'react';
import * as ReactDOMClient from 'react-dom/client';
import { Globals } from '@react-spring/web';
import Box from '@mui/material/Box';
import Grid from '@mui/material/Grid2';
import { SparkLineChart } from '@mui/x-charts/SparkLineChart';

Globals.assign({
  skipAnimation: true,
});

function StatCard() {
  return (
    <Grid size={6} sx={{ height: 50 }}>
      <SparkLineChart data={[1640, 220]} />
    </Grid>
  );
}

function App() {
  return (
    <Box sx={{ display: 'inline-block' }}>
      <Grid container spacing={2}>
        <StatCard />
        <StatCard />
        <StatCard />
      </Grid>
    </Box>
  );
}

const container = document.getElementById('react-root');
const children = <App />;
const reactRoot = ReactDOMClient.createRoot(container);
reactRoot.render(children);

https://github.com/user-attachments/assets/4c7c6405-daae-458a-982a-99f799fe7eb4

It looks like there is some interaction going on between the top level inline-block in the test viewer, the Grid2 and the SparkLineChart.

codesandbox behaves differently though.

Screenshot 2024-09-08 at 12 41 39
JCQuintas commented 1 month ago

@Janpot the issue here seems to be that the container doesn't have a proper width, which means the children has to "grow until they fit" into the container.

One alternative would be to change the component that has the display: 'inline-block' to have a width as well. <Box sx={{ display: 'inline-block', width: '100%' }}>

I've created https://github.com/mui/mui-x/pull/14553 which should fix the issue in most common cases if the proper solution above is not possible.

Janpot commented 1 month ago

it's possible but it updates all the screenshots https://github.com/mui/material-ui/pull/43656

github-actions[bot] commented 2 weeks ago

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue. Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.