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

o-spacing #10

Closed
gvonkoss opened this issue Feb 18, 2019 · 7 comments
Closed

o-spacing #10

gvonkoss opened this issue Feb 18, 2019 · 7 comments
Assignees

Comments

@gvonkoss
Copy link
Contributor

gvonkoss commented Feb 18, 2019

What

o-spacing will be a new low-level component that handles spacing (for the moment, margins and padding).

Why

We have found that our spacing mechanism across components has been rather inconsistent. Where there is consistency, the spacing for our components relies on o-typography, namely oTypographyMargin(), oTypographyPadding() (both of which only handle vertical spacing, e.g. top and bottom margins) and oTypographySpacingSize().

This would provide a more granular and consistent control for the spacing we use across our components, where we would be able to share and document our reasoning and the spacing values more accurately.

It would mean, however, that it would create an additional low level dependency for most of our maintained components, but if we build it so that we can change values under the hood, this shouldn't cause too much of a problem when it comes to making changes.

Supporting Examples

Entirely open to modification, but providing mixins (or functions) along the lines of:

@include oSpacingPadding($top, $right, $bottom, $left);
@gvonkoss
Copy link
Contributor Author

gvonkoss commented Mar 15, 2019

We need feedback from the design team for this, in order to align with the developments under the new design system—we think the primary contact for this would be Hector Crespo

@notlee
Copy link
Contributor

notlee commented May 8, 2019

Speaking to Mark Limb, his latest designs for the app has space between components of:
24px, 32px, 40px

Within components is much more granular. Taking forms as an example: 20px, 16px, 12px.

@notlee
Copy link
Contributor

notlee commented May 10, 2019

Update on this proposal...

Spacing in the design toolkit

The design team's toolkit has recently introduced spacing guidelines which uses a 4px baseline, the same as currently specified in o-typography. Perfect!

Screenshot 2019-05-10 at 15 06 53

The guide defines 32px as a basic space unit (1), with other units refereed to relative to this (e.g. 48px as 1.5). Keeping in mind each unit fits within the 4px baseline.

The spacing units are also split into three groups (small, medium, large) which act as an indication of where they should be used (for instance within components, between components, page sections).

Design toolkit consensus

Whilst we've had a 4px baseline for a while, the above space guideline are fairly new and not widely implemented. The app homepage is currently being rebuilt and wasn't quite following this guide (opting for 40px over 48px), but Mark L said they're happy to keep to the values already defined rather than change the design toolkit. I'll chat to other members of the design team including Simon C and Mark H to ensure everyone is aware of and using these units.

Space unit naming

Whilst chatting to Hector we concluded that using a letter to indicate the space size (s for small) with a number would be a useful way to communicate spacing units (s1, s2, s3... m1, m2...). This would enable spacing utility classes or css variables for build service users to add vertical rhythm to their project e.g. --o-space-m1. Although this naming convention prohibits adding new unit sizes, e.g. would a new smaller medium space become --o-space-m0?.

Talking in terms of multiplications from a basic space unit (32px) could work and avoid the above pitfall, e.g. a half space would be 16px, a quarter space 8px. Build service users could include a 16px margin as --o-space-2-5 (indicating 2.5 of a space), but this seems much less intuitive and "human" naming verbose --o-space-two-and-half.

Casual conversions so far have prompted differing opinions, but there has been more support for the first option. The second suggestion has been divisive.

Next up

  • Talk to more members of the uxd team to ensure there's a consensus on these units and find a preferred way of referring to them -- a common language which may be shared with developers.
  • Add a proposal for a Sass and build service interface to use spacing units in projects.

@notlee
Copy link
Contributor

notlee commented May 14, 2019

Speaking to Olga this morning. Her views, as a designer who works on internal tools, were that it's useful to be able to derive the size from the space name but "s1", "s2", "m1" is quite abstract. Gabi had similar thoughts. Using the px value in the name (regardless of whether a px or rem value is output) might be a good way to communicate the preset sizes. It's also flexible to additions. Should our baseline ever change from 4px, this naming makes less sense but still works:
"s4", "s8", "s12"... "m48".

Alternatively, we might use the division of the baseline unit (the number we currently give oTypographySpacingSize) along withs, m, or l to indicate a preset:
"s1", "s2" , "s3"... "m12".

But this diverges from how the design toolkit talks about space presets today.

@notlee
Copy link
Contributor

notlee commented May 14, 2019

Spacing in some other component libraries and design systems:

Proportional Numeric
Spaces are defined on a proportional numerical scale e.g. where a space of40 is twice as big as 20 on the scale.

  • Tailwind default scale has finite increments (e.g. a space named 24 is followed by space 32), the value is calculated by multiplying by 0.25rem.
  • Material UI has props to output a size using a multiplication of the baseline unit, which doesn't appear to be limited (same as our current o-typography solution). Material Design provides guidelines of what sizes to use where.

Simple Numeric
Spaces are defined on a scale of 0-x, e.g. 1, 2, 3, 4. The values go from small to large and are not linear.

  • Carbon Design splits spacing into two 0-x scales, one for "spacing" and one for "layout".
  • GDS has a 0-9 scale, which values are responsive. Our current approach is for users to output a different size within a media query to output a different scale value as needed (same as Carbon's approach).

@notlee
Copy link
Contributor

notlee commented May 14, 2019

I suspect a major case of bike shedding going on. I think naming is important to achieving our goals on consistent spacing though. Especially if we improve the number of developers and designers referring to the toolkit spec, and help us communicate spacing consistently.

We should have a Sass function to help output any space based on the baseline 4px baseline. But how to name our recommended spaces is an ongoing question. Here are some examples of previously discussed options with a pro/con checklist:

1. by proportion of basic unit

As per the design toolkit. Have 32px equal a "standard space" and refer to the rest relatively.

name value
0.125 4px
0.25 8px
0.375 12px
0.5 16px
0.75 24px
1 32px
1.5 48px
2 64px
2.5 72px
3 96px
  • New values can be added.

  • Values may be updated across all points whilst still making sense (e.g. larger baseline).

  • Values may be updated individually at different points (non-linear).

  • Can derive value from the name.

  • Ease of remembering options.

  • To increase the space between the last and second to last option, a new value would need to be added which projects would have to migrate to.

  • It's hard to remember the decimal values without referring to documentation.

🙃 Multiple people have feedback that the divisions from a basic space are difficult to calculate / remember.

2. by simple numeric scale

name value
1 4px
2 8px
3 12px
4 16px
5 24px
6 32px
7 48px
8 64px
9 72px
10 96px
  • New values can be added.

  • Values may be updated across all points whilst still making sense (e.g. larger baseline).

  • Values may be updated individually at different points (non-linear).

  • Can derive value from the name.

  • Ease of remembering options.

  • It's impractical to add to the scale, e.g. if we wanted to space components by 54px what would it be named? It would be straightforward to update an existing space on the scale though, e.g. to make 7 larger or 8 smaller.

  • It's impossible to derive the value from the space name which designers in particular may find useful. Allowed options 1-10 are straightforward to remember though.

🙃 The design guidelines aren't widely adopted, I suspect it won't adapt well to change. Multiple people have feedback it's useful to be able to derive the value from the name.

3. by baseline

name value
1 4px
2 8px
3 12px
4 16px
6 24px
8 32px
12 48px
16 64px
18 72px
24 96px
  • New values can be added.

  • Values may be updated across all points whilst still making sense (e.g. larger baseline).

  • Values may be updated individually at different points (non-linear).

  • Can derive value from the name.

  • Ease of remembering options.

  • It's easier to remember and refer to these options when compared to the option "proportion of basic unit", but it's not clear why all baseline units can't be used.

👍 Adaptable. Makes sense against our current way of doing spacing.

4. by baseline with s,m,l indicator

name value
s1 4px
s2 8px
s3 12px
s4 16px
s6 24px
s8 32px
m12 48px
m16 64px
l18 72px
l24 96px
  • Including the small, medium, or large in the name may prompt users to put these spaces in appropriate spaces, similar to Carbon's two scales but is that something we need?
  • Makes sense against our current way of doing spacing.

👍 Adaptable. Makes sense against our current way of doing spacing. Differentiates getting a named space from a limited set to deriving a any space from the baseline. E.g.

margin: oSpacingSize('s4'); // 16px
margin: oSpacingSize('s5'); // errors, s5 is not a recommended space
margin: oSpacingIncrement(5); // 20px, derived from the baseline

@notlee
Copy link
Contributor

notlee commented May 16, 2019

API Proposal

Build Service

consistent spacing

Output CSS classes to apply recommended spacing. We could follow a naming convention like Tailwind to allow build service users to apply any margin or padding with these spaces, however to keep bundle size and the api smaller I'd suggest providing classes only for the most common use case. That is bottom margin (vertical rhythm) to lay out components on a page (e.g. see biz-ops).

class name value
o-spacing-s1 margin-bottom: 4px
o-spacing-s2 margin-bottom: 8px
o-spacing-s3 margin-bottom: 12px
o-spacing-s4 margin-bottom: 16px
o-spacing-s6 margin-bottom: 24px
o-spacing-s8 margin-bottom: 32px
o-spacing-m12 margin-bottom: 48px
o-spacing-m16 margin-bottom: 64px
o-spacing-l18 margin-bottom: 72px
o-spacing-l24 margin-bottom: 96px

We may also provide css custom properties to cater for other usecases, especially as many internal tools or products browser support requirement support it.

custom property value
--o-spacing-s2 8px
--o-spacing-s3 12px
--o-spacing-s4 16px
--o-spacing-s6 24px
--o-spacing-s8 32px
--o-spacing-m12 48px
--o-spacing-m16 64px
--o-spacing-l18 72px
--o-spacing-l24 96px

granular spacing

For build service users to achieve more granular spacing off the baseline we can provide a CSS variable --o-spacing-baseline: 4px. This could be used in projects with the calc function padding: calc(var(--o-spacing-baseline) * 2);`

Sass Users

consistent spacing

To make Origami components more approachable for contributors outside the team or new Origami developers, we have been aiming to reduce our Sass footprint in favour of pure CSS. With this in mind a Sass function to get a size instead of a mixin to set a fill property feels appropriate oSpacingGetSize($size-name). E.g.

    padding: oSpacingGetSize('s3'); // 12px
    margin-bottom: oSpacingGetSize('m12'); // 48px

granular spacing

For Sass users who need to support legacy browsers a way to get an increment of a baseline unit would be useful, like oTypographySpacingSize now e.g. oSpacingGetIncrement($value).

    padding: oSpacingGetIncrement(1); // 4px
    margin-bottom: oSpacingGetIncrement(2); // 8px
    margin-top: oSpacingGetIncrement(68); // 272px

@notlee notlee closed this as completed Jun 11, 2019
notlee added a commit to Financial-Times/o-typography that referenced this issue Jul 2, 2019
This deprecates:
- oTypographyMargin
- oTypographyPadding
- oTypographySpacingSize

This is not considered a breaking change as o-spacing
is a new component.

See the [o-spacing proposal](Financial-Times/origami#10).
notlee added a commit to Financial-Times/o-typography that referenced this issue Jul 3, 2019
* Introduce o-spacing.

This deprecates:
- oTypographyMargin
- oTypographyPadding
- oTypographySpacingSize

This is not considered a breaking change as o-spacing
is a new component.

See the [o-spacing proposal](Financial-Times/origami#10).
notlee pushed a commit that referenced this issue Mar 6, 2020
* update build service tutorial

* add manual build process file

* add to tutorial

* all component markup added

* almost done, all external links and todos tbresolved

* changes

* tree --> folder

* Heroku review apps (#9)

* Create Rakefile

* Update Gemfile

* Updated dependancies

* ignore rubygems vendor directory from jekyll building

* Add repository field

* Update manual-build.md

* Update build-service.md

* make the asides work as they should?

* h3 --> h2

* add links, change werds, go to pub ist friyay

* small wording changes, add next steps

* use mixins so that we can o-brand in a different tutorial

* manual

* ¯\_(ツ)_/¯

* more tweaks

* fix comments/links
chee pushed a commit that referenced this issue Sep 15, 2021
…inimist-1.2.5

Bump minimist from 1.2.0 to 1.2.5
JakeChampion pushed a commit that referenced this issue Sep 16, 2021
* allow single column span

* retreat

* restore demo

* update bemness
JakeChampion pushed a commit that referenced this issue Sep 17, 2021
* update to dart-sass 1.26.3

* add some logging to the update script
chee added a commit that referenced this issue Sep 23, 2021
…tps://github.com/orgs/Financial-Times/projects/83?fullscreen=true (#10)
chee pushed a commit that referenced this issue Sep 23, 2021
…ocha-8.0.1

Bump mocha from 7.2.0 to 8.0.1
joannaskao added a commit that referenced this issue Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants