-
-
Notifications
You must be signed in to change notification settings - Fork 3.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
feat(ui): move to React 18 and base for using shadcn/ui #4174
Conversation
… in Button component
✅ Deploy Preview for ohif-platform-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for ohif-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Passing run #3984 ↗︎
Details:
Review all test suite changes for PR #4174 ↗︎ |
Great, Great Great ! |
We are making progress, there's only two issues left:
|
The cypress is failing because its running out of memory in CircleCI, the server exits with process code 1, we might need to increase the memory for CircleCI or run the server in the background instead of the same job as the test. I can't modify CircleCI so I wasn't able to try a fix |
Two issues remaining:
|
Cypress has been fixed. The only warnings in the console right now are two warnings from react-dates, otherwise everything works as expected. |
The following is now resolved:
|
🚀 Move to React 18 and shadcn/ui
We're excited to announce that we're upgrading to React 18 and transitioning our UI components to shadcn/ui 🎉
Why React 18?
Why
shadcn/ui
?TLDR: Maintaining a customizable, responsive, and accessible component library is challenging and time consuming...
But the main question is basically why
radix-ui
, sinceshadcn/ui
is basically radix + tailwindcss. radix-ui is a headless library that is highly customizable and accessible.What's Next?
platofmr/ui-next
or@ohif/ui-next
Migration Guide
It should not be a big deal.
react
andreact-dom
to^18.3.1
.defaultProps
, so you need to change how your custom components are defined.before
after
babel-inline-svg
tosvgr
for importing SVG files. If you have the following imports:You should change it to:
Todos
Seems like the SVG import currently causes some warnings in the console. This is related to the
babel-plugin-inline-react-svg
from Airbnb, which needs to move away fromdefaultProps
to spread props for React 18. It appears they are working on resolving this issue. See airbnb/babel-plugin-inline-react-svg#86For developers adding new shadcn/ui to the