fix(section): honour a per-block padding override - #186
Merged
Conversation
`cta` and `panels` both declare `padding` in their bookshop schema, so authors set it and reasonably expect the section's vertical padding to change. It never did. `utilities/section.html` derived padding solely from `utilities/GetPadding.html`, which reads site parameters, and no block passed anything through. The key was accepted, range-validated, and dropped. Measured on a real site before this change: four `cta` blocks carrying `padding: 4` and seven carrying nothing all rendered `py-5`, and `padding: 0` rendered `py-5` too. After: `padding: 4` renders `py-4`, `padding: 0` renders `py-0`, and a block with no `padding` still renders the site default `py-5`. The horizontal axis (`px-3 px-md-5`) is byte-identical in every case. ## The empty default is load-bearing `padding` inherits the shared definition in mod-utils `data/structures/_arguments.yml`, which carries `default: 3`. Left alone, an omitted `padding` would arrive as 3 rather than nil — "not supplied" becomes indistinguishable from "supplied", and every section that does not set one gets silently repadded. An earlier revision of this change hit exactly that: unset `cta` blocks dropped from `py-5` to `py-3`. `data/structures/section.yml` therefore redeclares `padding` with an empty `default:`. `ArgsSchema.html` merges the local node over the global one, so the empty value wins and a real nil is restored. The inherited `options` (0-5) survive that merge, so validation is unchanged -- `padding: 9` still fails with "out of range [-, 5]" before rendering. The nil comparison is deliberate for the same reason: `with` or `| default` would discard a legitimate `padding: 0`. ## Scope Vertical axis only. `x` is the page gutter, shared by every section and also feeding the outer container, so a per-block override there would misalign that section against its neighbours. Any mobile `y` override is dropped alongside, since it derives from the site value being replaced. Limited to `cta` and `panels` -- the two blocks where `padding` was purely inert. `approach`, `articles`, `cards`, `team` and `testimonials` also declare it, but forward it to inner partials (card group, section title) where it already means something; making it drive section padding as well would change existing sites, which is a semantic decision rather than a bug fix. `pnpm test` passes: 40 pages, no errors.
markdumay
force-pushed
the
fix/honour-block-padding
branch
from
July 30, 2026 18:01
b321ed7 to
fe92db5
Compare
Contributor
Author
|
🎉 This PR is included in version 2.3.3 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
markdumay
added a commit
that referenced
this pull request
Jul 31, 2026
`padding: N` set the section's vertical padding and left the horizontal axis on the site default, so a block with an override rendered an asymmetric box -- 24px vertical against 48px horizontal at the default config. One number producing two values surprises authors, which is how this surfaced. Both axes now take the override, scoped to the inner content container: no override inner 48px all sides (site default, unchanged) padding: 4 inner 24px all sides padding: 0 inner 0px all sides The outer container keeps the site padding in every case, so an overridden section still lines its edge up with its neighbours -- all three fixture sections start at x=48 on desktop and x=16 on mobile, with identical outer padding. The author is repadding their content, not moving the section out of the page grid. The site-level mobile overrides are dropped with the values they refine. They exist to taper the *site* defaults, and an author naming one number for one section is not asking for a taper. At 390px with the site's `mobile.x = 3`, an unoverridden section renders 48px vertical / 16px horizontal while `padding: 4` renders 24px on all four sides -- and the outer container still tapers to 16px either way. This reverses the vertical-only scoping from #186, which avoided the horizontal axis because it also feeds the outer container. Splitting the two containers removes that constraint rather than trading against it. `pnpm test` passes: 40 pages, no errors. Verified on a 1119-page site across all three cases at 1440px and 390px.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
ctaandpanelsboth declarepaddingin their bookshop schema, so authors set it and reasonably expect the section's vertical padding to change. It never did.utilities/section.htmlderived padding solely fromutilities/GetPadding.html, which reads site parameters, and no block passed anything through. The key was accepted, range-validated, and then dropped on the floor.Measured on a real site before this change — four
ctablocks carryingpadding: 4and seven carrying nothing, all identical:After
The horizontal axis is byte-identical in every case.
The empty
default:is load-bearingpaddinginherits the shared definition in mod-utilsdata/structures/_arguments.yml:Left alone,
default: 3arrives whenever a caller omits the argument — so "not supplied" is indistinguishable from "supplied", and every section that does not set one gets silently repadded. An earlier revision of this PR hit exactly that: unsetctablocks dropped frompy-5topy-3.data/structures/section.ymltherefore redeclarespaddingwith an emptydefault:.ArgsSchema.htmlmerges the local node over the global one (merge $def $val), so the empty value wins and a real nil is restored:The inherited
optionssurvive that merge, so validation is unchanged —padding: 9still fails before rendering:This keeps the author-facing name and needs no new argument.
The nil comparison in
section.htmlis deliberate for the same family of reason:withor| defaultwould discard a legitimatepadding: 0.Scope
Vertical axis only.
xis the page gutter — shared by every section and also feeding the outer container — so a per-block override there would misalign that section against its neighbours. Any mobileyoverride is dropped alongside the value it derives from, so the override then applies at every width.Two blocks only.
ctaandpanelsare wherepaddingwas purely inert.approach,articles,cards,teamandtestimonialsalso declare it, but forward it to inner partials (card group, section title) where it already means something. Making it drive section padding as well would change existing sites — a semantic decision rather than a bug fix, so it is left alone and noted here instead.Checks
4->py-4,0->py-0, omitted ->py-5,9-> rejected.pnpm test(exampleSite build) passes: 40 pages, no errors.