- Notifications
You must be signed in to change notification settings - Fork1k
chore: migrate MUI icons to lucide-react#20193
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
Just reviewed all the stories. I really like a lot of the changes, but some icons got way too big, and I think a couple of UI elements could be tweaked I accepted all the stories that I had zero issue with. The only rejected stories are the ones that I think absolutely need to change. Everything else was checked, and while I was unsure about the changes, I think I could be talked into accepting them pretty easily |
@Parkreiner I reviewed all your comments and made the requested changes. Thanks for the feedback — it was super helpful! 🙏 |
Parkreiner left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 caught a really small bug that affects the markup output in a weird way, but definitely not worth blocking over. Approving to get you unstuck
| /> | ||
| )} | ||
| <span | ||
| className={`${info.hasDiff&&"text-content-warning"}`} |
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.
Oh, this looks like a bug. Ifinfo.hasDiff evaluates tofalse, we'll short-circuit and not evaluate the string. But then the expression will still befalse, and we'll put that in the template literal, so then we'll haveclass="false" in the HTML
Feel likecn is probably the easiest way to fix this
cfacd1c to818a1bbCompare4b8d739 intomainUh oh!
There was an error while loading.Please reload this page.
No description provided.