mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.96k stars 32.27k forks source link

[Stack] allow spacing prop to use token strings #37945

Open Zach-Jaensch opened 1 year ago

Zach-Jaensch commented 1 year ago

Duplicates

Latest version

Summary πŸ’‘

The current implementation of the spacing prop in Stack, will only use the custom spacing function if the value is a number. A small change could be made that would allow string tokens to be used instead.

This could help development teams in guiding them as to which token to use, and can avoid larger refactors (by this I mean many small changes) if a new spacing value is added to the spacing design token in the future.

Examples 🌈

Providing the following spacing design token

const spacingMap = {
  xl: '2rem',
  l: '1.25rem',
  m: '1rem',
  s: '0.5rem',
  xs: '0.25rem',
};

// Create inverse value for when needed
Object.entries(spacingMap).forEach(([key, value]) => {
  spacingMap[('-' + key)] = '-' + value;
});

and custom spacing

function spacing(abs: number | string): string {
  if (abs in spacingMap) {
    return spacingMap[abs];
  }
  throw new Error(`Invalid spacing value: ${abs}`);
}

export const theme = createTheme({
  spacing,
});

You could use the Stack component (and other components that use a spacing like prop)

<Stack spacing="l" />

Currently this will use the string as the literal value instead of using the custom spacing function

image

Motivation πŸ”¦

In my teams project, we are building up our design system, based on MUI.

We would like to keep the code as close to the design, while also providing a clear description in the usage and are using non-index based tokens (ie, size values) to accomplish this.

Enabling this functionality could allow teams to choose the representation that works for them, like the size values work.

Zach-Jaensch commented 1 year ago

If it helps, I will also be more then happy to try my hands at a PR

oliviertassinari commented 1 year ago

Please provide a minimal reproduction test case with the latest version. This would help a lot πŸ‘·. A live example would be perfect. This codesandbox.io template may be a good starting point. Thank you!

I would expect this to already work, it's strange, looks like a bug.

Zach-Jaensch commented 1 year ago

Sorry, I should have started out with a live example. https://codesandbox.io/s/delicate-hooks-p8snrw (I hope that works, I rarely use it if not, I've pasted the code below)

App.ts code
import * as React from "react";
import { Stack, Box, createTheme, ThemeProvider } from "@mui/material";

const spacingMap = {
  xl: "2rem",
  l: "1.25rem",
  m: "1rem",
  s: "0.5rem",
  xs: "0.25rem"
};

// Create inverse value for when needed
Object.entries(spacingMap).forEach(([key, value]) => {
  spacingMap["-" + key] = "-" + value;
});

export default function App() {
  function spacing(abs: number | string): string {
    if (abs in spacingMap) {
      return spacingMap[abs];
    }
    throw new Error(`Invalid spacing value: ${abs}`);
  }

  const theme = createTheme({
    spacing
  });

  return (
    <ThemeProvider theme={theme}>
      <Stack spacing="m">
        <Box width="100%" height="2rem" backgroundColor="pink" />
        <Box width="100%" height="2rem" backgroundColor="red" />
        <Box width="100%" height="2rem" backgroundColor="pink" />
      </Stack>
    </ThemeProvider>
  );
}