Skip to content

fix(AppShell): animate nav surface enter/exit (#4174)#4194

Closed
cixzhang wants to merge 1 commit into
mainfrom
fix/appshell-nav-animate
Closed

fix(AppShell): animate nav surface enter/exit (#4174)#4194
cixzhang wants to merge 1 commit into
mainfrom
fix/appshell-nav-animate

Conversation

@cixzhang

Copy link
Copy Markdown
Contributor

What

Animate AppShell navigation surfaces (TopNav header + inline SideNav) so they fade and slide in when they appear, instead of snapping in instantly. The motion reuses the same tokens MobileNav already uses (--duration-medium / --ease-standard) so all shell nav motion is coordinated.

Closes #4174.

Why

MobileNav already slides via a transform + token-driven transition and honors prefers-reduced-motion. The other shell nav surfaces had no enter transition — showing a TopNav/SideNav popped in with no motion, which looked abrupt next to the drawer's slide. This brings the surface show under one motion language.

How

  • Replaced the unused, non-animatable styles.hidden = { display: 'none' } with two CSS-only enter animations built on @starting-style:
    • navSurfaceHeaderEnter — header/top bar fades + slides down (translateY(-100%) → 0).
    • navSurfaceSideEnter — inline SideNav fades + slides in from the inline-start edge (translateX(-100%) → 0, RTL-aware via :is([dir="rtl"] *)).
  • Both use --duration-medium / --ease-standard (no raw ms / cubic-bezier strings) and collapse to near-instant under @media (prefers-reduced-motion: reduce), matching MobileNav's pattern exactly.
  • Applied to the TopNav header wrapper, the sidenav-only auto mobile top bar, and the inline SideNav panel (fill mode) / sticky wrapper (auto mode).
  • No public API added — presence stays expressed by passing the slot or undefined; the animation is automatic.
  • StyleX only; verified the @starting-style (including the nested RTL transform) compiles to valid CSS via the build.

Accessibility

  • Nothing is unmounted or made non-focusable to express visible state — a shown surface is mounted and focusable the whole time, so focus and tab order are unaffected.
  • A hidden surface is fully unmounted (presence-based), so there is no off-screen focusable/tabbable nav.
  • prefers-reduced-motion is honored: the transition duration collapses to 0.01s so surfaces appear immediately with no motion.

Testing

  • pnpm build — passes (StyleX compiles the @starting-style rules, including the RTL variant, to valid CSS).
  • pnpm test — passes (AppShell: 38/38).
  • pnpm lint — passes.
  • Added a Toggle Nav Surfaces Storybook story that shows/hides the TopNav and SideNav so reviewers can see each surface animate in, and verify the reduced-motion behavior.

Notes / scope

@vercel

vercel Bot commented Jul 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
astryx Ready Ready Preview, Comment Jul 22, 2026 11:46am

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jul 22, 2026
@github-actions github-actions Bot added the needs:design-review Affects visuals — Design should review label Jul 22, 2026
github-actions Bot added a commit that referenced this pull request Jul 22, 2026
@github-actions

Copy link
Copy Markdown
Contributor

PR Analysis Report

📚 Storybook Preview

View Storybook for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

🧪 Sandbox Preview

View Sandbox for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

Modified Components

AppShell · View in Storybook
Metric Before After Delta
Bundle Size (ESM) N/A N/A N/A
Lines of Code N/A 596 -
Complexity N/A Very High (91) -

Bundle Size Summary

Package Size (ESM) Size (CJS) Gzipped
@astryxdesign/core N/A 4.7KB 0B

Accessibility Audit

Status: 1 accessibility violation(s) found — 1 serious.

AppShell - 1 issue(s)
  • 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
    • Rule: color-contrast · Affects 10/11 stories · Learn more
    • WCAG: 1.4.3 (Level AA)

Generated by PR Enrichment workflow | Storybook | Sandbox | View full report

@cixzhang

Copy link
Copy Markdown
Contributor Author

Self-review — Build Protocol Quick Self-Review (refactor/update checklist)

  1. Icons from registry — n/a; no icons added or changed.
  2. themeProps present — no new interactive elements. The animated wrappers are the existing header / sidenav panel containers, which already carry themeProps('app-shell-header' | 'app-shell-sidenav').
  3. No wrapper divs — no new wrapper DOM. The enter animation is applied to existing elements (header wrapper, LayoutPanel via xstyle, and the pre-existing auto-mode sticky wrapper). No extra nodes introduced.
  4. No CSS shorthands — used transitionProperty / transitionDuration / transitionTimingFunction individually (no transition shorthand); no flex shorthand touched.
  5. ARIA still correct — no primitives swapped; DOM structure and roles unchanged. AppShell tests (38) pass, including nav semantics/labels.
  6. Tests pass locallypackages/core/src/AppShell/AppShell.test.tsx 38/38 green; pnpm build green; pnpm test / pnpm lint run as gates.
  7. Sandbox/stories updated — added a Toggle Nav Surfaces story so reviewers can see each surface animate in and verify reduced-motion. No props removed, so no callsite cleanup needed.
  8. Popover background — n/a; no popover/layer involved.

Additional notes for reviewers

  • Motion tokens only--duration-medium + --ease-standard (no raw ms / cubic-bezier), matching MobileNav. Reduced-motion collapses the transition to 0.01s via @media (prefers-reduced-motion: reduce), exactly like MobileNav.
  • StyleX capability check — the @starting-style enter transforms (including the RTL variant :is([dir="rtl"] *) nested inside @starting-style) compile to valid CSS; verified in dist/astryx.css. @starting-style + transform + reduced-motion are all listed as supported in the StyleX capabilities reference.
  • A11y — nothing is unmounted or made non-focusable to represent visible state; a shown surface is fully present/focusable. A hidden surface is presence-based (unmounted), so there is no off-screen focusable nav.
  • Scope / known limitation — this implements the enter animation. A true exit animation would require keeping a hidden surface mounted through a transition (mounted-through-exit) or React <ViewTransition> (feat: View Transitions for AppShell and collapsible side nav #334) — a larger, spec-sized change deliberately left out to keep this patch-level. Hiding is an instant unmount, which the Animation System exploration notes is acceptable. @starting-style also fires on the shell's first mount, so surfaces animate in on initial load — consistent with existing @starting-style usage across the codebase (Toast, Dialog, TopNavMegaMenu).

@cixzhang

Copy link
Copy Markdown
Contributor Author

On the initial mount, we don't want the animation to fire. It should only fire when the navigation is hidden later. I think it could be helpful to add a storybook story where the top and sidenavs are hidden/shown. Potentially we need to work this into the design of the API as well, to make each area of the shell layout optionally hidden and inert.

@cixzhang

Copy link
Copy Markdown
Contributor Author

Closing this in favor of the broader direction. The reviewer feedback here reframed nav enter/exit animation into a coordinated Focus Mode, which has grown into a general AppShell region-transform plugin system — tracked in #4255 and owned there. Closing to hand it over; #4174 stays open under that umbrella.

@cixzhang cixzhang closed this Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot. needs:design-review Affects visuals — Design should review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AppShell: animate nav surface enter/exit (SideNav/TopNav hide-show, coordinate MobileNav slide)

1 participant