-
Notifications
You must be signed in to change notification settings - Fork 43
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
🐛 Refactor task state to icon mapping #1984
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1984 +/- ##
==========================================
+ Coverage 39.20% 42.23% +3.03%
==========================================
Files 146 170 +24
Lines 4857 5443 +586
Branches 1164 1355 +191
==========================================
+ Hits 1904 2299 +395
- Misses 2939 3128 +189
- Partials 14 16 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Icon to state mapping looks good.
Can you make taskStateToIcon
a normal functional component instead of a function? I think it'll look more consistent when used.
Follow the mapping used by the drawer with following changes: 1. use PauseCircleIcon also for QuotaBlocked state 2. use TaskIcon for Ready state (instead of CheckCircleIcon) 3. use CheckCircleIcon for state Succeeded Signed-off-by: Radoslaw Szwajkowski <[email protected]>
Signed-off-by: Radoslaw Szwajkowski <[email protected]>
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.
LGTM
Follow the mapping used by the drawer with following changes:
Part-of: #1969