fix(nav): collapse a responsive button group, and give every nav a first entry - #2158
Merged
Conversation
`responsive` hides the nav controls below the breakpoint and renders a
dropdown in their place, but the hide applied to every tab type while the
dropdown was gated on the type not being a button group. A responsive
`tab-type="buttons"` therefore had no controls at all at narrow widths --
the panes still rendered, with no way to reach any but the active one --
and since `responsive` defaults to true, that was what a button group did
unless its author opted out.
Render the companion for every type, so the two gates agree. The button
group announces it through `data-companion` the way the list already
announces its own, and its toggle takes the button treatment
assets/table.html gives the filter group it collapses: a control standing
in for buttons has to read as one rather than as body copy.
Two identifier defects sat underneath, because the dropdown carries no
behavior of its own and works only by replaying a click on the control its
`data-link` names. `id` is documented as the "unique identifier of the
current element" but named no element: the list took `nav-{id}` and a
button group took nothing, so `#{id}` resolved to nothing at all. The
wrapper now carries the caller's value verbatim, and only when one was
given -- stamping the shared fallback there would repeat a single id down
the page.
The two branches also spelled `{id}-btn-{index}` differently, the button
group from the argument's fallback and the list and the `nav-show` lookup
from the raw argument. A nav rendered without an id emitted
`id="%!s(<nil>)-btn-0"` while pointing its panes and its companion at
`0-btn-0`, so `nav-show` could never match and the companion could never
address a control. Both now read the same variable.
Assertions in tests/templates cover all three, at the partial and through
the shortcodes, along with guards for an opted-out group that must not
gain a dropdown it did not ask for and an id-less nav that must not stamp
the fallback on its wrapper.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
assets/nav.html has always marked the first control active when `nav-show` is unset, but a pane's `active` class came from the item's own `show` and from nothing else. An author who marked no item -- which the shortcode never required and never warned about -- got a nav with a highlighted tab above an empty content area, since `.tab-pane` is display:none until something activates it. An item cannot settle this alone. It renders in document order and cannot know whether a later sibling will claim `show`, so the fallback can only be resolved where the whole set is visible. The nav-item shortcode now hands its parent a description of its pane rather than the rendered markup, and the nav shortcode resolves the entry once -- an explicit `show` if any item reported one, its first entry otherwise -- before rendering the panes and passing the same value on as `nav-show`. Control, pane and companion dropdown then read one decision instead of each defaulting on its own, and an author who did mark an item still wins. The accordion rendering keeps its eager path. Nothing in the theme reads `alternative`, but an override of the nav shortcode reaches it the way it reached `inner`, so the contract is left as it was. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The nav-item shortcode reads `fade` off its parent and falls back to it when the item sets none, and the shared argument definition marks `fade` as a cascading one. The nav structure never declared it, so the cascade could not be reached: InitArgs rejected `fade` on the group as an unsupported argument, `$args.err` was set, and assets/nav.html rendered nothing at all -- an author reaching for the documented cascade lost the whole nav rather than gaining an animation. Declare it, scoped to the shortcode group, since the cascade lives in the item shortcode and a partial caller renders its own panes. The behaviour it unlocks is the item shortcode's existing one: the group's value is or-ed with the item's, so a group that sets `fade` fades every pane and an item cannot opt back out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
✅ 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.24.0 🎉 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.
Three defects in the nav component, found while tracking down why a
tab-type="buttons"group'sidnever appeared in the rendered HTML.1. A responsive button group lost its controls instead of collapsing them
responsivehides the nav controls below the breakpoint and renders a dropdown in their place, but the hide applied to every tab type while the dropdown was gated on the type not being a button group. A responsivetab-type="buttons"therefore had no controls at all at narrow widths — the panes still rendered, with no way to reach any but the active one.responsivedefaults totrue, so that was what a button group did unless its author opted out.The companion now renders for every type, so the two gates agree. The button group announces it through
data-companionthe way the list already announces its own, and its toggle takes the button treatmentassets/table.htmlgives the filter group it collapses.Two identifier defects sat underneath, because the dropdown carries no behaviour of its own and works only by replaying a click on the control its
data-linknames:idnamed no element. It is documented as the "unique identifier of the current element", but the list tooknav-{id}and a button group took nothing, so#{id}resolved to nothing at all. The wrapper now carries the caller's value verbatim, and only when one was given — stamping the shared fallback there would repeat a single id down the page.{id}-btn-{index}differently, the button group from the argument's fallback and the list and thenav-showlookup from the raw argument. A nav rendered without an id emittedid="%!s(<nil>)-btn-0"while pointing its panes and its companion at0-btn-0.2. A nav whose items were all unmarked showed an empty content area
assets/nav.htmlhas always marked the first control active whennav-showis unset, but a pane'sactiveclass came from the item's ownshowand from nothing else. An author who marked no item — which the shortcode never required and never warned about — got a highlighted tab above nothing, since.tab-paneisdisplay:noneuntil something activates it.An item cannot settle this alone: it renders in document order and cannot know whether a later sibling will claim
show.nav-itemnow hands its parent a description of its pane rather than rendered markup, andnavresolves the entry once — an explicitshowif any item reported one, its first entry otherwise — before rendering the panes and passing the same value on asnav-show. Control, pane and companion then read one decision, and an author who did mark an item still wins.The accordion rendering keeps its eager path: nothing in the theme reads
alternative, but an override of the nav shortcode reaches it the way it reachedinner, so that contract is left as it was.3.
fadeon the group aborted the navnav-itemreadsfadeoff its parent and falls back to it, and the shared argument definition marksfadeas cascading — but the nav structure never declared it. Writing it on the group did not cascade, it aborted: InitArgs rejected it as unsupported,$args.errwas set, andassets/nav.htmlrendered nothing. Declaring it makes the cascade the item shortcode already implements reachable.Verification
27 assertions added to
tests/templates, across four sections — at the partial, and through the shortcodes where the identifier chain and the fallback only exist. Each was run against the unfixed templates:NAV— companion gating, identifiersNAV CHAIN— wrapper → control → pane →data-linkNAV START— the fallback entryNAV FADE— the cascadeunsupported argument 'fade')The guards are deliberate: they hold what a naive repair would break — an opted-out group gaining a dropdown it did not ask for, an id-less nav stamping the shared fallback on its wrapper, and the first-entry fallback overriding an author who did mark an item.
Also driven in a real browser (Chromium,
hugo server), 21 checks across the three fixes:fade—transitionPropertyincludesopacity, intermediate opacities sampled through the switch, ends opaque, outgoing pane gone. Not an instant swap and not a transparent ghost.pnpm testandpnpm buildboth exit 0.Note on the version marker
release: v3.24.0on the newfadeargument assumes this lands as the next minor, which thefeat:commit produces. Happy to change it if the release lines up differently.🤖 Generated with Claude Code