[CircularProgress] Port component

[nathanmarks]

• Component
• Tests (at least unit tests)
• Docs
• Demo
• Fix [CircularProgress] should start at the top #4747
• Fix [CircularProgress] Only works with Chrome #5524

[mbrookes]

There are a few open CircularProgress PRs, but this stands out as worth trying to tackle when migrating: #707

[nathanmarks]

@mbrookes thanks, 100%!!!

[nathanmarks]

I've managed to reproduce this component with keyframes, maintaining all flexibility except thickness. FFS.

I have an idea for thickness -- but it would involve creating multiple sets of keyframes (for each thickness). Any opinions?

[mbrookes]

I guess it will depend on how cumbersome it becomes... thickness isn't in a released version yet, so worst case if we had to drop it, it wouldn't be completely the end of the world.

[turnerniles]

Would it be possible to add a "loaded" prop to the progress components such that when loaded is false the progress component shows and loaded is true the progress component hides?

[oliviertassinari]

@turnerniles What the advantage of having this logic in the component instead of in userland? What you described seems pretty common to every component. Wouldn't it be better to use composition. Does it involve a transition?

[turnerniles]

@oliviertassinari I think it is specifically applicable to the linear and circular progress components since anyone who is using a progress component will most likely want to hide or remove the component once a load is complete.

Yes it is easily done in composition; however it would be convenient to have the logic in the component.

In the angular material progress component there is the attribute ng-disabled which hides the component when true:
https://material.angularjs.org/latest/api/directive/mdProgressCircular

What do you think?

[oliviertassinari]

@turnerniles Sounds like what you proposed is to allow the following pattern:

<CircularProgress opened={opened} />

Instead of the following one

opened && <CircularProgress />

I can't see any advantage here. Actually, you probably want some more logic. E.g. Displaying the loader after X ms for at least Y ms. That logic would be better off in another module, as has nothing to do with the rendered component.
Any light?

[nathanmarks]

Yeah, I don't think that having an opened/loading state is a good idea, it is unnecessary given the simplicity of implementing it in userland.

It's also not uncommon to have a lot more than just the loader hiding/showing when the loading state in your app changes, so I don't see a huge benefit anyways.

[turnerniles]

@oliviertassinari, @nathanmarks ok, makes sense

[slavab89]

Regarding the thickness prop that's currently just a const.
It should be possible to achieve that using jss function-values. However i didnt find a way to access that update method when using jss-theme-reactor, or the ability to use dynamic-rules.

[oliviertassinari]

@slavab89 I think that we have to upgrade jss first.

[mbrookes]

That could be useful in a few places we've had to revert to inline styles for dynamic behaviour.

[hoschi]

CircularProcess is in the docs page of current alpha docs https://material-ui-1dab0.firebaseapp.com/component-demos/progress

Probably "Component", "Docs", "Demo" can be checked

[oliviertassinari]

I'm closing, let's fix #5524 aside