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

Standardise spacing #400

Closed
bia opened this issue Oct 30, 2018 · 5 comments
Closed

Standardise spacing #400

bia opened this issue Oct 30, 2018 · 5 comments
Assignees
Labels
needs-discussion Issue is a question or a proposal that needs discussion style-guide Part of the effort towards a unified design system

Comments

@bia
Copy link
Contributor

bia commented Oct 30, 2018

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

@fbarl fbarl added needs-discussion Issue is a question or a proposal that needs discussion style-guide Part of the effort towards a unified design system labels Oct 30, 2018
@ngehani
Copy link

ngehani commented Oct 30, 2018

nice!

@bia
Copy link
Contributor Author

bia commented Nov 1, 2018

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

export const PADDING_UNIT_PX = 10;

@bia bia added the tech-debt Unpleasantness that does (or may in future) affect development label Nov 13, 2018
@fbarl
Copy link
Contributor

fbarl commented Nov 22, 2018

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 fbarl removed the tech-debt Unpleasantness that does (or may in future) affect development label Nov 22, 2018
@fbarl
Copy link
Contributor

fbarl commented Nov 22, 2018

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

@bia
Copy link
Contributor Author

bia commented Nov 22, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-discussion Issue is a question or a proposal that needs discussion style-guide Part of the effort towards a unified design system
Projects
None yet
Development

No branches or pull requests

3 participants