chore(docs): Document spacing between components#2455
chore(docs): Document spacing between components#2455VincentSmedinga wants to merge 18 commits intodevelopfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new Designer Guide documentation page describing recommended spacing between components, and adjusts Storybook docs styling so table headers use the design system’s bold body-text weight.
Changes:
- Add a new “Space between” Designer Guide page with spacing matrices and a code example.
- Update Storybook docs CSS overrides to render table header cells (
th) in bold. - Tweak the Space design-token doc copy by removing two explanatory sentences.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
storybook/src/styles/overrides.css |
Ensures Storybook docs tables render header cells with the DS bold font-weight for clearer matrices. |
storybook/src/docs/designer-guide/space-between.docs.mdx |
Introduces the new spacing guidance page and reference tables. |
storybook/src/brand/design-tokens/space.docs.mdx |
Removes a couple of lines from the Spacious spacing intro text. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
RubenSibon
left a comment
There was a problem hiding this comment.
General comments: this documentation is aimed at the designers, but it is also important for developers to follow these guidelines when they implement components with or without a dedicated designer to guide them.
It's quite a responsibility that developers have, and I am not sure that these guidelines will always be followed or even known about.
Apply that size in Figma. Developers can then apply it using the corresponding margin class name on each component.
In practice, many development teams (and possibly content editors?) in the City work without designers to guide them. They might expect to drop these components into a container like Column, set a general spacing with gap and be done.
I don't see an easy way to avoid bad implementations without enforcing spacing by adding outside spacing to components (whether they have them themselves, or parent containers are setting them on children).
How can we make it as easy as possible for developers to do it correctly? Maybe a start would be to add a little bit about spacing to each of the components mentioned in this Space between documentation.
For example, in the Heading docs, there is no mention of vertical space whatsoever. I'd suggest we a line about vertical spacing and maybe a link to these Space between docs. Then do the same to all other components mentioned in the tables.
|
A way to avoid unwanted combinations of components on pages, is to start making more patterns. Patterns are fixed combinations of components. These patterns contain the correct amount of spacing. |
|
we're not yet done checking all the values in the cells of these space between matrixes. |
Bondt007
left a comment
There was a problem hiding this comment.
still need to check all exact values.
94feaae to
c8f97ad
Compare
6505878 to
fd9b691
Compare
e4241ec to
597f6dc
Compare
597f6dc to
973d68a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
# Conflicts: # storybook/src/pages/public/ArticlePage/ArticlePage.stories.tsx
Rewrites the prose-spacing mixin so the margins between children of .ams-prose match the pairwise matrix in SpaceBetweenFinder/config.ts. Introduces Sass variables for the recurring component families (heavy body, minor heading, list-or-paragraph, button-or-cta, plain paragraph) so they surface once at the top of the file without changing the compiled output.
Adds a README for the .ams-prose utility class and a matching Storybook entry under Utilities/CSS/Prose. The story renders a Dutch example page about werkzaamheden aan de Oranjeloper that exercises most of the spacing pairings handled by the prose-spacing mixin.
dlnr
left a comment
There was a problem hiding this comment.
I would need more time to check if all compiled selectors do what they are supposed to do, they get kind of long after Sass does it's thing:
: is(.ams-prose>:is(.ams-blockquote,.ams-image,.ams-paragraph--large,.ams-table):has(+:not(.ams-heading)),.ams-prose>:not(.ams-heading):has(+:is(.ams-blockquote,.ams-image,.ams-paragraph--large,.ams-table))) {
But here are some first impressions.
| .ams-prose { | ||
| @include prose-spacing; | ||
|
|
||
| .ams-accordion__panel { |
There was a problem hiding this comment.
I don't see an example of this in the story and I'm curious to see what happens. I would assume the ams-prose wrapper only has effect on direct children and this exemption is kind of weird specific inheritance. You could just wrap content in the tabs panel in a div with the className. If you have a custom component, or table, or card it should not have effect on this content either.
There was a problem hiding this comment.
Yes, it would be nice if this story shows all descendants that can be affected by Prose. Or if another story shows how an Accordion is affected.
If showing all possible permutation in Storybook is a bit too much, you could of course add that full example to Prototypes and/or a Test story for Chromatic.
Actually it might be a good idea to have snapshot tests for this specific utility class, because there are so many parts that could break at some point.
| <div {...restProps} className={clsx('_ams-space-between-finder sb-docs sb-docs-content', className)}> | ||
| <div className="_ams-space-between-finder__fields"> | ||
| <div> | ||
| <label htmlFor="space-between-above">Component on top:</label> |
There was a problem hiding this comment.
Missed opportunity to use our own components?
|
|
||
| ## Code example | ||
|
|
||
| {/* prettier-ignore */} |
There was a problem hiding this comment.
This is weird, the prettier config does not include markdown files.
|
|
||
| ## Templates te maken | ||
|
|
||
| - Contentpagina met h1 en h2 achter elkaar (zonder foto en intro) dus eigenlijk Czaar Peter |
|
|
||
| {/* prettier-ignore */} | ||
| ```tsx | ||
| <Grid.Cell> |
There was a problem hiding this comment.
Not recommending the prose util?
|
|
||
| /* Context-aware spacing for browsers with :has() support */ | ||
| /* stylelint-disable-next-line scss/operator-no-unspaced */ | ||
| @supports selector(:has(+ *)) { |
There was a problem hiding this comment.
If the has selector is widely available soon this might be overkill, and if a browser doesn't support the selector the rule will not be applied right?
| - It sets `margin-block-end` on each direct child based on what follows it, using the `:has()` selector. | ||
| Browsers without `:has()` support fall back to a uniform spacing. | ||
| - Content inside an Accordion panel nested under `.ams-prose` is spaced the same way, so an accordion and its surroundings share one rhythm. | ||
| - To add a single ad-hoc gap between two elements, prefer the [Margin Bottom](/docs/utilities-css-margin--docs) utility class on the first one. |
There was a problem hiding this comment.
Let’s add a reference to the Prose docs in the Margin utility docs.
| Browsers without `:has()` support fall back to a uniform spacing. | ||
| - Content inside an Accordion panel nested under `.ams-prose` is spaced the same way, so an accordion and its surroundings share one rhythm. | ||
| - To add a single ad-hoc gap between two elements, prefer the [Margin Bottom](/docs/utilities-css-margin--docs) utility class on the first one. | ||
| - To add even spacing between a set of siblings, prefer the [Gap](/docs/utilities-css-gap--docs) utility class on their parent. |
There was a problem hiding this comment.
Let’s also add a reference to the Prose docs in the Gap utility docs.
There was a problem hiding this comment.
I think it would be good to point implementers to vertical spacing early in the Developer Guide. Not sure where exactly.
And would it be helpful or distracting to add a line to every single component (text, media, etc.) affected by Prose?
| 'Brand', | ||
| 'Components', | ||
| ['Buttons', 'Containers', 'Feedback', 'Forms', 'Layout', 'Media', 'Navigation', 'Text'], | ||
| 'Utilities', |
There was a problem hiding this comment.
Having Utilities here feels wrong, because it between incrementally larger concerns: Tokens < Components < Patterns < Pages.
Either:
- move the entire Utilities category up so that it’s above Components and below Brand;
- remove the Utilities category (which has only one member anyway) and add a ‘CSS utilities’ folder to the Components category.
Option 2 makes the most sense because our CSS utilities are actually CSS components in (almost?) everyway.
| 'Components', | ||
| ['Buttons', 'Containers', 'Feedback', 'Forms', 'Layout', 'Media', 'Navigation', 'Text'], | ||
| 'Utilities', | ||
| 'Patterns', |
There was a problem hiding this comment.
This could be a good place for other common component compositions such as form fields with inputs, labels, error messages and whatnot.
|
|
||
| ## Guidelines | ||
|
|
||
| - Use this utility class on a container that wraps editorial content — an article, a rich-text block, or a section built from multiple components — so its children are spaced according to the [‘Space between’](/docs/docs-designer-guide-space-between--docs) recommendations. |
There was a problem hiding this comment.
We can consider adding an Article component that has the prose mixin applied by default.
With the as attribute we could even allow for the article element to be swapped with other element types.
Document spacing between components
What
a. Includes a little form tool to get the advised spacing between the two selected components; but note that values haven’t been updated since the earlier iteration in March.
a. The ‘real’ pages haven’t been updated yet.
Why
People were asking about it, and we ourselves aren’t always as sure about what value to choose.
How
n/a
Additional notes