Conversation
Fix Typography warning Color error icons red Cleanup: remove unused classnames Fix inlining of error dialog icon in tables space this out Make error icon a little smaller Color the namespace icons
klingerf
left a comment
klingerf
left a comment
There was a problem hiding this comment.
⭐️ Huge improvement! Just had some quibbles about the icons, but otherwise looks good to me.
| const isMeshedTooltip = ( | ||
| <Tooltip title="Namespace is meshed" placement="right-start"> | ||
| <CheckCircleIcon /> | ||
| <CheckCircleIcon color="primary" /> |
There was a problem hiding this comment.
I'm not sure what the issue is here, but the check icon doesn't show up for me in Chrome or Firefox:
It does show up in Safari:
Also, this is nit-picky, but is it possible to shrink the icon somewhat so that it matches the letter height more closely? And to possibly give it some left passing to separate it from the namespace name? It feels a bit to big at the moment.
There was a problem hiding this comment.
that is sooooo strange! the icons show up for me on Chrome and Safari, but not Firefox
There was a problem hiding this comment.
I've made #1805 to track this, which I'll address soon after merging this branch!
| } else { | ||
| return <ErrorIcon onClick={this.handleClickOpen} />; | ||
| return ( | ||
| <ErrorIcon color="error" fontSize="small" onClick={this.handleClickOpen} /> |
There was a problem hiding this comment.
Similar to my other comment -- would it be possible to make the height of this icon match the height of the text here, and remove the padding on the bottom?
There was a problem hiding this comment.
That's so strange... it's aligned for me:
There was a problem hiding this comment.
I figured it out, I was looking at the icons in the StatusTable, but the ones on the MetricsTable aren't aligned!
2 participants
This branch:
Before
After
Before
After
Fixes #1783