DT-1152: Migrate Components from @mui/styles #1751
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
rushtong
requested review from
pshapiro4broad and
s-rubenstein
and removed request for
a team
|
| <Button | ||
| variant="contained" | ||
| color="primary" | ||
| disableElevation | ||
| disabled={!isEmail(addEmailInput) && !_.some(usersToAdd, (user) => isEmail(user))} | ||
| className={clsx(classes.button, classes.section)} | ||
| sx={{ | ||
| backgroundColor: 'common.link', |
There was a problem hiding this comment.
This one is borderline. It only has three properties, but it also has the hover logic? What do you think about pulling this out?
There was a problem hiding this comment.
That would totally be reasonable to pull it into a styled component.
The approach I took with my PR was just to go with whichever option (sx or styled) copilot picked. Especially for cases where a selection is borderline, I would like to defer to copilot's pick mostly to help expedite these PRs. We can always revisit a given pick if we're working with a component in the future.
There was a problem hiding this comment.
I'm happy with the current approach, and I think once more components are converted, we'll have a better handle on how to generify this to handle more cases. For instance, our CustomTheme + useTheme() approach did not work in TextContent.jsx due to the span as an argument to styled.
Addresses
https://broadworkbench.atlassian.net/browse/DT-1152
Summary
For each component, followed the following process:
styledfrom'@mui/material/styles';instead of'@mui/system';. Copilot doesn't catch that.propsis the passed in function argument and destructure on a separate line in the function. This helps avoid areadonlywarning.readonlymodifiers// @ts-ignorewhere necessary. I found that we needed to do this for a variety of theme-related fields.Other notes:
Failure.jsxis unused, but I'm not aware of the history for this so it is included as part of the migration.InfoModal- Need some guidance on how to find where this is in use.TextContentis visible on all tablesAddUserAccess
FullViewSnapshotButton
LoadingSpinner
Screen.Recording.2025-02-11.at.7.28.44.AM.mov
TabPanel, TabWrapper
Screen.Recording.2025-02-11.at.7.30.04.AM.mov
TerraTooltip