Skip to content

Make centre rule configurable in grid module#15951

Merged
frederickobrien merged 6 commits into
mainfrom
custom-centre-rules-in-grid-api
Jun 10, 2026
Merged

Make centre rule configurable in grid module#15951
frederickobrien merged 6 commits into
mainfrom
custom-centre-rules-in-grid-api

Conversation

@frederickobrien

@frederickobrien frederickobrien commented May 21, 2026

Copy link
Copy Markdown
Contributor

This rejigs the verticalRules option added to the grid module in #15760 so that its configurable to the nth child of the grid, rather than the first by default. This has come up while working on #15358 and #15922 and feels like a worthwhile change in its own right as the central rule is likely to align with different pieces of article furniture depending on the layout (and breakpoint).

Instead of a binary yes/no on the centre rule which applies to the first child regardless, this makes it more controlled by separating verticalRules and centreRule into separate options:

${grid.container}
${grid.outerRules()}
${grid.centreRule(1)}
 
${from.leftCol} {
	${grid.centreRule(3)}
}

As a proving ground this PR applies the new approach to the gallery layout, which was the first to adopt grid-based vertical rules in #15776. I've also applied it to #15428 and it feels pretty tidy.

@frederickobrien frederickobrien self-assigned this May 21, 2026
@frederickobrien frederickobrien requested review from a team and JamieB-gu May 21, 2026 16:16
@github-actions

Copy link
Copy Markdown

Hello 👋! When you're ready to run Chromatic, please apply the run_chromatic label to this PR.

You will need to reapply the label each time you want to run Chromatic.

Click here to see the Chromatic project.

@frederickobrien frederickobrien added the feature Departmental tracking: work on a new feature label May 21, 2026
@frederickobrien frederickobrien added this to the Interactives milestone May 21, 2026
@github-actions

github-actions Bot commented May 21, 2026

Copy link
Copy Markdown

@frederickobrien frederickobrien added the run_chromatic Runs chromatic when label is applied label May 21, 2026
@github-actions github-actions Bot removed the run_chromatic Runs chromatic when label is applied label May 21, 2026
@frederickobrien frederickobrien force-pushed the custom-centre-rules-in-grid-api branch from acda072 to c98180b Compare May 28, 2026 12:40
@github-actions

github-actions Bot commented May 28, 2026

Copy link
Copy Markdown

@JamieB-gu

JamieB-gu commented May 29, 2026

Copy link
Copy Markdown
Contributor

It looks like the centre column has disappeared for the images at the desktop breakpoint? I can only see it appearing on the ads.

@frederickobrien

Copy link
Copy Markdown
Contributor Author

It looks like the centre column has disappeared for the images at the desktop breakpoint? I can only see it appearing on the ads.

Ah yep, my bad. Fixed with 940bc29

image

@frederickobrien frederickobrien force-pushed the custom-centre-rules-in-grid-api branch from 940bc29 to b90f67d Compare May 29, 2026 16:17

@JamieB-gu JamieB-gu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @frederickobrien . Looking at your screenshot above it looks like you've picked up an extra border on the left hand side with that change?

A few other small comments.

Comment thread dotcom-rendering/src/components/GalleryImage.tsx Outdated
Comment thread dotcom-rendering/src/components/GalleryImage.tsx Outdated
Comment thread dotcom-rendering/src/grid.ts Outdated
type VerticalRuleOptions = {
centre?: boolean;
color?: string;
plusChild?: number;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may be misunderstanding something, but what does the "plus" refer to in plusChild?

@frederickobrien frederickobrien Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was the idea yes but agree it's unclear. Have changed to a stadndalone centreRule which together with the number is more readable I think(?). E.g.

${grid.centreRule(2)}

@frederickobrien frederickobrien force-pushed the custom-centre-rules-in-grid-api branch from b90f67d to 6719ce9 Compare June 3, 2026 09:45
@frederickobrien

frederickobrien commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

It looks like the centre column has disappeared for the images at the desktop breakpoint? I can only see it appearing on the ads.

Ah yep, my bad. Fixed with 940bc29

image

Eep sorry yes. Have fixed in 1ee0fca. Here's a gallery piece at different breakpoints...

Tablet:

image

Desktop:

image

LeftCol:

image

@frederickobrien frederickobrien force-pushed the custom-centre-rules-in-grid-api branch 2 times, most recently from d5cd11f to 172a642 Compare June 3, 2026 10:33
@frederickobrien frederickobrien added the run_chromatic Runs chromatic when label is applied label Jun 3, 2026
@github-actions github-actions Bot removed the run_chromatic Runs chromatic when label is applied label Jun 3, 2026
@frederickobrien

frederickobrien commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

After thinking about it I've gone a step further and separated outerRules (formerly verticalRules) and centreRule into distinct options in the grid module. @JamieB-gu I think you actually suggested something like this when we first discussed it a while back?

${grid.container}
${grid.outerRules()}
${grid.centreRule(1)}
 
${from.leftCol} {
	${grid.centreRule(3)}
}

I think this is more readable and allows for flexibility across breakpoints.

@frederickobrien frederickobrien force-pushed the custom-centre-rules-in-grid-api branch from 466d913 to d3ea03b Compare June 3, 2026 14:20
@frederickobrien frederickobrien requested a review from JamieB-gu June 3, 2026 15:11
@frederickobrien frederickobrien force-pushed the custom-centre-rules-in-grid-api branch 2 times, most recently from 528bb6f to a06b8bf Compare June 5, 2026 12:59

@JamieB-gu JamieB-gu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, a few last comments.

Comment thread dotcom-rendering/src/grid.ts Outdated
Comment thread dotcom-rendering/src/grid.ts
Comment thread dotcom-rendering/src/grid.ts
@frederickobrien frederickobrien force-pushed the custom-centre-rules-in-grid-api branch from a06b8bf to ad559c4 Compare June 10, 2026 13:32
@frederickobrien frederickobrien added the run_chromatic Runs chromatic when label is applied label Jun 10, 2026
@github-actions github-actions Bot removed the run_chromatic Runs chromatic when label is applied label Jun 10, 2026
@frederickobrien frederickobrien merged commit ec04511 into main Jun 10, 2026
28 checks passed
@frederickobrien frederickobrien deleted the custom-centre-rules-in-grid-api branch June 10, 2026 14:07
@gu-prout

gu-prout Bot commented Jun 10, 2026

Copy link
Copy Markdown

Seen on PROD (merged by @frederickobrien 8 minutes and 19 seconds ago) Please check your changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Departmental tracking: work on a new feature Seen-on-PROD

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants