Skip to content

Commit

Permalink
Visual improvements to the job sidebar (#4133)
Browse files Browse the repository at this point in the history
This change makes the sidebar for viewing a job's information visually more consistent across its tabs and the rest of the application. We also migrate to using styled components using emotion JS, which is provided by MUI, instead of separate CSS files. This provides better type-safety and keeps styles in the same place as the components. Note that this has necessitated an upgrade to vitest, and the installation and use of @emotion/babel-plugin.
  • Loading branch information
mauriceyap authored Jan 14, 2025
1 parent 419484d commit b2d99ad
Show file tree
Hide file tree
Showing 26 changed files with 1,782 additions and 2,249 deletions.
3 changes: 2 additions & 1 deletion internal/lookout/ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
]
},
"devDependencies": {
"@emotion/babel-plugin": "^11.13.5",
"@eslint/compat": "^1.2.4",
"@tanstack/eslint-plugin-query": "^5.62.1",
"@testing-library/dom": "^10.4.0",
Expand Down Expand Up @@ -96,7 +97,7 @@
"prettier": "^3.4.2",
"typescript": "^4.9.3",
"vite": "^6.0.3",
"vitest": "^2.1.8"
"vitest": "^3.0.0-beta.4"
},
"resolutions": {
"@types/react": "^18",
Expand Down
87 changes: 74 additions & 13 deletions internal/lookout/ui/src/components/CodeBlock.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useCallback } from "react"

import { Download } from "@mui/icons-material"
import { IconButton, styled, useColorScheme } from "@mui/material"
import { IconButton, Skeleton, styled, useColorScheme } from "@mui/material"
import { Highlight, themes } from "prism-react-renderer"
import Prism from "prismjs"
import "prismjs/components/prism-bash"
Expand All @@ -13,6 +13,9 @@ import { CopyIconButton } from "./CopyIconButton"
// import "prismjs/components/prism-{language}"
type SupportedLanguage = "bash" | "yaml"

const DEFAULT_LOADING_LINES = 20
const DEFAULT_LOADING_LINE_LENGTH = 80

const DARK_PRISM_THEME = themes.oneDark
const LIGHT_PRISM_THEME = themes.oneLight

Expand Down Expand Up @@ -69,31 +72,55 @@ const CodeLineNumber = styled("span")({
},
})

export type CodeBlockProps = {
language: SupportedLanguage | "text"
interface CodeBlockLoadingProps {
loading: true
code?: undefined | string
language?: undefined | SupportedLanguage | "text"
}

interface CodeBlockLoadedProps {
loading: false
code: string
language: SupportedLanguage | "text"
}

interface CodeBlockDownloadbaleProps {
downloadable: true
downloadBlobType: string
downloadFileName: string
}

interface CodeBlockNonDownloadbaleProps {
downloadable: false
downloadBlobType?: undefined | string
downloadFileName?: undefined | string
}

interface CodeBlockbaseProps {
showLineNumbers: boolean
} & (
| {
downloadable: true
downloadBlobType: string
downloadFileName: string
}
| { downloadable: false; downloadBlobType?: undefined | string; downloadFileName?: undefined | string }
)
loadingLines?: number
loadingLineLength?: number
}

export type CodeBlockProps = CodeBlockbaseProps &
(CodeBlockLoadedProps | CodeBlockLoadingProps) &
(CodeBlockDownloadbaleProps | CodeBlockNonDownloadbaleProps)

export const CodeBlock = ({
language,
code,
showLineNumbers,
loading,
downloadable,
downloadBlobType,
downloadFileName,
showLineNumbers,
loadingLines = DEFAULT_LOADING_LINES,
loadingLineLength = DEFAULT_LOADING_LINE_LENGTH,
}: CodeBlockProps) => {
const { colorScheme } = useColorScheme()

const downloadFile = useCallback(() => {
if (!downloadable) {
if (!downloadable || loading) {
return
}

Expand All @@ -107,6 +134,40 @@ export const CodeBlock = ({
element.click()
}, [code, downloadable, downloadBlobType, downloadFileName])

if (loading) {
return (
<CodeBlockContainer>
<Highlight
prism={Prism}
theme={colorScheme === "dark" ? DARK_PRISM_THEME : LIGHT_PRISM_THEME}
language="text"
code={Array(loadingLines).fill("").join("\n")}
>
{({ style, tokens, getLineProps }) => (
<StyledPre style={style}>
<Code>
{tokens.map((line, i) =>
showLineNumbers ? (
<CodeLine key={i} {...getLineProps({ line })}>
<CodeLineNumber />
<span>
<Skeleton>{Array(loadingLineLength).fill(" ").join("")}</Skeleton>
</span>
</CodeLine>
) : (
<div key={i} {...getLineProps({ line })}>
<Skeleton>{Array(loadingLineLength).fill(" ").join("")}</Skeleton>
</div>
),
)}
</Code>
</StyledPre>
)}
</Highlight>
</CodeBlockContainer>
)
}

return (
<CodeBlockContainer>
<CodeActionsContainer className="codeActionsContainer">
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,14 +1,29 @@
import { useEffect, useMemo, useState } from "react"
import { useEffect, useMemo } from "react"

import { CircularProgress, Collapse, ListItemButton, Typography } from "@mui/material"
import {
Accordion,
AccordionDetails,
AccordionSummary,
Alert,
Paper,
Skeleton,
Stack,
styled,
Typography,
} from "@mui/material"

import styles from "./ContainerDetails.module.css"
import { KeyValuePairTable } from "./KeyValuePairTable"
import { SidebarTabSubheading } from "./sidebarTabContentComponents"
import { useCustomSnackbar } from "../../../hooks/useCustomSnackbar"
import { Job } from "../../../models/lookoutV2Models"
import { useGetJobSpec } from "../../../services/lookoutV2/useGetJobSpec"
import { SPACING } from "../../../styling/spacing"
import { CodeBlock } from "../../CodeBlock"

const LoadingContainerPaper = styled(Paper)(({ theme }) => ({
padding: theme.spacing(2),
}))

export interface ContainerData {
name: string
command: string
Expand Down Expand Up @@ -73,17 +88,16 @@ export const ContainerDetails = ({ job }: ContainerDetailsProps) => {

if (getJobSpecResult.status === "pending") {
return (
<div className={styles.container + " " + styles.centerContent}>
<CircularProgress />
</div>
<LoadingContainerPaper variant="outlined">
<Skeleton />
</LoadingContainerPaper>
)
}
if (containers.length === 0) {
return <>No containers found.</>
return <Alert severity="info">No containers found.</Alert>
}
return (
<div className={styles.container}>
<Typography>Containers</Typography>
<div>
{containers.map((c, i) => (
<SingleContainerDetails key={i} container={c} openByDefault={i === 0} />
))}
Expand All @@ -92,23 +106,19 @@ export const ContainerDetails = ({ job }: ContainerDetailsProps) => {
}

const SingleContainerDetails = ({ container, openByDefault }: { container: ContainerData; openByDefault: boolean }) => {
const [open, setOpen] = useState<boolean>(openByDefault)
const entrypoint = container.command + " " + container.args

const handleClick = () => {
setOpen(!open)
}

const containerName = container.name !== "" ? container.name : "No name"

return (
<>
<ListItemButton onClick={handleClick}>{containerName}</ListItemButton>
<Collapse in={open}>
<div className={styles.singleContainer}>
<div className={styles.commandContainer}>
<Accordion defaultExpanded={openByDefault}>
<AccordionSummary>
<SidebarTabSubheading>{containerName}</SidebarTabSubheading>
</AccordionSummary>
<AccordionDetails>
<Stack spacing={SPACING.sm}>
<div>
<Typography>Command</Typography>
<CodeBlock code={entrypoint} language="bash" downloadable={false} showLineNumbers={false} />
<CodeBlock code={entrypoint} language="bash" downloadable={false} showLineNumbers={false} loading={false} />
</div>
<div>
<Typography>Resources</Typography>
Expand All @@ -131,8 +141,8 @@ const SingleContainerDetails = ({ container, openByDefault }: { container: Conta
]}
/>
</div>
</div>
</Collapse>
</>
</Stack>
</AccordionDetails>
</Accordion>
)
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import { Table, TableBody, TableCell, TableRow, Link } from "@mui/material"
import { Table, TableBody, TableCell, TableRow, Link, styled } from "@mui/material"
import validator from "validator"

import styles from "./KeyValuePairTable.module.css"
import { ActionableValueOnHover } from "../../ActionableValueOnHover"

const StyledTableCell = styled(TableCell)({
width: "50%",
wordBreak: "break-all",
})

export interface KeyValuePairTable {
data: {
key: string
Expand Down Expand Up @@ -35,12 +39,12 @@ export const KeyValuePairTable = ({ data }: KeyValuePairTable) => {
)
return (
<TableRow key={key}>
<TableCell className={styles.cell}>{key}</TableCell>
<TableCell className={styles.cell}>
<StyledTableCell>{key}</StyledTableCell>
<StyledTableCell>
<ActionableValueOnHover copyAction={allowCopy ? { copyContent: value } : undefined}>
{nodeToDisplay}
</ActionableValueOnHover>
</TableCell>
</StyledTableCell>
</TableRow>
)
})}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { Alert } from "@mui/material"

import { JobState } from "../../../models/lookoutV2Models"

const jobStatusMayRunInFuture: Record<JobState, boolean> = {
[JobState.Queued]: true,
[JobState.Leased]: true,
[JobState.Pending]: true,
[JobState.Running]: true,
[JobState.Succeeded]: false,
[JobState.Failed]: false,
[JobState.Cancelled]: false,
[JobState.Preempted]: false,
[JobState.Rejected]: false,
}

export interface NoRunsAlertProps {
jobState: JobState
}

export const NoRunsAlert = ({ jobState }: NoRunsAlertProps) => (
<Alert severity="info">{jobStatusMayRunInFuture[jobState] ? "This job has not run." : "This job did not run."}</Alert>
)

This file was deleted.

Loading

0 comments on commit b2d99ad

Please sign in to comment.