weaveworks / ui-components

A collection of UI components that are shared across Weaveworks projects
http://weaveworks-ui-components.s3-website-us-west-2.amazonaws.com/
Apache License 2.0
30 stars 26 forks source link

Standardise spacing #400

Closed bia closed 5 years ago

bia commented 5 years ago

Now that fonts and colours are quite organised, spacing could still use a little love. Airbnb's design system could be a bit of inspiration : dls-foundation

We could have a theme file which would set some values that are multiples of eachother that we can keep reusing :

export const spacing = {
  xs: '4px',
  mini: '8px',
  small: '16px',
  base: '24px',
  large: '48px',  
  xl: '64px',
};

(these values are totally arbitrary and can change as we find appropriate)

cc @fbarl @ngehani

ngehani commented 5 years ago

nice!

bia commented 5 years ago

In fact, we already have a file called constants.jsx which only contains

export const PADDING_UNIT_PX = 10;

fbarl commented 5 years ago

I might have mentioned this before, but if we want to eventually enforce these sizes on all our paddings and margins, we'll probably need 2px as well.

Having said that, I'm not sure we want to go that far with spacing as we went with colors for example, i.e. enforcing it on all CSS across the app (e.g. what if we just wanna center an icon by adding a 1px top margin to it?).

However, if we don't enforce it with a linting rule eventually, there is a danger that more bad spacings will slip through the crack :)

In conclusion, I'd say let's try applying it xl down to xs and see if we're happy enough with the direction to make it a linting rule eventually.

p.s. I'll open the follow-up issues.

fbarl commented 5 years ago

Also, I think it makes more sense to have xs: 8px and mini: 4px - wdyt @bia ?

bia commented 5 years ago

For our reference, here's a breakdown of how our spacing is organised now. 8px and 16px spaces are similar to the proposed system, but larger sizes are all over the place.

400-standardise-spacing5