refactor: [M3-8339] - Clean up and fix Linode Details after CDS 2.0 #10662
Conversation
| @@ -858,7 +858,7 @@ export const darkTheme: ThemeOptions = { | |||
| palette: { | |||
| background: { | |||
| default: customDarkModeOptions.bg.app, | |||
| paper: Color.Neutrals[100], | |||
| paper: Color.Neutrals[90], | |||
There was a problem hiding this comment.
bg.bgPaper is set to Color.Neutrals[90] so I think this should probably be set to the same thing.
bg.bgPaper should probably be phased out in the long run in favor of MUI's built in palette value theme.palette.background.paper
There was a problem hiding this comment.
We will clean some of that up in future theme layer updates. Ideally we don't want to use either directly anyway. In our perfect world we won't have any exportable theme constants.
| <StyledTypography variant="h3">IP Addresses</StyledTypography> | ||
| </Grid> | ||
| <StyledWrapperGrid> | ||
| <Hidden smDown> |
There was a problem hiding this comment.
I removed this Hidden because we should probably be allowing mobile users to access IP Transfer and IP Sharing
There was a problem hiding this comment.
This does lead to some wrapping at narrow widths but it doesn't affect usability:
There was a problem hiding this comment.
Updated this to switch over to an ActionMenu on small screens in a22ef07
|
Coverage Report: ✅ |
| > | ||
| <Stack> | ||
| <Box | ||
| bgcolor={(theme) => theme.palette.background.paper} |
There was a problem hiding this comment.
Why are we adding a background color to the headers in this section, is it to match what we had before?
There was a problem hiding this comment.
Yeah, exactly. The previous implementation was using theme.color.white, but since CDS was merged, the color changed causing these headers to have no visible background in dark mode. I updated this to use theme.palette.background.paper, which seems more fitting anyways.
We could probably use a Paper instead of a Box here, but I went with Box to keep things more "raw"
There was a problem hiding this comment.
I don't understand what keep things more "raw"means but using the right component is always the better option VS a one time style, especially as the theme will go through more changes in the near future
There was a problem hiding this comment.
Good point. I initially hesitated to use Paper because I was worried that its built-in padding would cause problems, but it doesn't matter because I'm specifying the padding manually anyway
Addressed in 8730aae
| > | ||
| <Stack> | ||
| <Box | ||
| bgcolor={(theme) => theme.palette.background.paper} |
There was a problem hiding this comment.
I don't understand what keep things more "raw"means but using the right component is always the better option VS a one time style, especially as the theme will go through more changes in the near future
packages/manager/src/features/Linodes/LinodesDetail/LinodeStorage/LinodeVolumes.tsx
Outdated
Show resolved
Hide resolved
| @@ -858,7 +858,7 @@ export const darkTheme: ThemeOptions = { | |||
| palette: { | |||
| background: { | |||
| default: customDarkModeOptions.bg.app, | |||
| paper: Color.Neutrals[100], | |||
| paper: Color.Neutrals[90], | |||
There was a problem hiding this comment.
We will clean some of that up in future theme layer updates. Ideally we don't want to use either directly anyway. In our perfect world we won't have any exportable theme constants.
Description 📝
styledcomponents 🗑️Grid🗑️AddNewLinkwithButton🔘Preview 📷
How to test 🧪
As an Author I have considered 🤔