react-icons upgrade from 2.2.7 to 4.10.1 #1586
Conversation
Signed-off-by: kirtanchandak <[email protected]>
Signed-off-by: Kirtan Manoj Chandak <[email protected]>
|
Hey @yurishkuro, I've not touched above logic in the unit tests and I think those are warnings, can we merge this request or do I need to do some more changes? |
| @@ -92,7 +92,8 @@ | |||
| "react-dom": "^18.2.0", | |||
| "react-ga": "^3.3.1", | |||
| "react-helmet": "^6.1.0", | |||
| "react-icons": "2.2.7", | |||
| "react-icons": "^4.10.1", | |||
| "react-metrics": "^2.3.2", | |||
| @@ -14,7 +14,7 @@ | |||
|
|
|||
| import React from 'react'; | |||
| import cx from 'classnames'; | |||
| import IoAndroidOpen from 'react-icons/lib/io/android-open'; | |||
| import { IoLogoAndroid } from 'react-icons/io5'; | |||
There was a problem hiding this comment.
What is your methodology when changing the icons? I just looked at IoLogoAndroid, and it's an android logo, which I don't recall using in Jaeger, certainly not as New Window Icon.
We're using Ant Design as the primary components library. Can use its icons components directly, instead of react-icons (which also has a section for Ant Design, but I am not sure what that means)?
There was a problem hiding this comment.
So, should I change all the react-icons with ant-icons ??
There was a problem hiding this comment.
I think so. I see that we have indirect dependency in the lock file on these:
"@ant-design/icons" "~2.1.1"
"@ant-design/icons-react" "~2.0.1"
There was a problem hiding this comment.
It would be good to keep a visual record (e.g. google doc with screenshots) of which icons are being changed for which.
There was a problem hiding this comment.
So finally should I use ant-icons or react-icons?
And yes I will maintain a google docs file with ss to record the changes!
There was a problem hiding this comment.
so let me ask you, since I am not an expert in this area - react-icons seems like an umbrella project that includes icons from many design systems. You can use ant-icons directly or via react-icons/ant - what would be the difference if you do? If we use react-icons/ant, does it make it easier to swap to react-icons/smth-else?
There was a problem hiding this comment.
I think as antd is the primary design system, we should use @ant-design/icons instead of all the react-icons
|
done in #1721 |
In this PR I have attempted to upgrade the react-icons version from 2.2.7 to 4.10.1