Skip to content
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

Merged
merged 30 commits into from
May 27, 2024

Conversation

sedghi
Copy link
Member

@sedghi sedghi commented May 24, 2024

🚀 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?

  • React 18 was released in March 2022 and brings new features like concurrent rendering and automatic batching
  • Most shadcn/ui components require React ^18, so this upgrade is necessary for our UI transition

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, since shadcn/ui is basically radix + tailwindcss. radix-ui is a headless library that is highly customizable and accessible.

  • Maintaining our own component library has become tiresome and time-consuming, and Radix Primitives has proven to be a great alternative for streamlining the UI development process.
  • We still like Tailwind CSS (looking at you, StyleX...).
  • We're pushing for a more responsive and modern user interface.
  • We want to improve accessibility in our user interface.
  • We want to focus our resources on core features like rendering, segmentation, and processing.

What's Next?

  • We'll be gradually transitioning components to shadcn/ui over the coming months in a new platofmr/ui-next or @ohif/ui-next
  • Our design system will be applied to the new components to maintain a consistent look and feel

Migration Guide

It should not be a big deal.

  1. In any of your custom extensions and modes, you need to change the version of react and react-dom to ^18.3.1.
  2. There will be errors regarding the defaultProps, so you need to change how your custom components are defined.

before

const MyComponent = ({ prop1, prop2 }) => {
  return <div>{prop1} {prop2}</div>
}

MyComponent.defaultProps = {
  prop1: 'default value',
  prop2: 'default value'
}

after

const MyComponent = ({ prop1 = 'default value', prop2 = 'default value' }) => {
  return <div>{prop1} {prop2}</div>
}
  1. We have moved away from babel-inline-svg to svgr for importing SVG files. If you have the following imports:
import arrowDown from './../../assets/icons/arrow-down.svg';

You should change it to:

import { ReactComponent as arrowDown } from './../../assets/icons/arrow-down.svg';

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 from defaultProps to spread props for React 18. It appears they are working on resolving this issue. See airbnb/babel-plugin-inline-react-svg#86

For developers adding new shadcn/ui to the

  • Copy the code from shadcn/ui, and put it in the @ohif/ui-next under Components
  • Follow the export pattern (default vs named)

Copy link

netlify bot commented May 24, 2024

Deploy Preview for ohif-platform-docs ready!

Name Link
🔨 Latest commit 00c73b6
🔍 Latest deploy log https://app.netlify.com/sites/ohif-platform-docs/deploys/6654cc6ea134b50008a3b408
😎 Deploy Preview https://deploy-preview-4174--ohif-platform-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented May 24, 2024

Deploy Preview for ohif-dev ready!

Name Link
🔨 Latest commit 00c73b6
🔍 Latest deploy log https://app.netlify.com/sites/ohif-dev/deploys/6654cc6e7a12e600084aef92
😎 Deploy Preview https://deploy-preview-4174--ohif-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@sedghi sedghi changed the title feat/workers ui ux feat(ui): move to React 18 and base for using shadcn/ui May 24, 2024
@sedghi sedghi requested a review from IbrahimCSAE May 24, 2024 18:42
Copy link

cypress bot commented May 24, 2024

Passing run #3984 ↗︎

0 43 0 0 Flakiness 0

Details:

chore: Update date range input selectors in Cypress tests
Project: Viewers Commit: 00c73b6fe0
Status: Passed Duration: 05:10 💡
Started: May 27, 2024 6:21 PM Ended: May 27, 2024 6:26 PM

Review all test suite changes for PR #4174 ↗︎

@salimkanoun
Copy link
Contributor

Great, Great Great !

@IbrahimCSAE
Copy link
Collaborator

We are making progress, there's only two issues left:

  1. react-resize-detector should be updated to latest, current doesn't support this react
  2. react-dates is outdated, and no solution for this one because the package is dead, but its fine since we'll replace the data picker soon anyway, I can't see any other warnings anywhere

@IbrahimCSAE
Copy link
Collaborator

IbrahimCSAE commented May 25, 2024

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

.circleci/config.yml Outdated Show resolved Hide resolved
@IbrahimCSAE
Copy link
Collaborator

Two issues remaining:

  1. react-dates
  2. cypress memory issue

@IbrahimCSAE
Copy link
Collaborator

Cypress has been fixed.

The only warnings in the console right now are two warnings from react-dates, otherwise everything works as expected.

@IbrahimCSAE
Copy link
Collaborator

The following is now resolved:

  • Migrated from babel-inline-svg to svgr
  • Replaced react-dates with a new component
  • Upgraded react-resize-detector
  • Fixed cypress in CI

@sedghi sedghi merged commit 70f2c79 into master May 27, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants