fix(card): decouple the horizontal card's button spacing from the section rhythm - #2150
Merged
Conversation
The horizontal branch interpolated the colour style and the caller's class with no separator between them, so a card that set both `color` and `class` emitted a single joined token: `bg-light text-bg-lightpb-0`. Both classes are lost - the colour utility the theme generated and the class the author asked for. The stacked branch a few lines down has always had the space; only this one is affected, which is why it went unnoticed. Also drop the duplicated "class" key in the stacked branch's button call. Go's `dict` keeps the last value, so the first one has never had any effect.
…adding
A horizontal card with a button reserved its bottom space with
`pb-{main.padding.y}` on the row wrapper and then pulled the button down into
that reserve with a hardcoded `mb-n4`. `main.padding.y` is the site-wide
*section* rhythm, so the geometry inside a single card became a function of the
spacing between sections. It was not meant to be: the class arrived as a
literal `pb-5` alongside the button itself, and a later commit that made the
section padding configurable swept the literal up with the genuine
section-level ones.
Two things follow from the coupling. The gap under the button is whatever
`main.padding.y - 1.5rem` happens to be, so raising the section rhythm silently
inflates every horizontal card. And below 4 the subtraction goes negative and
the button hangs out of the card, so a value the padding validator accepts
breaks the layout.
Neither is reachable by the author, because `class` lands on the `.card`
element and the padding sits on the row wrapper one level down. The card's own
`padding` argument - already cascading, already documented, already the control
the stacked branch uses for exactly this job at the other end of the file - had
no say at all.
Space the button with `pt-{padding}` and drop both the wrapper reserve and the
negative margin, which makes the two branches agree and puts the spacing back
under the argument that names it. `padding=0` now means no gap, as it reads.
At the theme's own default padding of 4 the space under the button is
unchanged; what a stock site gains is the card's padding between the body text
and the button, where the negative margin used to leave none. Sites that raised
the section rhythm above 4 lose the extra band it was adding, and a card that
wants it back can ask for it with a larger `padding`.
`wrapper` is declared in the card structure, so it is offered to authors and picked up by the CMS snippet generator, but the shortcode initialised its local `$wrapper` to the empty string and only ever assigned to it when `scroll` was set. Whatever the author wrote was dropped before the partial saw it. The card-group shortcode passes its own `wrapper` straight through, so the two have disagreed on an argument they share. Read the argument, and let the scroll classes prepend to it rather than replace it, so a scrolling card can carry an author class too.
Nothing in the build read the card's button markup, and no page in the theme's own content renders a horizontal card with a button, so the drift that coupled a card's internal spacing to the site's section rhythm had nowhere to show up. These cases give that path a reader. The harness sets `main.padding.y` to a value the card would otherwise have emitted, so a reintroduced coupling is legible rather than merely absent: the row wrapper is asserted in full, and the negative margin that paired with it is pinned as its own column. The padding cases run at 0, 2, 3 and 5 - the default alone would also be satisfied by a card that hardcoded the old middle value, while the ends are only reachable by one that reads the argument. Verified by reverting both templates to their previous state, which fails six of the cases and exits non-zero.
markdumay
force-pushed
the
fix/card-horizontal-button-spacing
branch
from
September 3, 2026 05:12
94874c1 to
9fd3eb4
Compare
✅ Deploy Preview for gethinode-demo ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Collaborator
Author
|
🎉 This PR is included in version 3.23.7 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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
1 participant
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.
Problem
A card with a button reserved the space beneath it with
pb-{main.padding.y}on the row wrapper, then pulled the button down into that reserve with a hardcodedmb-n4:main.padding.yis the rhythm between sections of a page. Using it here made the geometry inside a single card a function of a setting that has nothing to do with cards.git log -Lon the line shows how it got there: the class arrived as a literalpb-5in564a62e7("Add optional button to card"), hand-tuned against the button's negative margin.fe0d3553("Make padding globally configurable") then swept that literal up with the genuine section-level paddings.Three things follow.
The gap tracks a setting that isn't about cards. It comes out as
main.padding.y - 1.5rem, so raising the section rhythm inflates every card that carries a button. Below 4 the subtraction goes negative and the button hangs out of the card — so values the padding validator accepts break the layout.No author can reach it.
classlands on the.cardelement and the padding sits on the row wrapper one level down, so no utility class overrides it. The card's ownpaddingargument — cascading, documented, and already the control the stacked branch uses for exactly this job — had no say.classwas destroyed anyway. The horizontal branch interpolated the colour style and the caller's class with no separator, so a card setting bothcolorandclassemitted one joined token:text-bg-lightpb-0. Both classes are lost. The stacked branch has always had the space; only this one is affected.Found on a client site whose section rhythm is 5, where every horizontal card with a button carried a band of white space the editor could not remove —
class="pb-0"landed on the wrong element and was mangled on the way there.Change
Space the button with
pt-{padding}and drop both the wrapper reserve and the negative margin. That makes the two branches agree, and puts the spacing under the argument that names it —padding=0now means no gap, as it reads.Also in scope, both found while tracing the above:
classseparator on the horizontal.cardelement, plus a duplicated"class"key in the stacked branch's button call that Go'sdicthas always discarded.wrapperpassthrough in the card shortcode. It is declared in the card structure, so it reaches authors and the CMS snippet generator, but the shortcode initialised its own$wrapperto""and only assigned to it for scrolling cards — the author's value never reached the partial.card-grouppasses its identically-named argument straight through, so the two have disagreed on an argument they share.Tests
No page in the theme's own content renders a horizontal card with a button, which is why this drifted unnoticed.
tests/templatesnow covers the path: seven cases over the.cardclasses, the row wrapper, the button's spacing column and the negative margin, plus a fixture page for the shortcode argument.The harness sets
main.padding.y = 5deliberately — a value the card would otherwise have emitted — so a reintroduced coupling shows up as apb-5the assertions name, rather than as a silent absence. Padding runs at 0, 2, 3 and 5: the default alone would also be satisfied by a card that hardcoded the old middle value, while the ends are only reachable by one that actually reads the argument.Reverting both templates fails six of the cases and exits non-zero. On the fix,
hugo -s tests/templatesis green (79 PASS, 0 FAIL), and CI runs it —test:templatesis part ofbuild-command, so it executed on all six platform/node combinations.Compatibility
Patch. Cosmetic, and confined to where the button sits.
At the theme's default
padding.y = 4, with the card's defaultpadding = 3:pt-3→ 1rempb-4+p-3−mb-n4→ 1remp-3→ 1remSo a stock site keeps the space under the button unchanged and gains the card's own padding above it, where the negative margin previously left none. Sites that raised
main.padding.yabove 4 lose the extra band it was adding; a card that wants it back can ask with a largerpadding. Sites below 4 stop having the button hang out of the card.Sites that had worked around the mangled
classby relying on the joined token will see the colour utility apply correctly again.