-
Notifications
You must be signed in to change notification settings - Fork 838
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding width property to linear & circular progress #1797
Conversation
namnguyen20999
commented
Sep 17, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good.
Minor comments... let me know if I should re-review
@@ -110,7 +118,7 @@ const Progress = (props: ProgressBarProps) => { | |||
|
|||
return showValue && value !== undefined ? ( | |||
linear ? ( | |||
<Box sx={boxWithFlexDirectionSx}> | |||
<Box sx={boxWithFlexDirectionSx} data-testid="linear-progress-container"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need this attribute??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is used only for unit tests only, making it more stable than the auto-generated className from React, which might change due to styling or refactoring
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. But then.... all application get this... I mean users
Can you not add the class name when generating the component when testing (yes you can)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated, I've removed it from the component
Coverage report for
|
St.❔ |
Category | Percentage | Covered / Total |
---|---|---|---|
🟢 | Statements | 87.64% (+0.01% 🔼) |
3192/3642 |
🟡 | Branches | 69.09% (+0.04% 🔼) |
2186/3164 |
🟢 | Functions | 82.67% (+0.02% 🔼) |
582/704 |
🟢 | Lines | 88.21% (+0.01% 🔼) |
2956/3351 |
Test suite run success
642 tests passing in 43 suites.
Report generated by 🧪jest coverage report action from d18ecc8
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
Co-authored-by: Fabien Lelaquais <[email protected]>
… feature/progress_width
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍