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

frontend: Refactor workflows to use new spacing units #3160

Merged
merged 25 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
31efcab
frotnend: Adjust workflow layout whitespace
septum Oct 22, 2024
238c1c7
frontend: Refactor workflows to use new layout and spacing units
septum Oct 23, 2024
b3c8a49
Wrap children in fragment
septum Oct 23, 2024
04b18ed
Remove app configuration
septum Oct 23, 2024
bee2814
Merge 'main' branch into workflow-layout-refactoring
septum Oct 23, 2024
084705e
Fix failing CI jobs
septum Oct 23, 2024
cecc531
Rename flag to display breadcrumbs
septum Oct 28, 2024
f4276e1
Fix issues stated in code review
septum Oct 29, 2024
e7057b8
frontend: Refactor workflows to use new spacing units
septum Oct 29, 2024
95f2b1a
Fix issues stated in code review
septum Oct 31, 2024
bc11f05
Remove heading update capacity wizard
septum Oct 31, 2024
c179503
Fix issues raised in CI
septum Oct 31, 2024
b7ce854
Add alternative padding and rendering flags to theme
septum Nov 1, 2024
14688fd
Fix issues stated in code review
septum Nov 1, 2024
f541446
frontend: Include layout options in scaffolding template (#3161)
septum Nov 4, 2024
1693a21
Update unit test and snapshot
septum Nov 4, 2024
3178e30
Merge workflow-layout-refactoring into workflow-spacing-refactoring
septum Nov 4, 2024
8693a13
Fix variant logic missing a case
septum Nov 4, 2024
a311939
Merge branch 'workflow-layout-refactoring' of ssh://github.com/lyft/c…
septum Nov 5, 2024
499a9ca
frontend: Refactor theme spacing to allow units as strings (#3170)
septum Nov 5, 2024
b6d3aea
Fix issues raised in code review
septum Nov 6, 2024
36b9b0e
Handle none spacing unit
septum Nov 6, 2024
ed00a3e
Merge main into workflow-layout-refactoring
septum Nov 6, 2024
7448ac9
Merge branch 'workflow-layout-refactoring' of ssh://github.com/lyft/c…
septum Nov 6, 2024
d8bf150
Merge main into workflow-spacing-refactoring
septum Nov 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 45 additions & 30 deletions frontend/packages/core/src/Theme/theme.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,28 @@ import { clutchColors, THEME_VARIANTS } from "./colors";
import palette from "./palette";
import type { ThemeVariant } from "./types";

type SpacingUnit = "xs" | "sm" | "base" | "md" | "lg" | "xl";

// NOTE: `string & {}` allows `SpacingUnit` to be autocompleted
type SpacingArg = SpacingUnit | number | (string & {});

declare module "@emotion/react" {
export interface Theme extends MuiTheme {}
}

declare module "@mui/material/styles" {
interface Spacing {
(value: SpacingArg): string;
(topBottom: SpacingArg, rightLeft: SpacingArg): string;
(top: SpacingArg, rightLeft: SpacingArg, bottom: SpacingArg): string;
(top: SpacingArg, right: SpacingArg, bottom: SpacingArg, left: SpacingArg): string;
}

interface Theme {
spacing: Spacing;
clutch: {
useWorkflowLayout: boolean;
spacing: {
none: number;
xs: number;
sm: number;
base: number;
md: number;
lg: number;
xl: number;
};
spacing: Record<SpacingUnit, string>;
layout: {
gutter: string;
};
Expand All @@ -36,41 +41,51 @@ declare module "@mui/material/styles" {
interface ThemeOptions {
clutch: {
useWorkflowLayout: boolean;
spacing: {
none: number;
xs: number;
sm: number;
base: number;
md: number;
lg: number;
xl: number;
};
spacing: Record<SpacingUnit, string>;
layout: {
gutter: string;
};
};
}
}

// `8` is the default scaling factor in MUI, so we define and use it
// to allow predictability using a `number` value
// https://v5.mui.com/material-ui/customization/spacing/
const DEFAULT_SCALING_FACTOR = 8;

const CLUTCH_CUSTOM_SPACING: Record<SpacingUnit, string> = {
xs: "4px",
sm: "8px",
base: "16px",
md: "24px",
lg: "32px",
xl: "40px",
};

// Create a Material UI theme is propagated to all children.
const createTheme = (variant: ThemeVariant, useWorkflowLayout: boolean): MuiTheme => {
return createMuiTheme({
colors: clutchColors(variant),
palette: palette(variant),
// `8` is the default scaling factor in MUI, we are setting it again to make it explicit
// https://v5.mui.com/material-ui/customization/spacing/
spacing: 8,
spacing: (...args) => {
const spacingValues = args.map(value => {
if (typeof value === "number") {
return `${value * DEFAULT_SCALING_FACTOR}px`;
}

if (typeof value === "string" && CLUTCH_CUSTOM_SPACING[value] !== undefined) {
return CLUTCH_CUSTOM_SPACING[value];
}

return value;
});

return spacingValues.join(" ");
},
clutch: {
useWorkflowLayout,
spacing: {
none: 0,
xs: 0.5,
sm: 1,
base: 2,
md: 3,
lg: 4,
xl: 5,
},
spacing: { ...CLUTCH_CUSTOM_SPACING },
layout: {
gutter: useWorkflowLayout ? "0px" : "24px",
},
Expand Down
17 changes: 7 additions & 10 deletions frontend/packages/core/src/WorkflowLayout/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ const getContainerVariantStyles = (variant: LayoutVariant, theme: Theme) => {
const layoutVariantStylesMap: { [key in LayoutVariant]: CSSObject } = {
standard: {
...BASE_CONTAINER_STYLES,
padding: theme.spacing(theme.clutch.spacing.md),
padding: theme.spacing("md"),
},
wizard: {
...BASE_CONTAINER_STYLES,
width: "800px", // Taken from the Wizard Component default width
padding: theme.spacing(theme.clutch.spacing.lg, theme.clutch.spacing.none),
margin: theme.spacing(theme.clutch.spacing.none, "auto"),
padding: theme.spacing("lg", "none"),
margin: theme.spacing("none", "auto"),
},
};
return layoutVariantStylesMap[variant];
Expand All @@ -56,23 +56,20 @@ const LayoutContainer = styled("div")(
);

const PageHeader = styled("div")(({ $variant, theme }: StyledVariantComponentProps) => ({
padding: theme.spacing(
theme.clutch.spacing.none,
$variant === "wizard" ? theme.clutch.spacing.md : theme.clutch.spacing.none
),
paddingBottom: theme.spacing(theme.clutch.spacing.base),
padding: theme.spacing("none", $variant === "wizard" ? "md" : "none"),
paddingBottom: theme.spacing("base"),
width: "100%",
}));

const PageHeaderBreadcrumbsWrapper = styled("div")(({ theme }: { theme: Theme }) => ({
marginBottom: theme.spacing(theme.clutch.spacing.xs),
marginBottom: theme.spacing("xs"),
}));

const PageHeaderMainContainer = styled("div")(({ theme }: { theme: Theme }) => ({
display: "flex",
alignItems: "center",
height: "70px",
marginBottom: theme.spacing(theme.clutch.spacing.sm),
marginBottom: theme.spacing("sm"),
}));

const PageHeaderInformation = styled("div")({
Expand Down
4 changes: 2 additions & 2 deletions frontend/workflows/audit/src/logs/event-row.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const ENDPOINT = "/v1/audit/getEvents";
const COLUMN_COUNT = 6;
const MonospaceText = styled("div")(({ theme }: { theme: Theme }) => ({
fontFamily: "monospace",
padding: theme.spacing(theme.clutch.spacing.sm),
padding: theme.spacing("sm"),
border: "1px solid lightgray",
borderRadius: "8px",
background: theme.palette.secondary[200],
Expand All @@ -29,7 +29,7 @@ const MonospaceText = styled("div")(({ theme }: { theme: Theme }) => ({
}));

const ReactJsonWrapper = styled("div")(({ theme }: { theme: Theme }) => ({
padding: theme.spacing(theme.clutch.spacing.sm),
padding: theme.spacing("sm"),
}));

interface EventRowAction {
Expand Down
2 changes: 1 addition & 1 deletion frontend/workflows/envoy/src/remote-triage/dashboard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ const SummariesContainer = styled(Grid)({
});

const InformationContainer = styled("div")(({ theme }: { theme: Theme }) => ({
padding: theme.spacing(theme.clutch.spacing.base, theme.clutch.spacing.none),
padding: theme.spacing("base", "none"),
}));

interface DashboardProps {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type { Theme } from "@mui/material";

const Container = styled("div")(({ theme }: { theme: Theme }) => ({
"> *": {
padding: theme.spacing(theme.clutch.spacing.sm, theme.clutch.spacing.none),
padding: theme.spacing("sm", "none"),
},
}));

Expand Down
2 changes: 1 addition & 1 deletion frontend/workflows/k8s/src/k8s-dash-header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const Category = styled("div")(({ theme }: { theme: Theme }) => ({
lineHeight: "16px",
color: alpha(theme.palette.secondary[900], 0.38),
textTransform: "uppercase",
paddingBottom: theme.spacing(theme.clutch.spacing.sm),
paddingBottom: theme.spacing("sm"),
}));

const HeaderText = styled("div")(({ theme }: { theme: Theme }) => ({
Expand Down
4 changes: 2 additions & 2 deletions frontend/workflows/k8s/src/k8s-dash-search.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import type { Theme } from "@mui/material";
import * as yup from "yup";

const Container = styled.div(({ theme }: { theme: Theme }) => ({
margin: theme.spacing(theme.clutch.spacing.lg, theme.clutch.spacing.none),
margin: theme.spacing("lg", "none"),
}));

const schema = yup.object().shape({
Expand All @@ -27,7 +27,7 @@ const schema = yup.object().shape({
});

const Content = styled.div(({ theme }: { theme: Theme }) => ({
margin: theme.spacing(theme.clutch.spacing.lg, theme.clutch.spacing.none),
margin: theme.spacing("lg", "none"),
}));

const K8sDashSearch = ({ onSubmit }) => {
Expand Down
8 changes: 4 additions & 4 deletions frontend/workflows/k8s/src/k8s-dashboard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,16 @@ const Container = styled("div")(({ theme }: { theme: Theme }) => ({
display: "flex",
flexDirection: "column",
"> *:first-child": {
margin: theme.spacing(theme.clutch.spacing.none),
margin: theme.spacing("none"),
},
}));

const Content = styled("div")(({ theme }: { theme: Theme }) => ({
margin: theme.spacing(theme.clutch.spacing.lg, theme.clutch.spacing.none),
margin: theme.spacing("lg", "none"),
}));

const PlaceholderTitle = styled("div")(({ theme }: { theme: Theme }) => ({
paddingBottom: theme.spacing(theme.clutch.spacing.base),
paddingBottom: theme.spacing("base"),
fontWeight: 700,
fontSize: "22px",
lineHeight: "28px",
Expand All @@ -46,7 +46,7 @@ const PlaceholderText = styled("div")(({ theme }: { theme: Theme }) => ({
}));

const PlaceholderWrapper = styled("div")(({ theme }: { theme: Theme }) => ({
margin: theme.spacing(theme.clutch.spacing.lg),
margin: theme.spacing("lg"),
textAlign: "center",
}));

Expand Down
2 changes: 1 addition & 1 deletion frontend/workflows/kinesis/src/update-shard-count.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const Form = styled.form(({ theme }: { theme: Theme }) => ({
flexDirection: "column",
justifyItems: "space-evenly",
"> *": {
padding: theme.spacing(theme.clutch.spacing.sm, theme.clutch.spacing.none),
padding: theme.spacing("sm", "none"),
},
}));

Expand Down
10 changes: 5 additions & 5 deletions frontend/workflows/projectCatalog/src/catalog/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const initialState: CatalogState = {
};

const PlaceholderContainer = styled("div")(({ theme }: { theme: Theme }) => ({
margin: theme.spacing(theme.clutch.spacing.lg),
margin: theme.spacing("lg"),
}));

const Placeholder = () => (
Expand Down Expand Up @@ -61,20 +61,20 @@ const autoComplete = async (search: string): Promise<any> => {
};

const FormWrapper = styled("div")(({ theme }: { theme: Theme }) => ({
margin: theme.spacing(theme.clutch.spacing.base),
margin: theme.spacing("base"),
}));

const Form = styled.form({});

const MainContentWrapper = styled("div")(({ theme }: { theme: Theme }) => ({
display: "flex",
justifyContent: "space-between",
marginBottom: theme.spacing(theme.clutch.spacing.base),
marginTop: theme.spacing(theme.clutch.spacing.lg),
marginBottom: theme.spacing("base"),
marginTop: theme.spacing("lg"),
}));

const PlaceholderWrapper = styled(Grid)(({ theme }: { theme: Theme }) => ({
paddingTop: theme.spacing(theme.clutch.spacing.lg),
paddingTop: theme.spacing("lg"),
}));

const Catalog: React.FC<WorkflowProps> = ({ allowDisabled }) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,16 @@ interface CardProps extends CatalogDetailsCard, BaseCardProps {}
const StyledCard = styled(Card)(({ theme }: { theme: Theme }) => ({
width: "100%",
height: "100%",
padding: theme.spacing(theme.clutch.spacing.base),
padding: theme.spacing("base"),
}));

const StyledGrid = styled(Grid)({
height: "fit-content",
});

const StyledProgressContainer = styled("div")(({ theme }: { theme: Theme }) => ({
marginBottom: theme.spacing(theme.clutch.spacing.sm),
marginTop: theme.spacing(-theme.clutch.spacing.base),
marginBottom: theme.spacing("sm"),
marginTop: theme.spacing("-16px"),
septum marked this conversation as resolved.
Show resolved Hide resolved
height: "4px",
".MuiLinearProgress-root": {
backgroundColor: theme.palette.primary[400],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ const StyledPopperItem = styled(PopperItem)(({ theme }: { theme: Theme }) => ({
height: "auto",
},
"& span.MuiTypography-root": {
padding: theme.spacing(theme.clutch.spacing.none),
padding: theme.spacing("none"),
},
"& a.MuiTypography-root": {
padding: theme.spacing(theme.clutch.spacing.xs, theme.clutch.spacing.base),
padding: theme.spacing("xs", "base"),
},
}));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ interface ProjectInfoProps {
}

const StyledRow = styled(Grid)(({ theme }: { theme: Theme }) => ({
marginBottom: theme.spacing(theme.clutch.spacing.base),
marginBottom: theme.spacing("base"),
whiteSpace: "nowrap",
width: "100%",
}));
Expand Down
Loading