fix(Progress): clamp bar width and improve propTypes check for progress prop
Fix some inconsistencies in Progress bar width and invalid prop combos.
-
bar width:
progress='value'andvalue > totale.g.<Progress progress='value' value={51} total={50} />Expected Result: progress bar with text51and width clamped to 100% to match behavior with any other value forprogressActual Result: progress bar with text51and width 102% -
bar width:
valuewithouttotale.g.<Progress value={50} />Expected Result: progress bar with width 50% to match behavior withprogress='value'Actual Result: progress bar with min width -
bad prop combo:
progress='ratiowithoutvalueortotale.g.<Progress progress="ratio" />Expected Result:propTypeserror Actual Result: nopropTypeserror, progress bar with textundefined/undefined -
bad prop combo:
progress='value'withoutvaluee.g.<Progress progress="value" />Expected Result:propTypeserror Actual Result: nopropTypeserror
Version
2.1.1
Testcase
https://codesandbox.io/s/progress-problem-with-progress-value-odipb?file=/index.js
💖 Thanks for opening this pull request! 💖
Here is a list of things that will help get it across the finish line:
- Run
yarn lintlocally to catch formatting errors. This will fix some errors automatically, commit and push any changes. - Run
yarn testlocally to catch errors. This ensures all components still behave as they should. - Run
yarn startto run the doc site locally and try a few pages, ensuring everything is in good working order. - Include tests when adding/changing behavior.
We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.
I thought for sure that the progress bar would allow to go past 100 in the parent SUI project, but I wanted to verify. Sure enough, it does not let you go past 100%. I put a Codesandbox together to check just to make sure, as that parent project is our source of truth. We get an error if the percentage exceeds 100%. Looks like this was something that was missed when the Progress component was built. But maybe this was intentional.
This could be considered a breaking change for this component as some people might be using it to show values exceeding 100%. I know there are scenarios in some of the projects I use the React bindings where we have used the Progress component in this way.