0.72.2 - Fix DashboardLayout sidebar links swallowing next/link navigation#76
Merged
Conversation
…ation DashboardSidebarItemRenderer passed the consumer <Link> an onClick that called e.preventDefault(). Spec-compliant routers (next/link, React Router, TanStack Router, ...) treat a click whose default was prevented as "the handler is navigating itself, stand down", so every sidebar item became a dead link on every viewport. The only real work the handler did was collapse the sidebar. Fix: drop the preventDefault() call so the Link navigates from its href. Modifier-clicks (Cmd/Ctrl+click to open a new tab) now work too, since the router handles those itself once we stop cancelling the default. The eager setSidebarOpen(false) is now gated behind 'if (mobile)', matching useCloseDashboardSidebarOnRouteChange (which also only auto-closes on mobile). Behavior change for desktop: the persistent sidebar no longer collapses on every item click. The mobile overlay Sheet still closes immediately on tap, and also via the route-change hook once navigation lands. Added a 'NextLinkStyleNavigation' Storybook story that wires up a next/link-style Link adapter (runs the consumer onClick, then navigates only if the default was not prevented) with an on-screen navigation log and a play() assertion that the forwarded click is not defaultPrevented and navigation proceeds. Downstream note: this fix ships in @schemavaults/ui 0.72.2. Consumers that added a defensive Proxy wrapper to neutralize the library's preventDefault() (e.g. botree-workflow-discovery) can revert to a plain pass-through Link adapter once they bump to >=0.72.2. https://claude.ai/code/session_01BXzLQntYxxQsiVbRcM6jRK
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The NextLinkStyleNavigation play() used getByRole('link'), which failed
with "Unable to find role='link'" for two reasons:
1. Each sidebar item link is rendered inside a Radix Tooltip trigger
<button>. A button's descendants are presentational, so the nested
<a> never appears in the accessibility tree — role-based queries
cannot see it.
2. On a narrow viewport (e.g. the autodocs Docs preview) the sidebar
renders as a closed Sheet, so no sidebar links are mounted at all,
and when opened its content is portaled to document.body (outside the
story canvas).
Fix: locate the item link by its href via document.querySelector (which
ignores ARIA roles and spans portals), and open the sidebar once via the
header trigger when the link isn't mounted yet (mobile Sheet). Also tag
the story '!autodocs' so this responsive regression test runs only in
its own Canvas rather than the narrow Docs preview. The assertions
(consumer onClick does not preventDefault; navigation is logged) are
unchanged.
https://claude.ai/code/session_01BXzLQntYxxQsiVbRcM6jRK
Wire up @storybook/test-runner (Playwright) so every story's play() interaction test runs in CI, alongside a render smoke test for all other stories. This makes the NextLinkStyleNavigation regression test (and any future interaction tests) executable and enforced. - devDeps: @storybook/test-runner@0.24.4 (peer-supports Storybook 10), plus concurrently/http-server/wait-on to serve the static build and run the runner against it. (bun add normalized devDependencies order.) - scripts: 'test-storybook' (against a running dev server) and 'test-storybook:ci' (serve storybook-static on :6006, wait, then run). - CI: new 'Storybook Tests' job in pull_request_checks.yml that installs the Playwright Chromium browser (with system deps), builds Storybook, and runs test-storybook:ci. Gated on Lint + Typecheck like Build. - Documented the commands in CLAUDE.md. Note: the browser-based run could not be executed in the dev sandbox (its network policy blocks the Playwright browser CDN); it executes on GitHub Actions runners, which this branch push triggers. https://claude.ai/code/session_01BXzLQntYxxQsiVbRcM6jRK
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
2 participants
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.
DashboardSidebarItemRenderer passed the consumer an onClick that
called e.preventDefault(). Spec-compliant routers (next/link, React
Router, TanStack Router, ...) treat a click whose default was prevented
as "the handler is navigating itself, stand down", so every sidebar
item became a dead link on every viewport. The only real work the
handler did was collapse the sidebar.
Fix: drop the preventDefault() call so the Link navigates from its href.
Modifier-clicks (Cmd/Ctrl+click to open a new tab) now work too, since
the router handles those itself once we stop cancelling the default.
The eager setSidebarOpen(false) is now gated behind 'if (mobile)',
matching useCloseDashboardSidebarOnRouteChange (which also only
auto-closes on mobile). Behavior change for desktop: the persistent
sidebar no longer collapses on every item click. The mobile overlay
Sheet still closes immediately on tap, and also via the route-change
hook once navigation lands.
Added a 'NextLinkStyleNavigation' Storybook story that wires up a
next/link-style Link adapter (runs the consumer onClick, then navigates
only if the default was not prevented) with an on-screen navigation log
and a play() assertion that the forwarded click is not defaultPrevented
and navigation proceeds.
Downstream note: this fix ships in @schemavaults/ui 0.72.2. Consumers
that added a defensive Proxy wrapper to neutralize the library's
preventDefault() (e.g. botree-workflow-discovery) can revert to a plain
pass-through Link adapter once they bump to >=0.72.2.
https://claude.ai/code/session_01BXzLQntYxxQsiVbRcM6jRK