Material-ui: [Theme] spacing function

[MarkMurphy]

I'd like to propose the following additive change to theme.spacing:

import warning from 'warning';

const unit = 8;

const spacing = (...args) => {
  if (process.env.NODE_ENV !== 'production') {
    warning(
      args.length <= 4,
      `Too many arguments, exepected 1..4, got ${args.length}`,
    );

    args.forEach((arg, index) => {
      warning(
        typeof arg === 'number',
        `Expected argument ${index + 1} to be a number, got ${arg}`,
      );
    });
  }

  return args
    .map((multiplier = 0) => multiplier && `${unit * multiplier}px`)
    .join(' ');
};

spacing.unit = unit;

export default spacing;

This is a backward compatible change that allows you to then:

Single value:

Example

padding: theme.spacing(1)

Result

padding: 8px;

Multiple values:

Example

padding: theme.spacing(1, 2)

Result

padding: 8px 16px;

---

Example

padding: theme.spacing(.5, 1, 2, 3)

Result

padding: 4px 8px 16px 24px;

I'd like to open a pull request for this if we're onboard.

[oliviertassinari]

@MarkMurphy I love the idea of a spacing function! I think that we should provide more options to people. The variadic aspect of your proposal can be really useful.

I have tried to push this approach further in @material-ui/system: https://material-ui.com/system/spacing/#transformation. We have the same type of challenge to solve with the Grid #14099. I think that we should:

• find a way to expose a clean spacing API as well as a way to configure it so people can use an array of values, a function or a multiplier.
• Isolate the CSS helpers into a @material-ui/css-helpers package. We could host a fluid range function #11452, the color helpers #13039, the breakpoint helpers, the spacing helpers, transition helpers, etc.

What do you think?

[valoricDe]

How would I write margin: 16px auto?
[theme.spacing(2), 'auto']?

[eps1lon]

@valoricDe Have you tried using theme.spacing(2, 'auto')? Otherwise you could just create your own transformer that handles auto separately. Since there's only a single keyword it might be a better idea to handle this internally? @oliviertassinari

[oliviertassinari]

We could support it with a minor change. But should we 🤔?

[eps1lon]

We could support it with a minor change. But should we ?

I guess this can become pretty "ugly" if it's used very often e.g.
${theme.spacing(2, 3)} auto ${theme.spacing(2)} instead of "just" theme.spacing(2, 3, 'auto', 2). But then again every custom theme transformer would have to catch that case to not blindly use multiplication on strings. I think it's ok to not support it and wait for upvotes. This wasn't supported before anyway, right?

[oliviertassinari]

@valoricDe If you want to push for the addition of this feature, I would encourage you to open an issue dedicated to it. We will add the waiting for upvotes issue label.

[socketpair]

16163