Skip to content

fix: NavigationBar no longer reserves space for absent start content#664

Merged
adrienzheng-cb merged 5 commits intomasterfrom
adrien/fix-nav-bar
May 6, 2026
Merged

fix: NavigationBar no longer reserves space for absent start content#664
adrienzheng-cb merged 5 commits intomasterfrom
adrien/fix-nav-bar

Conversation

@adrienzheng-cb
Copy link
Copy Markdown
Contributor

@adrienzheng-cb adrienzheng-cb commented May 5, 2026

Problem

Two related layout bugs in NavigationBar:

  1. Phantom gap when no start is provided. The Collapsible wrapping the start slot was
    always a flex child of the outer HStack. CSS gap applies to all flex items regardless of
    size — so even when the Collapsible was fully collapsed (zero width), it still contributed a
    leading gap before the page title.

  2. Start slot transition was jarring. The CSS gap between the Collapsible and the title is
    static — it snapped to full size on expand and disappeared abruptly on collapse. This made the
    transition feel sudden rather than smooth.

Changes

Collapsible — new collapsedStyle prop

Adds collapsedStyle?: 'visibility-hidden' | 'display-none' to control how the element is hidden
after the collapse animation completes.

  • 'visibility-hidden' (default) — existing behavior, element stays in the layout flow
  • 'display-none' — applies display: none, fully removing the element from layout (no flex gap,
    not in the accessibility tree, not focusable)

The hidden-state tracking is unified into a single isHidden boolean (replacing the previous
visibility string state), cleared immediately on expand and set in onAnimationComplete after
collapse.

NavigationBar — layout restructure

  • collapsedStyle="display-none" on the start Collapsible — when no start is provided the
    element has display: none from the start and contributes no gap.
  • paddingEnd restored on the inner start HStack — spacing between the start content and the
    title now lives inside the Collapsible, so it animates smoothly from 0 → columnGap on expand
    and back on collapse. The outer HStack gap is set to 0 to prevent double spacing.
  • Content + end wrapped in an inner HStack — preserves columnGap spacing between the title
    and end content without relying on the outer HStack's gap.

Story

Adds NavigationBarStartToggle — a story with a button that toggles the start slot on and off,
demonstrating both the gap fix and the smooth animated transition.

Consumer impact

  • CollapsiblecollapsedStyle defaults to 'visibility-hidden', so all existing usages
    are unaffected.
  • NavigationBar — no API changes. Consumers rendering without a start prop will see the
    phantom gap disappear. Consumers using start will see the gap between start content and title
    animate smoothly (previously snapped).

UI changes

new nav bar story

Testing

How has it been tested?

  • Unit tests
  • Interaction tests
  • Pseudo State tests
  • Manual - Web
  • Manual - Android (Emulator / Device)
  • Manual - iOS (Emulator / Device)

Testing instructions

Illustrations/Icons Checklist

Required if this PR changes files under packages/illustrations/** or packages/icons/**

  • verified visreg changes with Terran (include link to visreg run/approval)
  • all illustration/icons names have been reviewed by Dom and/or Terran

Change management

type=routine
risk=low
impact=sev5

automerge=false

@adrienzheng-cb adrienzheng-cb requested a review from hcopp May 5, 2026 17:45
@cb-heimdall
Copy link
Copy Markdown
Collaborator

cb-heimdall commented May 5, 2026

✅ Heimdall Review Status

Requirement Status More Info
Reviews 1/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 1
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1
CODEOWNERS ✅ See below

CODEOWNERS

Code Owner Status Calculation
ui-systems-eng-team 1/1
Denominator calculation
Additional CODEOWNERS Requirement
Show calculation
Sum 0
0
From CODEOWNERS 1
Sum 1

*
* @default 'visibility-hidden'
*/
collapsedStyle?: 'visibility-hidden' | 'display-none';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it is beneficial in some cases to still support visibility rather than display? I could see us using our new breaking change policy to go ahead and change to display none.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call. switched to always using display:none.

Comment thread packages/web/src/collapsible/Collapsible.tsx Outdated
Copy link
Copy Markdown
Contributor

@hcopp hcopp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes look great, thank you. A lot of changes in visreg (> 10 for NavBar as expected + a few flaky ones), could in the future combine stories for NavBar so we have one screenshot.

@adrienzheng-cb adrienzheng-cb merged commit 2cfbe9d into master May 6, 2026
35 checks passed
@adrienzheng-cb adrienzheng-cb deleted the adrien/fix-nav-bar branch May 6, 2026 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants