Material-ui: CircularProgress, length of the line does not animate under load

[henrylearn2rock]

• [x] I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

CircularProgress animation should be smooth and predictable under load

Current Behavior

The animation is okay but the dynamic length of the line only updates once a while under load. Maybe my understanding of Promise is wrong? I thought it would move the load away from the main UI thread?

Steps to Reproduce (for bugs)

1. generate some load inside a promise
2. observe CircularProgress animation, it still runs but the length of the circle stays constant

demo: https://codesandbox.io/s/j4xvnvv8nv (warning, Chrome pls, firefox crashes under this artificial load)

Context

My app uses canvas to resize a bunch of user selected files. I tried CircularProgress but to my surprise the animation is semi broken. My workaround: animated Gif.

Your Environment

| Tech | Version |
|--------------|---------|
| Material-UI | v1.0.0-beta.33 |
| React | 16.3 |
| browser | Chrome |

[oliviertassinari]

The circular progress animation requires CPU/GPU processing power to run, much more than a simple rotation. I don't think that we could do anything to improve the current situation. It's definitely a limitation. Under heavy load, we lose the stroke dash animation, the only thing that keeps working is the rotation. At least, we rely 100% on CSS to run the animation, some different implementation relies on JS too that makes it less efficient.

Some benchmark on a single frame:

without the dash animation

with the dash animation

The performance drawer when disabling the dash animation

(the spike of CPU scripting is the hot reloading, the dash animation freeze)

[oliviertassinari]

@henrylearn2rock My best advise would that you run scripting intensive operations in a web worker or by batch in order not to block the main rendering thread. Also, displaying a single circular progress on the page should cover 90% of the use cases. Don't overuse the component. Maybe we should warn if more than 10 instances are mounted?

[SergeyVolynkin]

I ran into this problem using Apollo Client Query component, it allows to display different components on different situations (Loading, Error, Success). And when I render CircularProgress on Loading it freezes in an ugly way.

The solution was to remove shrink animation from CircularProgress:

const styles = {
  circleIndeterminate: {
    animation: 'none',
    strokeDasharray: '80px, 200px',
    strokeDashoffset: '0px'
  }
}

const fixedCircularProgress = ({classes}) => <CircularProgress classes={{circleIndeterminate: classes.circleIndeterminate}}/>

export default withStyles(styles)(fixedCircularProgress)

I suggest adding shrinkAnimation prop to the CircularProgress component, so we can use:

<CircularProgress shrinkAnimation={false}/>

[oliviertassinari]

@SergeyVolynkin Your example can be condensed down to:

export default withStyles({
  circleIndeterminate: {
    animation: 'none',
    strokeDasharray: '80px, 200px',
    strokeDashoffset: '0px'
  }
})(CircularProgress);

Do you have a visual illustration of what's happening?
It would help us evaluate whether it's something worth adding or not in the core.

[SergeyVolynkin]

@oliviertassinari
Exactly like this one (But just from the start of the component mount):

Except in my case, the length of the line can become as small as 10px (that's what I describe it as "in an ugly way")

[oliviertassinari]

@SergeyVolynkin Oh sure, this sounds like a great idea! It's what Facebook is doing:

Do you want to open a pull request? :)

[SergeyVolynkin]

@oliviertassinari
I'll be able to implement the feature in ~3 weeks, if someone else will not do that.

[oliviertassinari]

@SergeyVolynkin Awesome :)

[joshwooding]

@oliviertassinari will the shrinkAnimation prop only work on the indeterminate variant?

[oliviertassinari]

@joshwooding Yes, we can add a warning to ensure that constraint.

[joshwooding]

@oliviertassinari Is it okay if I work on this?

Do you have a warning in mind? I was thinking:

''Material-UI: you have provided a shrinkAnimation property with a variant other than indeterminate. This will have no effect.''

Also I'm pretty sure I would have to change CircularProgress to a class component to prevent the warning from printing every render. Is this cool?

[oliviertassinari]

@joshwooding The warning wording is great. Printing the warning at each render isn't great. However, it's something we are already doing. It's a valid option. The alternative is to use the chainPropTypes() helper. You have some example in the codebase.