Skip to content

Commit

Permalink
Align aggregate counts for job statuses
Browse files Browse the repository at this point in the history
On the Lookout UI, when grouping is enabled for the jobs table, change
how counts for different statuses are displayed, so that they are
aligned and are less visually noisy. This change is in response to user
feedback.
  • Loading branch information
Maurice Yap authored and GitHub Enterprise committed Jan 6, 2025
1 parent 6e5c1eb commit d1a7ffd
Show file tree
Hide file tree
Showing 14 changed files with 282 additions and 96 deletions.
9 changes: 7 additions & 2 deletions internal/lookout/ui/src/components/ActionableValueOnHover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,17 @@ import { CopyIconButton } from "./CopyIconButton"
import { AddFilter } from "./icons"

const OuterContainer = styled("div")({
display: "flex",
display: "inline-flex",
width: "100%",
flexDirection: "row",
alignItems: "center",
gap: "0.5ch",
})

const ContentContainer = styled("div")<{ minWidth: boolean }>(({ minWidth }) => ({
flexGrow: minWidth ? undefined : 1,
}))

const StyledIconButton = styled(IconButton)<IconButtonProps & { hidden: boolean }>(({ hidden }) => ({
visibility: hidden ? "hidden" : "unset",
}))
Expand Down Expand Up @@ -40,7 +45,7 @@ export const ActionableValueOnHover = ({
const [hovering, setHovering] = useState(false)
return (
<OuterContainer onMouseEnter={() => setHovering(true)} onMouseLeave={() => setHovering(false)}>
<div>{children}</div>
<ContentContainer minWidth={Boolean(copyAction || filterAction)}>{children}</ContentContainer>
{copyAction && (
<div>
<CopyIconButton
Expand Down
8 changes: 7 additions & 1 deletion internal/lookout/ui/src/components/CopyIconButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,13 @@ export const CopyIconButton = ({ content, size, onClick, hidden = false }: CopyI
const [tooltipOpen, setTooltipOpen] = useState(false)

return (
<Tooltip title="Copied!" onClose={() => setTooltipOpen(false)} open={tooltipOpen} leaveDelay={LEAVE_DELAY_MS}>
<Tooltip
title="Copied!"
onClose={() => setTooltipOpen(false)}
open={tooltipOpen}
leaveDelay={LEAVE_DELAY_MS}
arrow={false}
>
<StyledIconButton
size={size}
onClick={(e) => {
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,37 +1,52 @@
import { Chip, Tooltip, Typography } from "@mui/material"
import { Chip, styled, Tooltip } from "@mui/material"

import styles from "./JobGroupStateCounts.module.css"
import { JobState, jobStateColors, jobStateIcons } from "../../models/lookoutV2Models"
import { JobState, jobStateColors } from "../../models/lookoutV2Models"
import { formatJobState } from "../../utils/jobsTableFormatters"

const CountsContainer = styled("div")({
display: "grid",
gridAutoColumns: "minmax(0, 1fr)",
gridAutoFlow: "column",
textAlign: "center",
})

const StateCountChip = styled(Chip)({
width: "100%",
borderRadius: 0,

"& .MuiChip-label": {
overflow: "visible",
},
})

interface JobGroupStateCountsProps {
jobStatesToDisplay?: JobState[]
stateCounts: Partial<Record<JobState, number>>
}

export const JobGroupStateCounts = ({ stateCounts }: JobGroupStateCountsProps) => (
<div className={styles.container}>
{Object.values(JobState)
.flatMap(
(jobState) => (jobState in stateCounts ? [[jobState, stateCounts[jobState]]] : []) as [JobState, number][],
)
.map(([_jobState, count]) => {
const jobState = _jobState as JobState
const Icon = jobStateIcons[jobState]
return (
<Tooltip key={jobState} title={formatJobState(jobState)}>
<Chip
size="small"
variant="outlined"
label={
<Typography fontSize="inherit" color="text.primary" fontWeight="medium">
{count.toString()}
</Typography>
}
color={jobStateColors[jobState]}
icon={<Icon />}
/>
export const JobGroupStateCounts = ({
stateCounts,
jobStatesToDisplay = Object.values(JobState),
}: JobGroupStateCountsProps) => (
<CountsContainer>
{Object.values(JobState).map((_jobState) => {
const jobState = _jobState as JobState
const count = stateCounts[jobState] ?? 0
return (
jobStatesToDisplay.includes(jobState) && (
<Tooltip key={jobState} title={`${formatJobState(jobState)} (${count.toString()})`}>
<div>
<StateCountChip
size="small"
variant="shaded"
label={count.toString()}
color={jobStateColors[jobState]}
disabled={count === 0}
/>
</div>
</Tooltip>
)
})}
</div>
)
})}
</CountsContainer>
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { styled, Tooltip } from "@mui/material"

import { JobState, jobStateColors, jobStateIcons } from "../../models/lookoutV2Models"
import { formatJobState } from "../../utils/jobsTableFormatters"

const OuterContainer = styled("div")({
display: "grid",
gridAutoColumns: "minmax(0, 1fr)",
gridAutoFlow: "column",
textAlign: "center",
})

export interface JobGroupStateCountsColumnHeaderProps {
jobStatesToDisplay?: JobState[]
}

export const JobGroupStateCountsColumnHeader = ({
jobStatesToDisplay = Object.values(JobState),
}: JobGroupStateCountsColumnHeaderProps) => (
<OuterContainer>
{Object.values(JobState).map((_jobState) => {
const jobState = _jobState as JobState
const Icon = jobStateIcons[jobState]
return (
jobStatesToDisplay.includes(jobState) && (
<Tooltip key={jobState} title={formatJobState(jobState)} placement="top">
<div>
<Icon fontSize="inherit" color={jobStateColors[jobState]} />
</div>
</Tooltip>
)
)
})}
</OuterContainer>
)
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,7 @@ export const JobStateChip = ({ state }: JobStateChipProps) => {
return null
}
const Icon = jobStateIcons[state]
return <Chip label={formatJobState(state)} size="small" color={jobStateColors[state]} icon={<Icon />} />
return (
<Chip label={formatJobState(state)} size="small" color={jobStateColors[state]} icon={<Icon />} variant="shaded" />
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export const JobStateCountChip = ({ state, count, onClick }: JobStateCountChipPr
const label = count.toString()

return count > 0 ? (
<StyledChip label={label} color={jobStateColors[state]} clickable onClick={onClick} variant="filled" size="small" />
<StyledChip label={label} color={jobStateColors[state]} clickable onClick={onClick} variant="shaded" size="small" />
) : (
<>{label}</>
)
Expand Down
124 changes: 76 additions & 48 deletions internal/lookout/ui/src/components/lookoutV2/JobsTableCell.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { RefObject } from "react"
import { ReactNode, RefObject } from "react"

import { KeyboardArrowRight, KeyboardArrowDown } from "@mui/icons-material"
import { TableCell, IconButton, TableSortLabel, Box, styled } from "@mui/material"
Expand All @@ -7,10 +7,11 @@ import { Cell, ColumnResizeMode, flexRender, Header, Row } from "@tanstack/react
import styles from "./JobsTableCell.module.css"
import { JobsTableFilter } from "./JobsTableFilter"
import { JobRow, JobTableRow } from "../../models/jobsTableModels"
import { Match } from "../../models/lookoutV2Models"
import { FilterType, getColumnMetadata, toColId } from "../../utils/jobsTableColumns"
import { JobState, Match } from "../../models/lookoutV2Models"
import { ColumnId, FilterType, getColumnMetadata, StandardColumnId, toColId } from "../../utils/jobsTableColumns"
import { matchForColumn } from "../../utils/jobsTableUtils"
import { ActionableValueOnHover } from "../ActionableValueOnHover"
import { JobGroupStateCountsColumnHeader } from "./JobGroupStateCountsColumnHeader"

const sharedCellStyle = {
padding: 0,
Expand All @@ -19,14 +20,14 @@ const sharedCellStyle = {
whiteSpace: "nowrap",
overflow: "hidden",
borderRight: "1px solid #cccccc",
}
} as const

const sharedCellStyleWithOpacity = {
...sharedCellStyle,
"&:hover": {
opacity: 0.85,
},
}
} as const

const HeaderTableCell = styled(TableCell)(({ theme }) => ({
backgroundColor: theme.palette.grey[200],
Expand All @@ -35,6 +36,13 @@ const HeaderTableCell = styled(TableCell)(({ theme }) => ({
}),
}))

const JobGroupStateCountsColumnHeaderContainer = styled("div")({
marginTop: 5,
// Body cells have 8px horizontal padding while header cells have 10px. -2px margin on this container adjusts for this
marginLeft: -2,
marginRight: -2 - 5, // -5px to adjust for the column resizer
})

export interface HeaderCellProps {
header: Header<JobRow, unknown>
columnResizeMode: ColumnResizeMode
Expand All @@ -43,6 +51,7 @@ export interface HeaderCellProps {
parseError: string | undefined
onColumnMatchChange: (columnId: string, newMatch: Match) => void
onSetTextFieldRef: (ref: RefObject<HTMLInputElement>) => void
groupedColumns: ColumnId[]
}

export function HeaderCell({
Expand All @@ -53,6 +62,7 @@ export function HeaderCell({
parseError,
onColumnMatchChange,
onSetTextFieldRef,
groupedColumns,
}: HeaderCellProps) {
const id = toColId(header.id)
const columnDef = header.column.columnDef
Expand Down Expand Up @@ -178,6 +188,15 @@ export function HeaderCell({
onSetTextFieldRef={onSetTextFieldRef}
/>
)}

{header.column.id === StandardColumnId.State &&
groupedColumns.filter((id) => id !== StandardColumnId.State)?.length > 0 && (
<JobGroupStateCountsColumnHeaderContainer>
<JobGroupStateCountsColumnHeader
jobStatesToDisplay={header.column.getFilterValue() as JobState[] | undefined}
/>
</JobGroupStateCountsColumnHeaderContainer>
)}
</div>
<div
{...{
Expand All @@ -197,28 +216,28 @@ export function HeaderCell({
)
}

const BodyTableCell = styled(TableCell)({
...sharedCellStyleWithOpacity,
padding: "2px 8px 2px 8px",
})

export interface BodyCellProps {
cell: Cell<JobRow, unknown>
rowIsGroup: boolean
rowIsExpanded: boolean
onExpandedChange: () => void
onClickRowCheckbox: (row: Row<JobTableRow>) => void
}

export const BodyCell = ({ cell, rowIsGroup, rowIsExpanded, onExpandedChange, onClickRowCheckbox }: BodyCellProps) => {
const columnMetadata = getColumnMetadata(cell.column.columnDef)
const cellHasValue = cell.renderValue()
const isRightAligned = columnMetadata.isRightAligned ?? false
return (
<TableCell
key={cell.id}
align={isRightAligned ? "right" : "left"}
sx={{
...sharedCellStyleWithOpacity,
padding: "2px 8px 2px 8px",
}}
>
{rowIsGroup && cell.column.getIsGrouped() && cellHasValue ? (
// If it's a grouped cell, add an expander and row count

const cellContent = ((): ReactNode => {
// If it's a grouped cell, add an expander and row count
if (rowIsGroup && cell.column.getIsGrouped() && cellHasValue) {
return (
<Box
sx={{
display: "flex",
Expand Down Expand Up @@ -251,39 +270,48 @@ export const BodyCell = ({ cell, rowIsGroup, rowIsExpanded, onExpandedChange, on
{flexRender(cell.column.columnDef.cell, cell.getContext())}
</div>
</Box>
) : cell.getIsAggregated() ? (
// If the cell is aggregated, use the Aggregated
// renderer for cell
flexRender(cell.column.columnDef.aggregatedCell ?? cell.column.columnDef.cell, {
)
}

// If the cell is aggregated, use the Aggregated renderer for cell
if (cell.getIsAggregated()) {
return flexRender(cell.column.columnDef.aggregatedCell ?? cell.column.columnDef.cell, {
...cell.getContext(),
onClickRowCheckbox,
})
}

return (
<ActionableValueOnHover
stopPropogationOnActionClick
copyAction={
!rowIsGroup && Boolean(cell.getValue()) && columnMetadata.allowCopy
? { copyContent: String(cell.getValue()) }
: undefined
}
filterAction={
rowIsGroup || !cell.getValue() || columnMetadata.filterType === undefined
? undefined
: {
onFilter: () => {
cell.column.setFilterValue(
columnMetadata.filterType === FilterType.Enum ? [cell.getValue()] : cell.getValue(),
)
},
}
}
>
{flexRender(cell.column.columnDef.cell, {
...cell.getContext(),
onClickRowCheckbox,
})
) : (
<ActionableValueOnHover
stopPropogationOnActionClick
copyAction={
!rowIsGroup && Boolean(cell.getValue()) && columnMetadata.allowCopy
? { copyContent: String(cell.getValue()) }
: undefined
}
filterAction={
rowIsGroup || !cell.getValue() || columnMetadata.filterType === undefined
? undefined
: {
onFilter: () => {
cell.column.setFilterValue(
columnMetadata.filterType === FilterType.Enum ? [cell.getValue()] : cell.getValue(),
)
},
}
}
>
{flexRender(cell.column.columnDef.cell, {
...cell.getContext(),
onClickRowCheckbox,
})}
</ActionableValueOnHover>
)}
</TableCell>
})}
</ActionableValueOnHover>
)
})()

return (
<BodyTableCell key={cell.id} align={isRightAligned ? "right" : "left"}>
{cellContent}
</BodyTableCell>
)
}
Loading

0 comments on commit d1a7ffd

Please sign in to comment.