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

[Dashboard] Upgrade to MUIv5 and dependency updates #45789

Merged
merged 12 commits into from
Jul 2, 2024
27,419 changes: 8,866 additions & 18,553 deletions dashboard/client/package-lock.json

Large diffs are not rendered by default.

17 changes: 6 additions & 11 deletions dashboard/client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,20 @@
"@emotion/styled": "^11.11.0",
"@mui/icons-material": "^5.15.5",
"@mui/material": "^5.15.5",
"@mui/styles": "^5.15.5",
"@reduxjs/toolkit": "^1.3.1",
"@types/classnames": "^2.2.10",
"@types/jest": "^27.5.2",
"@types/lodash": "^4.14.161",
"@types/lowlight": "^0.0.1",
"@types/node": "13.9.5",
"@types/numeral": "^0.0.26",
"@types/react-redux": "^7.1.7",
"@types/react-window": "^1.8.2",
"axios": "^0.21.1",
"classnames": "^2.2.6",
"copy-to-clipboard": "^3.3.2",
"dayjs": "^1.9.4",
"js-yaml": "^4.1.0",
"lodash": "^4.17.20",
"lowlight": "^1.14.0",
"react": "^17.0.2",
"react-dom": "^17.0.2",
"lowlight": "^2.9.0",
"react": "^18.3.0",
"react-dom": "^18.3.0",
"react-icons": "^4.7.1",
"react-router-dom": "^6.4.3",
"react-scripts": "^5.0.1",
Expand All @@ -37,11 +32,11 @@
},
"devDependencies": {
"@testing-library/jest-dom": "^5.16.5",
"@testing-library/react": "^12.1.5",
"@testing-library/react": "^14.3.0",
"@testing-library/user-event": "^14.4.3",
"@types/js-yaml": "^4.0.5",
"@types/react": "^17.0.50",
"@types/react-dom": "^17.0.17",
"@types/react": "^18.3.0",
"@types/react-dom": "^18.3.0",
"@typescript-eslint/eslint-plugin": "^5.41.0",
"@typescript-eslint/parser": "^5.41.0",
"eslint-plugin-import": "^2.26.0",
Expand Down
11 changes: 1 addition & 10 deletions dashboard/client/src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
import { CssBaseline } from "@mui/material";
import {
StyledEngineProvider,
Theme,
ThemeProvider,
} from "@mui/material/styles";
import { StyledEngineProvider, ThemeProvider } from "@mui/material/styles";
import dayjs from "dayjs";
import duration from "dayjs/plugin/duration";
import React, { Suspense, useEffect, useState } from "react";
Expand Down Expand Up @@ -57,11 +53,6 @@ import { TaskPage } from "./pages/task/TaskPage";
import { getNodeList } from "./service/node";
import { lightTheme } from "./theme";

declare module "@mui/styles/defaultTheme" {
// eslint-disable-next-line @typescript-eslint/no-empty-interface, @typescript-eslint/consistent-type-definitions
interface DefaultTheme extends Theme {}
}

dayjs.extend(duration);

// lazy loading fro prevent loading too much code at once
Expand Down
81 changes: 43 additions & 38 deletions dashboard/client/src/common/CodeDialogButton/CodeDialogButton.tsx
Original file line number Diff line number Diff line change
@@ -1,23 +1,26 @@
import { Card, Link, Typography } from "@mui/material";
import createStyles from "@mui/styles/createStyles";
import makeStyles from "@mui/styles/makeStyles";
import classNames from "classnames";
import {
Box,
Card,
Link,
SxProps,
Theme,
Typography,
useTheme,
} from "@mui/material";
import yaml from "js-yaml";
import React, { useState } from "react";
import DialogWithTitle from "../DialogWithTitle";
import { ClassNameProps } from "../props";

const useStyles = makeStyles((theme) =>
createStyles({
configText: {
whiteSpace: "pre",
fontFamily: "SFMono-Regular,Consolas,Liberation Mono,Menlo,monospace",
padding: theme.spacing(2),
overflow: "scroll",
maxHeight: 600,
},
}),
);
const useStyles = (theme: Theme) => ({
configText: {
whiteSpace: "pre",
fontFamily: "SFMono-Regular,Consolas,Liberation Mono,Menlo,monospace",
padding: theme.spacing(2),
overflow: "scroll",
maxHeight: 600,
},
});

export type CodeDialogButtonProps = {
/**
Expand All @@ -32,6 +35,7 @@ export type CodeDialogButtonProps = {
* Code to show in the dialog. If an object is passed in, that object will be stringified to yaml.
*/
code: string | object;
sx?: SxProps<Theme>;
};

/**
Expand All @@ -42,7 +46,7 @@ export const CodeDialogButton = ({
buttonText = "View",
code,
}: CodeDialogButtonProps) => {
const classes = useStyles();
const styles = useStyles(useTheme());

const [showConfigDialog, setShowConfigDialog] = useState(false);

Expand All @@ -63,7 +67,7 @@ export const CodeDialogButton = ({
}}
>
<Card variant="outlined">
<Typography className={classes.configText}>
<Typography sx={styles.configText}>
{typeof code === "string" ? code : yaml.dump(code, { indent: 2 })}
</Typography>
</Card>
Expand All @@ -73,23 +77,21 @@ export const CodeDialogButton = ({
);
};

const useCodeDialogButtonWithPreviewStyles = makeStyles((theme) =>
createStyles({
root: {
display: "flex",
flexWrap: "nowrap",
flexDirection: "row",
gap: theme.spacing(1),
},
previewText: {
display: "block",
whiteSpace: "nowrap",
overflow: "hidden",
textOverflow: "ellipsis",
flex: 1,
},
}),
);
const useCodeDialogButtonWithPreviewStyles = (theme: Theme) => ({
root: {
display: "flex",
flexWrap: "nowrap",
flexDirection: "row",
gap: theme.spacing(1),
},
previewText: {
display: "block",
whiteSpace: "nowrap",
overflow: "hidden",
textOverflow: "ellipsis",
flex: 1,
},
});

type CodeDialogButtonWithPreviewProps = CodeDialogButtonProps & ClassNameProps;
/**
Expand All @@ -99,9 +101,10 @@ export const CodeDialogButtonWithPreview = ({
code,
buttonText,
className,
sx,
...props
}: CodeDialogButtonWithPreviewProps) => {
const classes = useCodeDialogButtonWithPreviewStyles();
const styles = useCodeDialogButtonWithPreviewStyles(useTheme());

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There shouldn't be a need to create an object like this.

When using sx props, you can also easily get access to the theme by passing a function into sx.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In most cases, I would prefer if we inlined styles instead of creating a separate styles object, but this is not a blocker.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood, I've removed the redundant object and directly passed the function instead. Additionally, converted some parts to use inlined styles, while leaving the rest as is for readability considerations.

const codeText =
typeof code === "string"
Expand All @@ -111,13 +114,15 @@ export const CodeDialogButtonWithPreview = ({
const buttonTextToPass = buttonText ?? "Expand";

return (
<div className={classNames(classes.root, className)}>
<span className={classes.previewText}>{codeText}</span>
<Box className={className} sx={Object.assign({}, styles.root, sx)}>
<Box component="span" sx={styles.previewText}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<Box className={className} sx={Object.assign({}, styles.root, sx)}>
<Box className={className} sx={[styles.root, sx]}>

Please use array syntax instead

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! Made a minor adjustment to sx={[styles.root, ...(Array.isArray(sx) ? sx : [sx])]} to ensure the type matches.

{codeText}
</Box>
<CodeDialogButton
code={codeText}
buttonText={buttonTextToPass}
{...props}
/>
</div>
</Box>
);
};
77 changes: 32 additions & 45 deletions dashboard/client/src/common/CollapsibleSection.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
import { Box, Typography } from "@mui/material";
import createStyles from "@mui/styles/createStyles";
import makeStyles from "@mui/styles/makeStyles";
import classNames from "classnames";
import { Box, SxProps, Theme, Typography, useTheme } from "@mui/material";
import React, {
forwardRef,
PropsWithChildren,
Expand All @@ -11,24 +8,22 @@ import React, {
import { RiArrowDownSLine, RiArrowRightSLine } from "react-icons/ri";
import { ClassNameProps } from "./props";

const useStyles = makeStyles((theme) =>
createStyles({
title: {
display: "flex",
flexDirection: "row",
flexWrap: "nowrap",
alignItems: "center",
fontWeight: 500,
cursor: "pointer",
marginRight: theme.spacing(1),
},
icon: {
marginRight: theme.spacing(1),
width: 24,
height: 24,
},
}),
);
const useStyles = (theme: Theme) => ({
title: {
display: "flex",
flexDirection: "row",
flexWrap: "nowrap",
alignItems: "center",
fontWeight: 500,
cursor: "pointer",
marginRight: theme.spacing(1),
},
icon: {
marginRight: theme.spacing(1),
width: 24,
height: 24,
},
});

type CollapsibleSectionProps = PropsWithChildren<
{
Expand All @@ -49,6 +44,7 @@ type CollapsibleSectionProps = PropsWithChildren<
* When enabled, we will keep the content around when collapsing but hide it via css.
*/
keepRendered?: boolean;
sx?: SxProps<Theme>;
} & ClassNameProps
>;

Expand All @@ -66,10 +62,11 @@ export const CollapsibleSection = forwardRef<
children,
keepRendered,
icon,
sx,
},
ref,
) => {
const classes = useStyles();
const styles = useStyles(useTheme());
const [internalExpanded, setInternalExpanded] = useState(startExpanded);
const finalExpanded = expanded !== undefined ? expanded : internalExpanded;

Expand All @@ -79,17 +76,17 @@ export const CollapsibleSection = forwardRef<
};

return (
<div ref={ref} className={className}>
<Box ref={ref} className={className} sx={sx}>
<Box display="flex" flexDirection="row" alignItems="center">
<Typography
className={classes.title}
sx={styles.title}
variant="h4"
onClick={handleExpandClick}
>
{finalExpanded ? (
<RiArrowDownSLine className={classes.icon} />
<Box component={RiArrowDownSLine} sx={styles.icon} />
) : (
<RiArrowRightSLine className={classes.icon} />
<Box component={RiArrowRightSLine} sx={styles.icon} />
)}
{title}
</Typography>
Expand All @@ -98,21 +95,17 @@ export const CollapsibleSection = forwardRef<
<HideableBlock visible={finalExpanded} keepRendered={keepRendered}>
{children}
</HideableBlock>
</div>
</Box>
);
},
);

const useHideableBlockStyles = makeStyles((theme) =>
createStyles({
body: {
marginTop: theme.spacing(1),
},
bodyHidden: {
display: "none",
},
const useHideableBlockStyles = (theme: Theme) => ({
body: (hidden: boolean) => ({
marginTop: theme.spacing(1),
display: hidden ? "none" : "block",
}),
);
});

type HideableBlockProps = PropsWithChildren<
{
Expand All @@ -135,7 +128,7 @@ export const HideableBlock = ({
keepRendered,
children,
}: HideableBlockProps) => {
const classes = useHideableBlockStyles();
const styles = useHideableBlockStyles(useTheme());

// visible represents whether the component is viewable in the browser.
// Rendered represents whether the DOM elements exist in the DOM tree.
Expand All @@ -152,12 +145,6 @@ export const HideableBlock = ({
// Optimization to keep the component rendered (but not visible) when hidden
// to avoid re-rendering when component is shown again.
return visible || (keepRendered && rendered) ? (
<div
className={classNames(classes.body, {
[classes.bodyHidden]: !visible,
})}
>
{children}
</div>
<Box sx={styles.body(!visible)}>{children}</Box>
) : null;
};
Loading
Loading