-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[icons] Add X logo #38811
[icons] Add X logo #38811
Conversation
Netlify deploy previewhttps://deploy-preview-38811--material-ui.netlify.app/ Bundle size report |
+1 on this PR. I need the updated icon from MUI |
so when can we see the update? i can provide the svg file if needed. I would like to use the new twitter icon with material without having to import my own icons into my project. |
@siriwatknp Can you review this? |
@siriwatknp could you review this PR, please? We need to use this icon in our app asap. |
+1 on this PR. We need the updated icon from MUI |
Also bumping +1 |
@siriwatknp could you please review this? Thanks |
Bumping this as well. I would love to get this reviewed! |
|
Bumping this |
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.
Tested, it works! @michaldudak Can you do a final review?
Since it's not called Twitter anymore, I'd be more inclined to create a new X icon and leave the old one as is (and remove it in the next major). |
please merge it fast , as a user I am also waiting |
Any updates on the icon? |
🤔 But I think we should replace the "Twitter" icon because https://twitter.com/ logo is |
The service is called X, the logo is X, and the domain x.com (whoever owned that has probably since retired 😂) redirects to twitter.com, but presumably they'll flip that at some point (possibly afraid of the SEO impact?). Name confusion aside, quite simply, changing the code of the Twitter icon component to display the X logo is a breaking change, and the component name no longer makes sense. Pros:
Cons:
So, we simply add it as X.js, and keep Twitter.js (for now at least). Why this wasn't done months ago is beyond me. This week please. |
Thanks for your comment. I'll create a new X logo. |
Thanks! |
Hey guys, great work so far, I can't seem to use XIcon or X on 5.15.2 Getting this error
|
For some reason, the new icon wasn't added to the index file. Try importing it with #40365 fixes this. |
@michaldudak thanks, worked. |
Wait remove the old one? Please don't... :"( |
Add
X
iconIterate on #38480