[RFC] Spacing

[connor-baer]

Components to amend

Heading, SubHeading, Text, Input, and more

Context

When the initial version of Circuit UI was created, we decided to add a default bottom spacing of 16px to some components. The intention was to enforce consistency but in reality, these spacings are rarely consistent. Thus, we introduced the noMargin prop to turn off this default spacing. Fighting with bad defaults isn't a nice developer experience, so this RFC explores more ergonomic alternatives.

Goals

• consistent default spacing across all components (no default spacing would also be consistent 😉)
• match the gutter width of the grid across viewports
• encourage consistent spacings between components in userland ("pit of success")
• provide an escape hatch when custom spacings are required

Proposals

These two proposals do not have to be mutually exclusive.

1. Style mixins

Mixins are functions that accept a space enum + the theme and return styles. They're simple to use, can be responsive, and encourage consistency.

import { css } from '@emotion/core';
import styled from '@emotion/styled';
import { Button, spacing } from '@sumup/circuit-ui';

const ButtonWithMargin = props => (
  <Button css={spacing('mega')}>Example</Button>
);

// or

const buttonStyles = ({ theme }) => css`
  ${spacing('mega', theme)};
  color: red;
`;

const StyledButton = styled(Button)(buttonStyles);

The spacing value can take different shapes to make it easy to apply any combination of spacings:

// Add spacing on all sides
spacing('mega', theme)
spacing(['mega'], theme)

// Add vertical spacing
spacing(['mega', null], theme)

// Add horizontal spacing
spacing([null, 'mega'], theme)

// Add individual spacing for each side
spacing(['kilo', 'mega', 'giga', 'tera'], theme)

// Add individual spacing for one side
spacing({ top: 'kilo' }, theme)

The mixins could be added to the theme or exported separately (better tree-shaking). The same concept could be used for other common styles, e.g. shadows, font sizes.

2. Spacing component

Circuit UI already has a rudimentary <Spacing /> component. We could extend it:

import { css } from '@emotion/core';
import styled from '@emotion/styled';
import { Button, Spacing } from '@sumup/circuit-ui';

const ButtonWithMargin = props => (
  <Spacing bottom="mega">
    <Button>Example</Button>
  </Spacing>
);

Same as the spacing mixing, the component can be used for any combination of spacings:

// Add spacing on all sides
<Spacing all="mega" />

// Add vertical spacing
<Spacing vertical="mega" />

// Add horizontal spacing
<Spacing horizontal="mega" />

// Add individual spacing for each side
<Spacing top="kilo" right="mega" bottom="giga" left="tera" />

// Add individual spacing for one side
<Spacing top="kilo" />

The downside to this approach is that it requires another JSX and DOM element.

Migration

Changing the default spacing is a breaking change that will require significant work from users when upgrading. That's why the transition needs to be gradual.

1. Implement the new spacing helpers as described above.
2. Any Circuit UI components that have external spacing should have a noMargin prop to remove this spacing.
3. Whenever a component is used without the noMargin prop, a deprecation warning should be logged. This way, developers can track down the components that rely on the built-in spacing and replace it with custom styles or a style helper.
4. In the end, all components will have a redundant noMargin prop. This can easily be removed with a codemod once we fully remove the built-in spacing in a major version.

[connor-baer]

Leaving this here for later reference Density on the web 🔗

[larimaza]

The performance impact of having an extra DOM element (in the Spacing component approach) is not clear to me. Could somebody with more context please add on this? 👀

[herberthenrique]

My only concern here is about having only warnings for project maintainers, can we create something like an eslint plugin that has a warning in the build time? This will be super good for big projects like ze-dashboard that we can't measure the whole impact

[connor-baer]

The performance impact of having an extra DOM element (in the Spacing component approach) is not clear to me. Could somebody with more context please add to this? 👀

While keeping the HTML lean is more a personal preference of mine, it can have a performance impact in extreme cases: https://web.dev/dom-size/

My only concern here is about having only warnings for project maintainers, can we create something like an eslint plugin that has a warning in the build time? This will be super good for big projects like ze-dashboard that we can't measure the whole impact

Interesting idea. I've never written an Eslint plugin before, so I'm not sure about the required effort. I imagine it's similar to writing a codemod since it's also operating on an AST.

[hleote]

Hey everyone 👋

Thanks for the great RFC @connor-baer, I think they are a great way to communicate between remote teams... 👍 🙂
IMO sounds like a good idea to have no default spacing.

Regarding the 1. Style mixins or 2. Spacing component I personally prefer number 1 but I wonder if it would be a good idea to support both? 🙂
We could recommend the usage of option 1 for bigger projects... but I guess it would be nice to allow devs to use different approaches depending on their code preferences... Hopefully that makes sense... 🙂

[mamant]

it will be useful to have this spacing not only as a component but also as an utility classes for non react applications
we've made a small util for that

[robinmetral]

I second @hleote here, I think both would be great. It seems like using a Spacing component or a mixin might depend on personal preference and there are good use cases for both.

Personally I'm not a fan of the Spacing component in logic-heavy components because I feel that it clutters the JSX, but I do like it in more UI-focused components where it can avoid extra styled components.

Also the Spacing does add an extra div, but in some cases a wrapper is necessary anyways:

const FlexContainer = styled(Spacing)`
  display: flex;
`;

const Component = () => (
  <>
    <FlexContainer>
      <FlexItem />
      <FlexItem />
    </FlexContainer>
    <Footer />
  </>
);

☝️ here the element created by the Spacing has to be there anyways

Long story short, I'm very much in favor of rethinking our spacings and I think both suggestions are good approaches 🙂

[connor-baer]

Thanks for the feedback everyone! 🙌

I am going to implement the new style helpers tomorrow soon, as well as adding the deprecation warning when not using the noMargin prop.

it will be useful to have this spacing not only as a component but also as a utility class for non react applications

Support for static CSS was experimental in Circuit UI v1 and broke with the migration to TypeScript in v2. I plan to publish a separate RFC soon to fix and stabilize static CSS extraction.

[felixjung]

Very much in favor of the spacing mixin, having used it in personal projects after our discussions.

but I wonder if it would be a good idea to support both?

I think providing both wouldn't be something you'd want to do in Circuit UI, as it leads to ambiguity. As a library, I feel Circuit UI should come with one way to do a certain thing whenever possible. If someone wants to go down the route of using a component, they can implement that component with very little effort in userland.

[connor-baer]

@mcntsh asked about support for responsive spacing values, ie. spacing that changes with the viewport width. This would add quite a bit of complexity and I'm not sure we need it (yet), so we'll omit this for now. If in the future we discover that this is required, we can add it then.

[connor-baer]

🎉 This issue has been resolved in version 2.4.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release