UI: Improve landmark navigation #33457

Sidnioulz · 2026-01-02T20:57:55Z

Closes #32271

What I did

Peek.2026-01-09.19-27.webm

Aligned site structure with feedback from accessibility experts who audited our recent a11y changes
Applied landmarks to
Sidebar (header element, banner role, was previously a nav)
Search (search role)
Explorer (nav role)
Preview (main role)
Toolbar (region role)
Addon panel (aside role)
In mobile, applied header/banner to the MobileNavigation bottom bar
Added global keyboard shortcuts for landmark nav (F6 / Shift+F6) based on React Aria, so that everyone can keyboard nav like a pro
Added focus outline specific to landmark regions so they can be spotted (reviewed with @MichaelArestad)
Fixed focus outline visibility on preview region
Fix issue with nav being unreachable when Search is focused
~~Fix preview iframe eating up F6 key events~~ (can't find a way)

Still gotta handle a few focus / keyboard handling edge cases.

Checklist for Contributors

Testing

Not tested

Manual testing

With screen reader

Run Storybook with NVDA or VoiceOver on
Use your screen reader's landmark navigation shortcuts (NVDA: D and Shift+D, when in browse mode)
NVDA will show a decorative outline (NVDA baked-in feature), VoiceOver won't

Without screen reader

Run Storybook in your favourite browser
Press F6 and Shift+F6 to quick nav between landmarks, using React Aria's hook
Notice the highlight that helps you spot in which region you are
Notice the quick nav remembers which element of the landmark region you were last on, and returns to that element
This quick nav also does not suffer from limitations between browse mode and focus mode in screen readers

Limitations

Can't quick nav from within the preview iframe with React Aria shortcut
Can't change the keyboard shortcut, it is hardcoded by React Aria, so we can't show it in the shortcuts UI for it to be customised
Quicknav feature is undocumented at the moment

Documentation

@kylegach @jonniebigodes @MichaelArestad I'd like to get your opinions on how to make this feature discoverable to users. Should we add it in the shortcuts page, even though it can't be edited, just for visibility? Is there a best page to document it in the docs website?

Checklist for Maintainers

When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts
Make sure this PR contains one of the labels below:
Available labels
- bug: Internal changes that fixes incorrect behavior.
- maintenance: User-facing maintenance tasks.
- dependencies: Upgrading (sometimes downgrading) dependencies.
- build: Internal-facing build tooling & test updates. Will not show up in release changelog.
- cleanup: Minor cleanup style change. Will not show up in release changelog.
- documentation: Documentation only changes. Will not show up in release changelog.
- feature request: Introducing a new feature.
- BREAKING CHANGE: Changes that break compatibility in some way with current major version.
- other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>

Summary by CodeRabbit

New Features
- Added accessibility landmarks for keyboard navigation (F6 key) with visual border highlighting.
- Enhanced semantic HTML structure across the interface for better accessibility.
- Added support for custom outline properties in Card component.
Bug Fixes
- Updated skip link labels for improved clarity.
Style
- Added global styling for landmark navigation indicators.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

nx-cloud · 2026-01-06T09:09:59Z

View your CI Pipeline Execution ↗ for commit 7feac0b

Command	Status	Duration	Result
`nx run-many -t compile,check,knip,test,pretty-d...`	⛔ Cancelled	8m 14s	View ↗

☁️ Nx Cloud last updated this comment at 2026-01-09 18:44:29 UTC

coderabbitai · 2026-01-09T18:40:03Z

📝 Walkthrough

Walkthrough

Adds accessibility landmark navigation via new @react-aria/landmark dependency. Introduces useLandmark and useLandmarkIndicator hooks to manage semantic regions across manager UI components. Integrates landmarks into sidebar, panel, preview, and toolbar. Updates Card with outlineProps prop. Refactors Search render-props API and updates skip-link labels.

Changes

Cohort / File(s)	Change Summary
Dependency Management `package.json`	Added `@react-aria/landmark` v^3.0.8 to dependencies and devDependencies
New Landmark Hooks `src/manager/hooks/useLandmark.ts`, `src/manager/components/layout/useLandmarkIndicator.ts`	Created wrapper hook `useLandmark()` around upstream react-aria landmark; created `useLandmarkIndicator()` to listen for F6 key, find parent landmarks via `data-sb-landmark` attribute, and animate border highlight (2px solid) using theme primary color for 1.5s
Sidebar Components - Landmark Integration `src/manager/components/sidebar/Explorer.tsx`, `src/manager/components/sidebar/Sidebar.tsx`, `src/manager/components/sidebar/Search.tsx`, `src/manager/components/sidebar/TestingWidget.tsx`	Added `useLandmark` integration with landmark props and hidden heading labels; changed container elements from `div`/`nav` to semantic elements (`nav`, `header`); updated render logic to pass navigation visibility state
Sidebar Components - Skip Links & Labels `src/manager/components/sidebar/Heading.tsx`, `src/manager/components/sidebar/Heading.stories.tsx`, `src/manager/components/sidebar/Tree.tsx`, `src/manager/components/sidebar/Tree.stories.tsx`	Updated skip-link text from "Skip to canvas" to "Skip to content"; removed IconSymbols from Tree container; added IconSymbols import to story decorators
Sidebar Type Definitions `src/manager/components/sidebar/types.ts`	Replaced `isBrowsing` boolean flag in `SearchChildrenFn` render-props with `isNavVisible`, `isNavRendered`, and `isSearchResultRendered` properties
Sidebar Stories Cleanup `src/manager/components/sidebar/Sidebar.stories.tsx`	Removed IconSymbols component usage from decorators (3 lines)
Panel & MobileNavigation - Landmark Integration `src/manager/components/panel/Panel.tsx`, `src/manager/components/mobile/navigation/MobileNavigation.tsx`	Added `useLandmark` hook and landmark props integration; changed Panel Section to styled `aside` element; changed MobileNavigation to semantic `section` and `header` elements with landmark props
Preview Components - Landmark Integration `src/manager/components/preview/Preview.tsx`, `src/manager/components/preview/Toolbar.tsx`, `src/manager/components/preview/utils/components.ts`	Added `useLandmark` integration to Preview (main) and Toolbar (region); replaced static `aria-labelledby` with dynamic landmark props; refactored `PreviewContainer` from `styled.main` to `styled.div` and `FrameWrap` from `styled.section` to `styled.main`
Preview - Accessibility `src/manager/components/preview/FramesRenderer.tsx`	Removed `title` attribute from skip-to-sidebar anchor element
Card Component Enhancement `src/components/components/Card/Card.tsx`	Added `DOMAttributes` import from React; extended CardProps with `outlineProps?: DOMAttributes<HTMLDivElement>` and forwarded to CardOutline element
Layout Manager `src/manager/components/layout/Layout.tsx`	Added `useLandmarkIndicator()` invocation to install landmark navigation listener
Global Styling `src/theming/global.ts`	Added CSS rules for `[data-sb-landmark]` elements: position container, suppress focus outlines, render 2px color border on focus-visible with ::after pseudo-element using theme primary color

Sequence Diagram

sequenceDiagram
    participant User as User
    participant KeyListener as Keyboard Listener<br/>(useLandmarkIndicator)
    participant DOM as DOM Tree
    participant Animation as Web Animations API
    participant Theme as Theme System

    User->>KeyListener: Press F6
    KeyListener->>KeyListener: Capture keydown event
    KeyListener->>DOM: Search up from activeElement<br/>for [data-sb-landmark]
    DOM-->>KeyListener: Return parent landmark element
    alt Landmark Found
        KeyListener->>Animation: Cancel existing animation<br/>on landmark
        KeyListener->>Theme: Fetch theme.color.primary
        Theme-->>KeyListener: Return primary color
        KeyListener->>Animation: Start border highlight animation<br/>(2px solid, 1.5s duration)<br/>on ::after pseudo-element
        Animation->>DOM: Render border highlight<br/>on landmark
        Animation->>Animation: Wait 1.5s
        Animation->>DOM: Remove animation, cleanup state
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

[A11y]: Sidebar is not accessible #31267 — PR directly addresses sidebar accessibility enhancements by implementing ARIA landmark navigation and integrating accessibility hooks across sidebar components.

Possibly related PRs

Core: Onboarding guide #32874 — Modifies the same Card.tsx file; PR #32874 refactors Card to use forwardRef while this PR adds outlineProps prop support.
UI: Refocus search input after clearing it #33165 — Both PRs modify Search.tsx component; retrieved PR alters search clear/focus behavior while this PR changes landmark integration and render-props API shape.
UI: Make preview Toolbar and core Bar implement the toolbar role #32270 — Both PRs modify manager UI components with ARIA/accessibility enhancements; retrieved PR refactors toolbars/tabs to use AriaToolbar/AriaTabs while this PR adds landmark hooks and attributes.

✨ Finishing touches

📝 Generate docstrings

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In @code/core/src/manager/components/sidebar/Explorer.tsx:
- Line 27: Update the ref type for containerRef to match the rendered element:
change useRef<HTMLDivElement>(null) to useRef<HTMLElement | null>(null)
(referencing the containerRef variable in Explorer.tsx) so the ref can point to
the <nav> element and satisfy type safety wherever containerRef.current is used.

In @code/core/src/manager/components/sidebar/TestingWidget.tsx:
- Around line 257-264: The hooks cardRef = useRef<HTMLDivElement>(null) and
const { landmarkProps } = useLandmark(...) are being invoked after an early
return, violating the Rules of Hooks; move those hook calls (cardRef and
useLandmark invocation that provides landmarkProps) to execute unconditionally
before any early return or conditional logic in the TestingWidget component so
they run in the same order on every render, then use landmarkProps and cardRef
where needed after the early-return check.

🧹 Nitpick comments (5)

code/core/src/manager/components/sidebar/Heading.stories.tsx (1)

237-240: Text update correctly reflects the new skip link label.

The change from "Skip to canvas" to "Skip to content" aligns with the PR's accessibility improvements. The play function will correctly focus the updated skip link elements for Chromatic testing.

Optional: Consider renaming the story export for consistency.

The story is named SkipToCanvasLinkFocused (line 229) but now targets "Skip to content" text. For clarity, you might rename it to SkipToContentLinkFocused, though story names primarily serve as identifiers and the current name is still functional.
code/core/src/components/components/Card/Card.tsx (1)
100-107: Consider placing ref after outlineProps spread.

While DOMAttributes doesn't include ref (TypeScript prevents passing it), React convention places ref last to prevent accidental override and provide defense in depth.
♻️ Suggested prop order
-      <CardOutline animation={outlineAnimation} color={outlineColor} ref={ref} {...outlineProps}>
+      <CardOutline animation={outlineAnimation} color={outlineColor} {...outlineProps} ref={ref}>
code/core/src/manager/components/preview/FramesRenderer.tsx (1)

99-103: Removing the anchor title here is fine; consider dropping redundant tabIndex={0}.

Since the <a> has an href, it’s already tabbable; keeping tabIndex={0} is harmless but unnecessary.
code/core/src/manager/components/layout/useLandmarkIndicator.ts (1)
37-43: Consider browser compatibility for the Animation API.

The Web Animations API is well-supported in modern browsers but may fail in older environments. Consider adding a try-catch or feature detection for maximum robustness.
♻️ Optional defensive coding enhancement
-         const animation = landmarkElement.animate(
-           [{ border: `2px solid ${theme.color.primary}` }, { border: `2px solid transparent` }],
-           {
-             duration: 1500,
-             pseudoElement: '::after',
-           }
-         );
+         try {
+           const animation = landmarkElement.animate(
+             [{ border: `2px solid ${theme.color.primary}` }, { border: `2px solid transparent` }],
+             {
+               duration: 1500,
+               pseudoElement: '::after',
+             }
+           );
 
-         currentAnimationRef.current = animation;
+           currentAnimationRef.current = animation;
 
-         animation.onfinish = () => {
-           currentAnimationRef.current = null;
-         };
+           animation.onfinish = () => {
+             currentAnimationRef.current = null;
+           };
+         } catch (e) {
+           // Animation API not supported or failed, silently continue
+         }
code/core/src/manager/components/sidebar/Search.tsx (1)
316-322: Consider adding an explicit label to the search landmark.

The search landmark lacks an aria-label or aria-labelledby attribute. While the input field has an associated label, the landmark region itself would benefit from explicit labeling for better accessibility.
💡 Suggested enhancement
  const searchLandmarkRef = useRef<HTMLDivElement>(null);
  const { landmarkProps } = useLandmark(
    {
+     'aria-label': 'Search components',
      role: 'search',
    },
    searchLandmarkRef
  );

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9511a9c and 7feac0b.

⛔ Files ignored due to path filters (1)

yarn.lock is excluded by !**/yarn.lock, !**/*.lock

📒 Files selected for processing (22)

code/core/package.json
code/core/src/components/components/Card/Card.tsx
code/core/src/manager/components/layout/Layout.tsx
code/core/src/manager/components/layout/useLandmarkIndicator.ts
code/core/src/manager/components/mobile/navigation/MobileNavigation.tsx
code/core/src/manager/components/panel/Panel.tsx
code/core/src/manager/components/preview/FramesRenderer.tsx
code/core/src/manager/components/preview/Preview.tsx
code/core/src/manager/components/preview/Toolbar.tsx
code/core/src/manager/components/preview/utils/components.ts
code/core/src/manager/components/sidebar/Explorer.tsx
code/core/src/manager/components/sidebar/Heading.stories.tsx
code/core/src/manager/components/sidebar/Heading.tsx
code/core/src/manager/components/sidebar/Search.tsx
code/core/src/manager/components/sidebar/Sidebar.stories.tsx
code/core/src/manager/components/sidebar/Sidebar.tsx
code/core/src/manager/components/sidebar/TestingWidget.tsx
code/core/src/manager/components/sidebar/Tree.stories.tsx
code/core/src/manager/components/sidebar/Tree.tsx
code/core/src/manager/components/sidebar/types.ts
code/core/src/manager/hooks/useLandmark.ts
code/core/src/theming/global.ts

💤 Files with no reviewable changes (1)

code/core/src/manager/components/sidebar/Sidebar.stories.tsx

🧰 Additional context used

📓 Path-based instructions (5)

**/*.{js,jsx,ts,tsx,json,md,html,css,scss}