[CircularProgress] Backport simplified determinate style & deprecate static#22094
[CircularProgress] Backport simplified determinate style & deprecate static#22094eps1lon merged 9 commits intomui:masterfrom mbrookes:cp-backport-deprecate
Conversation
|
I wasn't sure about backporting this, since determinate simplification is a visually breaking change, but here it is anyway for discussion purpose if nothing else. |
Details of bundle changes.Comparing: 1d351a8...5d6e86d Details of page changes
|
eps1lon
left a comment
There was a problem hiding this comment.
The functionaliy of static should still be kept (otherwise it is breaking).
It's hard to tell from the test removal and removed classes. Keeping the classes is important. Otherwise any theme override would break. Same for the docs: The docs should still mention deprecated props since they (should) work perfectly fine. |
| React.useEffect(() => { | ||
| const timer = setInterval(() => { | ||
| setProgress((prevProgress) => (prevProgress >= 100 ? 10 : prevProgress + 10)); | ||
| setProgress((prevProgress) => (prevProgress >= 100 ? 0 : prevProgress + 10)); |
There was a problem hiding this comment.
Is this change intentional in this PR?
There was a problem hiding this comment.
It's a harmless change, so I left it.
packages/material-ui/src/CircularProgress/CircularProgress.test.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
The changes look breaking. I don't see how we can possibly add a deprecation for #22060.
…t.js Co-authored-by: Marija Najdova <mnajdova@gmail.com>
🤷 |
|
Now, we did remove the demos for variant="determinate" a couple of months ago, to prepare this breaking change. |
|
MB >>>> I wasn't sure about backporting this, since determinate simplification is a visually breaking change OT >>> The changes look breaking. MB >> 🤷 OT > Master: https://codesandbox.io/s/vibrant-wilbur-ph6ol Are you supporting or rejecting this PR? I can't quite tell. (I'm fine with it either way.) |
|
OK, I would vote for moving forward with the pull request then, it's pragmatic. |
That is to say - a passing test fails linting due to nesting, a non-nested equivalent doesn't pass. |
eps1lon
left a comment
There was a problem hiding this comment.
Could you push a version that works at runtime? I don't understand where the linter would complain.
packages/material-ui/src/CircularProgress/CircularProgress.test.js
Outdated
Show resolved
Hide resolved
|
Yep - I would have done the same if it hadn't been suggested not to. |
eps1lon
left a comment
There was a problem hiding this comment.
Sorry for the confusion with .checkPropTypes. I'm trying to hold two different Material-UI versions with 2 different React versions in my head and it shows.
Related to #22060.