Remove @ant-design/icons dependency #1458
Conversation
jvorcak
commented
Sep 24, 2024
•
jvorcak
commented
jvorcak
requested review from
malinskibeniamin,
frankieshakes and
rikimaru0345
| @@ -310,7 +308,6 @@ const ConnectorWizard = observer(({ connectClusters, activeCluster }: ConnectorW | |||
| { | |||
| title: 'Properties', | |||
| description: 'Configure basic connection properties.', | |||
| icon: <ApiOutlined />, | |||
There was a problem hiding this comment.
After the migration, we don't support icons in the Wizards
| @@ -605,7 +589,7 @@ class ColAfter extends Component<{ | |||
| There is no offset for this partition at or after the given timestamp (<code>{new Date(this.props.selectedTime ?? 0).toLocaleString()}</code>). As a fallback, the last offset in that partition will be used. | |||
| </div> | |||
| } | |||
| icon={<WarningOutlined/>} | |||
| icon={<MdOutlineWarningAmber size={14} />} | |||
There was a problem hiding this comment.
Any reason why we have different icon sizes in various locations? I think 16 is the default?
| <span style={{fontSize: '19px'}}> | ||
| <WarningTwoTone twoToneColor="orange"/> | ||
| </span> | ||
| <MdOutlineWarningAmber color="orange" size={20} /> |
There was a problem hiding this comment.
Why is there different sizing for MdOutlineWarningAmber in various places?
There was a problem hiding this comment.
I think I'll answer both of your question in one comment. At first, I tried to keep everything unified, but the current implementation also did have various sizes on various places + MD icons are visually positioned differently than ant-design icons if we keep the same dimensions.
So I tried to adjust this to reduce/minify visual regressions, even though it was a bit handy to do. I think we can make it continuously more consistent over time (and we should). But that would require much more time now, and I think removing this dependency is not so business critical now.
There was a problem hiding this comment.
Left some comments, I will let @rikimaru0345 take a final look
