-
Notifications
You must be signed in to change notification settings - Fork 513
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
[LoadingIndicator]: Replace defaultProps with destructuring #2601
[LoadingIndicator]: Replace defaultProps with destructuring #2601
Conversation
Partially Resolves jaegertracing#2596 - Updated the LoadingIndicator component to remove defaultProps so as to avoid the deprecation warnings. - Updated the snapshots so as to match the new LoadingIndicator. Signed-off-by: Abhishek <bumblebee31304@gmail.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2601 +/- ##
=======================================
Coverage 96.61% 96.61%
=======================================
Files 255 255
Lines 7745 7745
Branches 1999 1988 -11
=======================================
Hits 7483 7483
Misses 262 262 ☔ View full report in Codecov by Sentry. |
Partially Resolves jaegertracing#2596 - Updated the LoadingIndicator component to remove LoadingIndicatorProps so as to avoid redundancy. Signed-off-by: Abhishek <bumblebee31304@gmail.com>
Partially Resolves jaegertracing#2596 - Removed optional availability from multiple fields Signed-off-by: Abhishek <bumblebee31304@gmail.com>
make sure |
this linting issue (similar but not the same)
is there in all the components, either i will have to explicitly mention all the non optional props that are not passed yet, from all the components that use loading indicator, or make these optional in the loading indicator itself. |
maybe make |
Partially Resolves jaegertracing#2596 - Added optional availability to multiple fields Signed-off-by: Abhishek <bumblebee31304@gmail.com>
Done, the lint tests pass now |
Signed-off-by: Yuri Shkuro <github@ysh.us>
Thanks! |
Which problem is this PR solving?
Description of the changes
How was this change tested?
npm ci
npm run update-snapshots
andnpm test
passes all the testsChecklist
jaeger
:make lint test
jaeger-ui
:npm run lint
andnpm run test