Skip to content

Animate sub navigation items - #93

Merged
thomasgallagher merged 2 commits into
mainfrom
fix/navigation-section-chevron-no-transition-on-load
Jun 7, 2026
Merged

Animate sub navigation items#93
thomasgallagher merged 2 commits into
mainfrom
fix/navigation-section-chevron-no-transition-on-load

Conversation

@thomasgallagher

Copy link
Copy Markdown
Collaborator

Suppress chevron transition when restoring navigation section state on load.

Restored open/closed state was animating the section chevron on page load. Apply a temporary --no-transition modifier while setting the initial aria-expanded state, then remove it after two animation frames so the chevron renders in position without animating.

…n load

Restored open/closed state was animating the section chevron on page
load. Apply a temporary --no-transition modifier while setting the
initial aria-expanded state, then remove it after two animation frames
so the chevron renders in position without animating.
@claude

claude Bot commented Jun 7, 2026

Copy link
Copy Markdown

Review

This PR adds a slide-down animation for navigation section items when a user opens a section, and suppresses the chevron's CSS transition when restoring open/closed state from localStorage on page load (using a temporary --no-transition modifier removed after two animation frames).

Issues

Severity File Line(s) Description
HIGH engine.css 751-752 transition-none added to the [aria-expanded="false"] selector kills the chevron's close animation for all user-initiated collapses, not just page-load restores. The base .l-ui-navigation__section-chevron rule has transition-transform duration-150. Per the CSS Transitions spec, the transition is determined from the after-change style: when the user closes a section, aria-expanded becomes "false", the after-change state matches this selector, and transition-none overrides the base transition - so the chevron jumps instantly to -rotate-90 without animating. The separately-added --no-transition modifier class already handles the load-restore case; transition-none in the [aria-expanded="false"] selector is redundant for that purpose and should be removed.

Verdict

🚫 Changes required - the transition-none addition to the collapsed-state selector is an undocumented regression that removes the chevron's closing animation for all user-driven collapses.

@claude

claude Bot commented Jun 7, 2026

Copy link
Copy Markdown

PR adds a slide-in animation for navigation section items on user-initiated open, and suppresses the chevron transition when restoring saved open/closed state on page load.

The implementation is clean. A few things I verified:

  • double rAF pattern for removing --no-transition is correct; the browser paints the initial state before transitions re-enable.
  • animationend bubbling: the listener is on panel but the animation runs on > li children - animationend does bubble, so cleanup fires correctly on the first li that finishes.
  • prefers-reduced-motion: the global rule at line 197 (animation-duration: 0.01ms !important) covers the new keyframe without a dedicated guard, so animationend still fires and the --opening class is still cleaned up under reduced-motion.
  • rapid toggle: stacking multiple animateOpen calls before animationend fires leaves extra { once: true } listeners, but each is a no-op after the class is removed, so no leak.

No issues found.

Ship

@thomasgallagher
thomasgallagher merged commit 6f79bb7 into main Jun 7, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant