fix(nav): the sidebar section label was not a control - #840
Merged
Conversation
Clicking "Fund" / "Coordinate" / "Finance" did nothing — the label was an <h3> in a <div> and only the small chevron beside it carried the onClick. Three defects in the same family, all of them "the click did nothing": 1. The whole header row is now one <button>. A word that looks like a control but isn't reads as a dead click, and the arrow alone was a 44px target inside a full-width row of affordance. Second time this class landed in this sidebar (see ContextSwitcher, "not a dead pixel"), so it gets a guard rather than a third fix. 2. Collapsing the section holding the active page was a visual no-op: the render gate was `!isCollapsed || hasActiveItem`, so the chevron flipped to > while the list stayed open. An explicit toggle now wins. 3. Expanding every section silently reverted on reload — an empty saved set was read as "nothing was ever saved" and replaced by the defaults. Key presence, not set size, now decides. Also removes an SSOT violation found next door: buildInitialCollapsedSections branched on viewport (`isMobile ? section.priority > 3 : ...`), so on a phone the declared `defaultExpanded` was ignored in favour of a bare magic number and the same account saw different sections open on phone than on laptop. `defaultExpanded` is now the only input — which does mean Create and Fund start collapsed on mobile, matching the generator's documented intent. 5 tests, each verified by reverting the source and confirming it fails. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XeELB8b3N4JrT2asYL9WvE
A heading sitting BESIDE a chevron-only toggle is a dead click: the visitor aims at the word and nothing happens, while the only real target is a small arrow in a full-width row of apparent affordance. This shape has now been reported twice in this one sidebar — on the ContextSwitcher avatar (two separate visitors, see its test's header) and on the Fund / Coordinate / Finance section headers, fixed in the previous commit. Per the never-twice rule the third occurrence should be impossible, not cheap, so the class gets a check rather than another sweep. check:dead-labels scans src/components for a heading followed within 12 lines by a <button> containing a Chevron, with no heading inside that button. The correct shape — label INSIDE the control, which HeaderNavigation already does — does not trip it. Zero hits across src/components today, so the sidebar was the last one. Mutation-proven both ways: restoring the pre-fix SidebarNavigation makes it exit 1 naming that exact line, and the current tree exits 0. A gate that has never been shown to fail is not a gate. Note for the fleet: this covers AUTHED surfaces, which the central ui-defect-audit.mjs structurally cannot reach — it renders public entry pages only. The two are complementary, not duplicates. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XeELB8b3N4JrT2asYL9WvE
catomean
force-pushed
the
worktree-sidebar-nav-click
branch
from
August 29, 2026 09:33
41d8eec to
3cb2f6c
Compare
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.
Clicking the words Fund, Coordinate or Finance in the sidebar did nothing. The label was an
<h3>inside a<div>; the only thing carrying theonClickwas the small chevron beside it. Reported by George.Reading the surrounding code turned up two more defects in the same family — all three are "the click did nothing" — plus one SSOT violation next door.
What was wrong
1. The section label was not a control. The whole header row is now a single
<button>wrapping both the label and the chevron, witharia-expandedand the chevron markedaria-hidden. A word that looks like a control but isn't reads as a dead click, and the arrow alone was a 44px target sitting inside a full-width row of apparent affordance.2. Collapsing the section holding the active page was a visual no-op. The render gate was
!isCollapsed || hasActiveItem, so collapsing "Fund" while on/projectsflipped the chevron to ▶ and left the list open — the arrow lied about the state. An explicit toggle now wins.3. Expanding every section silently reverted on reload.
useNavigationStorageusedcollapsedFromStorage.size > 0 ? saved : defaults, so an empty saved set — meaning "the user opened everything" — was indistinguishable from "nothing was ever saved". Key presence, not set size, now decides. (FleetCrown already reads its equivalent state with!== null; the correct answer existed in the fleet and hadn't travelled.)4.
defaultExpandedwas not actually the source of truth.buildInitialCollapsedSectionsbranched on viewport —isMobile ? section.priority > 3 : !section.defaultExpanded— so on a phone the declared config was ignored in favour of a bare magic number, and the same account saw different sections open on phone than on laptop with nothing recording why.Deliberate behaviour change
Making
defaultExpandedauthoritative means Create and Fund now start collapsed on mobile, where they previously started open. This matches the generator's own documented intent ("all groups collapsed by default, progressive disclosure").Recommended consciously, not incidentally: at 390px the drawer now opens showing Main's six items plus six section headers — about one screen, no scrolling — and the first thing a user reads is the shape of the product rather than 28 nouns. The cost is one extra tap to the long tail, paid once per device since the state persists, while the five highest-frequency destinations already sit in the mobile tab bar. This is only a safe default because of fix 1; a collapsed section is acceptable only when the way to open it is obvious. Changing it back is now a one-field edit in
config/navigation.tsrather than a magic number in a hook.Tests
Five tests across two suites, each verified by reverting the source and confirming it fails — 3 fail against the old component, 2 against the old hook.
Gate
scripts/check-dead-labels.mjs, wired intonpm run verify. This shape has now been reported twice in this one sidebar (the ContextSwitcher avatar, and these section headers), so per the never-twice rule the third occurrence should be impossible rather than cheap. Zero hits acrosssrc/componentstoday. Mutation-proven both ways: exits 1 naming the exact line when the pre-fix component is restored, exits 0 on the current tree.It covers authed surfaces, which the fleet's central
ui-defect-audit.mjsstructurally cannot reach — that one renders public entry pages only. Complementary, not duplicate.Verification
tscclean · eslint clean · 10 nav/sidebar/layout suites green (71 tests) · full pre-commit gate passed on both commits.Not done: I have not viewed this rendered in a browser. Everything above is from the code and the tests.
🤖 Generated with Claude Code
https://claude.ai/code/session_01XeELB8b3N4JrT2asYL9WvE