-
Notifications
You must be signed in to change notification settings - Fork 23
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
Fix @mui/styled
#1085
base: main
Are you sure you want to change the base?
Fix @mui/styled
#1085
Conversation
We were using it in one place only, and while there are certainly some advantages over dynamic inline styles, we are using those pervasively as well anyway. Plus, we pull in MUI for some stuff, which by default relies on Emotion for that stuff, so if we want those benefits, I would propose that it's at least worth considering to get them through that.
As per https://mui.com/material-ui/getting-started/installation/#default-installation. These were installed as (optional?) transitive (peer?) dependencies, but that can of course change randomly. I've seen this happen while preparing a custom build of this repo. Note that there is also https://mui.com/material-ui/getting-started/installation/#with-styled-components, which is arguably more fitting to this project, because we were already using `styled-components`, but I opted for this solution because it seemed simpler and better supported, and also because our usage of `styled-components` isn't really justified. However, see the previous commit for that.
Use Run test server using develop.opencast.org as backend:
Specify a different backend like stable.opencast.org:
It may take a few seconds for the interface to spin up. |
This pull request is deployed at test.admin-interface.opencast.org/1085/2025-01-30_16-11-40/ . |
This PR does two things:
Get rid of
styled-components
We were using it in one place only, and while there are certainly some advantages over dynamic inline styles, we are using those pervasively as well anyway. Plus, we pull in MUI for some stuff, which by default relies on Emotion for that stuff, so if we want those benefits, I would propose that it's at least worth considering to get them through that.
Explicitly npm install @emotion/react @emotion/styled
As per https://mui.com/material-ui/getting-started/installation/#default-installation.
These were installed as (optional?) transitive (peer?) dependencies, but that can of course change randomly. I've seen this happen while preparing a custom build of this repo.
Note that there is also https://mui.com/material-ui/getting-started/installation/#with-styled-components, which is arguably more fitting to this project, because we were already using
styled-components
, but I opted for this solution because it seemed simpler and better supported, and also because our usage ofstyled-components
isn't really justified. However, see the previous commit for that.