fix(sidebar): set the icon indent in markup, not with :has() - #2105
Merged
Conversation
Reported after #2103 shipped: the child items visibly shift on load. The sidebar HTML is cached per section and the collapse trail is expanded client side, so the nested list goes from display:none to visible after first paint. A `:has()` selector has to be re-resolved at that point, and a browser is free to paint the old `ps-3` first and correct it a frame later - which is exactly what the shift looks like. The template already knows whether the group has an icon, so it now emits `sidebar-nav-icon-indent` in place of `ps-3` and the rule is a plain class selector. The value is in the cached markup, so it is right from the first paint with no re-resolution and no dependence on when the collapse opens. Geometry is unchanged from #2103 - Guides 104, Organization 104, Invite a Member 120, with the icon-bearing lists at 24px and the rest at 16px. Groups without an icon keep `ps-3` exactly as before, now by emitting a different class rather than by failing to match a selector.
✅ Deploy Preview for gethinode-demo ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
markdumay
enabled auto-merge
August 1, 2026 15:55
Collaborator
Author
|
🎉 This PR is included in version 3.14.1 🎉 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.
Fixes a layout shift introduced by #2103: the child items visibly move on load.
Cause
The sidebar HTML is cached per section, and
critical/sidebar-active.jsexpands the collapse trail client side — the served markup ships<div class="collapse">with noshow. So the nested list goes fromdisplay:noneto visible after first paint.#2103 derived the indent with
:has(). That selector has to be re-resolved when the subtree becomes visible, and a browser is free to paint the oldps-3first and correct it a frame later. That is what the shift is.I could not reproduce it in Chromium —
layout-shiftentries came back empty, CLS 0.00000 — so this fixes the mechanism rather than a symptom I observed. The reasoning stands on its own: a value that must be recomputed as a subtree is revealed is inherently a worse choice here than one that is already in the markup.The fix
The template already knows whether the group has an icon (
$pre), so it emits the class instead of CSS deriving it:The class is in the cached HTML, so it is correct at first paint with no re-resolution, no dependence on when the collapse opens, and no
!importantneeded. Groups without an icon keepps-3exactly as before — now by being emitted a different class rather than by failing to match a selector.Evidence
Verified the class is present in the served markup, before any JS:
Geometry is unchanged from #2103:
Guides(has icon)OrganizationInvite a MemberIcon-bearing lists compute 24px, the rest 16px, and no
:has()rule targetingbtn-toggle-navremains in the stylesheet.pnpm testpasses.