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

RFC 2: Add spacing (margins) model to components #292

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 95 additions & 0 deletions docs/rfc-002.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
# Create a universal component model for spacing

## Problem and background

We need a model for applying different spacing to components depending on their context, for example adding or removing bottom margin. This issue has been [extensively documented](https://trello.com/c/KEkNsxG3/60-implement-customisable-spacing-for-components) prior to now, but was put on hold as there was no urgent need for it.

Organisation pages are being migrated out of whitehall and this has prompted the reopening of this issue. Specifically it is intended that the heading component be extended for use throughout these pages, which will require varying spacing being applied to it in different places.

In addition, the recent [RFC relating to organisation pages colours](https://github.com/alphagov/govuk_publishing_components/pull/287) may provide a similar solution to this issue.

## Proposed solution

A helper sass file in the gem that provides shared classes for any component that wants them. The spacing model itself would be based on the `$gutter` variables and would be reduced by a third on mobile.

Spacing model:

| Class | Value | Desktop | Mobile |
| ------------------------------ | ------- | ------------------- | ------------------------------ |
| margin-top--gutter | 1 | $gutter | $gutter-two-thirds |
| margin-top--gutter-two-thirds | 2 | $gutter-two-thirds | ($gutter-two-thirds / 3) * 2 |
| margin-top--gutter-one-half | 3 | $gutter-one-half | ($gutter-one-half / 3) * 2 |
| margin-top--gutter-one-third | 4 | $gutter-one-third | ($gutter-one-third / 3) * 2 |
| margin-top--gutter-one-quarter | 5 | $gutter-one-quarter | ($gutter-one-quarter / 3) * 2 |

This would be included for each of the following attributes (probably using a mixin to generate):

- margin-top
- margin-right
- margin-bottom
- margin-left

Sass example:

```
.margin-top--gutter {
margin-top: $gutter-two-thirds;

@include media(tablet) {
margin-top: $gutter;
}
}
```

### In the component

Components adapted to use this model could accept the parameter `margin` as an array of numbers, which would be used to add classes as appropriate. The array would contain four numbers, corresponding to top, right, bottom and left margins, for example `margins: [1, 0, 2, 0]` would give a margin value of 1 (gutter) to the top and a margin value of 2 (gutter-two-thirds) to the bottom.

```
margins ||= []
margin_classes = [
'gutter',
'gutter-two-thirds',
'gutter-one-half',
'gutter-one-third',
'gutter-one-quarter'
]

applied_margin_classes = ""

if margins
margins.each do |margin|
applied_margin_classes << " margin-top--" + margin_classes[margin - 1] if margin
end
end

<div class="gem-c-component <%= applied_margin_classes %>">
...
</div>
```

Would probably split the code above out into a helper for use by all components to reduce repetition.

Might need error checking if a number was passed that was too high.

## Other approaches considered

- see the [Trello card](https://trello.com/c/KEkNsxG3/60-implement-customisable-spacing-for-components)

## Decisions explained

- this doesn't necessarily work for all of the [spacing already defined in existing components](https://docs.google.com/spreadsheets/d/1mL-zAgi_Vfb7BO3KPF8b9xAPq_pIXIZHHSjvlB1Tulg/edit#gid=0) but the fact that spacing is so inconsistent has presented a barrier to solving this problem up until now. I think it's better to define something that works, then refine it and adjust instances of components over time.

## Assumptions made

- we could use the same model for padding but that would quickly get unwieldy due to the number of class names involved. Also padding is more likely to be specific to the design of a component and not something that should be adjustable
- components are built in such a way that margins are respected (e.g. no margin collapse due to floated elements anywhere)

## Problems and questions

- we already have individual ways in which components handle margin, so this is just adding another one that may or may not be used by more than one component
- aligning the classes to a scale like this (1 to 5) is a little confusing as it doesn't correspond to the numbers involved
- aligning the classes to a scale like this doesn't allow for flexibility later if we want to insert another class inside that scale
- this model only adds margin, it provides no option to have no margin i.e. it assumes no margin as the default. Components that already have margins by default may require more work to adapt to using this system.
- should the classes be more specifically namespaced? e.g. `.spacing-margin-top--gutter`?
- should the class names be shorter?