-
Notifications
You must be signed in to change notification settings - Fork 473
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
fix: Add owner icon to confirmations #1436
Conversation
Branch preview✅ Deploy successful! |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
component={OwnersIcon} | ||
inheritViewBox | ||
fontSize="small" | ||
sx={{ color: ({ palette }) => palette.text.primary }} |
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.
Can this be inside the icon itself?
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.
I tried but it overrides the other places where we use this icon.
In the queue the icon is either grey or black/green.
In the dashboard its always black/white.
Might be a good idea to make this more consistent but maybe there is a reason the icons + text are not green on the dashboard in dark mode. cc @liliiaorlenko
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 but tests are failing.
ec107ec
to
b4d910e
Compare
I reverted the color changes and suggest we tackle it as a separate task after aligning with design. |
Ok, so only the Owner Icon is being take in account for this ticket, and the icon looks good. |
504196e
to
36d5474
Compare
ESLint Summary View Full Report
Report generated by eslint-plus-action |
What it solves
Resolves #1279
How this PR fixes it
How to test it
Screenshots