-
Notifications
You must be signed in to change notification settings - Fork 3
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
VE-219 Migrate RUI to MUI v6 #984
Conversation
Apart from the above everything LGTM. |
No tests are failing now so we dont need to update this, confirmed with @mattcorner |
I need to combine the current seperate themse variables we have for dark and white into one as MUI suggests here . Also planning to handle this in a seperate PR. |
src/ThemeProvider/ThemeProvider.tsx
Outdated
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 thought we agreed to back out of these changes for now and deal with them seperately.
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.
Yes, handling seperality here. I will remove the flags
package.json
Outdated
@@ -34,7 +34,7 @@ | |||
"devDependencies": { | |||
"@emotion/react": "^11.11.4", | |||
"@emotion/styled": "^11.11.5", | |||
"@mui/material": "^5.16.4", | |||
"@mui/material": "^6.0.0", |
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.
Why just 6.0.0? Why not 6.1.9 which is latest?
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.
Yes we can update to 6.1.9, i was following what documentation suggests here
src/DeletableList/DeletableList.tsx
Outdated
sx={theme => ({ | ||
py: theme.spacing(1) | ||
})} |
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.
sx={theme => ({ | |
py: theme.spacing(1) | |
})} | |
sx={{ py: 1 }} |
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've done no testing, but the code changes look fine to me. @nikitharani to complete the full review.
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.
LGTM
Closes VE-219
This PR focuses on migrating RUI from MUI v5 to v6. There are few breaking changes based on this documentation. There will be separate PR's to handle deprecations, migrating to Pigment CSS(if we prefer) etc. Some major changes introduced and updated in this PR are as follows
cssVariables
flag, to enable access to css variables from thetheme.vars
object, read more.colorSchemes
node to simplify the process to support light and dark mode, read moretheme.applyStyles()
to replace thetheme.palette.mode === 'dark'
condition. This now solves the dark mode flickering issue when combined with the CSS theme variables feature above.Grid
is deprecated , importing it asGrid2
from'@mui/material/Grid2'
orGrid2 as Grid
from"@mui/material"
, read morecolor
prop in theTypography
component is not a system prop anymore. Updated to use assx
prop now, read moreChanges
Package Updates
"@mui/material"
and"@mui/icons-material"
to6.0.0
Accessing theme
sx
prop changes in this PR as follows. There is no harm following the old usage but we are updating this for future protection.Accessing mode
Grid Updates
Grid
is deprecated and we use[Grid2](https://mui.com/material-ui/migration/upgrade-to-v6/#grid2)
now and props likexs, sm, md, lg, xl
andxsOffset, smOffset, mdOffset, lgOffset, xlOffset
renamed asSize
andOffset
. Usage as followsRemoving a few deprecations
TextField
were updated to useslotProps
instead of the deprecatedinputProps
. As this PR grows, I plan to handle all other deprecations separately.Spell checks
PasswordChangeDialog.jsx
Typography
color
as system prop, instead using as followsUI/UX
N/A
Testing notes
There are no new visible UI changes, but as this is a major package update, I suggest checking that all components still work fine as before.
Author checklist
Before I request a review:
I have populated the deploy-preview with relevant test data.I have tagged a UI/UX designer for review if this PR includes UI/UX changes.