Skip to content

fix(router): exclude the document root from typed view transitions - #9375

Merged
Varixo merged 2 commits into
QwikDev:mainfrom
Nelmad:bugfix/view-transition-root
Nov 6, 2026
Merged

Varixo merged 2 commits into
QwikDev:mainfrom
Nelmad:bugfix/view-transition-root

Conversation

@Nelmad

@Nelmad Nelmad commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

What is it?

Bug

Description

Opted-in SPA navigation in Chromium cross-faded the whole document because the nested :root selector matched a descendant of html. Apply view-transition-name: none directly to html:active-view-transition-type(qwik-navigation), preserving the unsupported-selector fallback and the existing CSS layer.

Adds a Playwright regression that samples the root's computed viewTransitionName at transition.ready, plus a patch changeset for @qwik.dev/router.

Fixes #9373.

Verification

The review follow-up in 9706e9c removes the redundant nested & block:

  • pnpm build.core.dev and focused Prettier check pass.
  • esbuild produces identical flattened CSS before and after this simplification for Chrome 100, Firefox 100 and Safari 15 targets.
  • The fresh Chromium run could not launch the browser in the local macOS sandbox (MachPortRendezvousServer: Permission denied); none of its three tests reached their assertions.

Before this CSS-only simplification, commit def9050 passed:

  • Regression before the original fix: expected none, received root; after the fix: all 3 view-transition tests passed.
  • pnpm build.core, type checking, ESLint, Prettier, Syncpack and changeset validation.
  • Full unit suite: 4558 passed, 19 skipped, 18 todo (--maxWorkers=2).
  • Full Chromium e2e suite: 671 passed, 4 skipped (--retries=0).

Initial concurrent broad runs had intermittent failures in existing core tests; separate reruns passed. Browsers without typed-selector support were not run separately; their fallback block is unchanged.

Hosted CI still needs maintainer approval; the earlier Qwik CI and Ruler guidance runs were action_required.

Checklist

  • Read the applicable contributor and repository instructions.
  • Performed a self-review of the code.
  • Added and validated a patch changeset.
  • Added a regression test and demonstrated failure before the original fix.

Summary by CodeRabbit

  • Bug Fixes
    • Prevented unintended full-page animations during typed view transitions.

@Nelmad
Nelmad requested a review from a team as a code owner November 4, 2026 14:15
@changeset-bot

changeset-bot Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 9706e9c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
@qwik.dev/router Patch
eslint-plugin-qwik Patch
@qwik.dev/mcp Patch
create-qwik Patch
@qwik.dev/devtools Patch
@qwik.dev/react Patch
@qwik.dev/utils Patch
@qwik.dev/core Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The active view-transition CSS rule now sets view-transition-name: none on the matching html element. An end-to-end test checks the document root’s computed transition name after SPA navigation.

Changes: View-transition root behavior

Layer / File(s) Summary
Apply and verify the root transition rule
packages/qwik-router/src/runtime/src/qwik-view-transition.css, e2e/qwik-e2e/tests/qwikrouter/view-transition.e2e.ts, .changeset/quiet-roots-transition.md
The active view-transition rule applies view-transition-name: none to the matching html element. The end-to-end test checks the document root’s computed transition name after navigation when the browser supports View Transitions. A patch changeset describes the fix.

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: wmertens, maiieul

Merge Risk: Minimal · up to 9706e

The root-rule change and regression test align with the stated fix. No actionable merge risk remains beyond normal checks.

Architecture risk: Low · up to def90

The change affects 2 systems.

Changed systems: e2e, packages/qwik-router

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — e2e (service) was modified; 1 changed file maps to changed impact.
  • observed — packages/qwik-router (library) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in e2e/qwik-e2e/tests/qwikrouter/view-transition.e2e.ts: Adds a test that navigates with View Transitions enabled, skips when the browser lacks document.startViewTransition, waits for router roots to initialize, and records the document root’s computed transition name after the transition is ready. After SPA navigation, it verifies the attribute was recorded and equals none.
  • observed — Modified behavior in packages/qwik-router/src/runtime/src/qwik-view-transition.css: Within the qwik-navigation active view-transition rule, :root is replaced by the nesting selector &; the view-transition-name: none declaration remains.
  • observed — Modified behavior in .changeset/quiet-roots-transition.md: Adds a patch changeset for @qwik.dev/router with a description of the typed view transition animation fix.
Pre-merge checks: Passed 5

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main router fix: excluding the document root from typed view transitions.
Linked Issues check Passed [#9373] requires the navigation rule to set view-transition-name: none on html itself, not on a nested :root descendant selector. At the reviewed head, qwik-view-transition.css sets `view-tran…
Out of Scope Changes check Passed The whole-PR changes against the merge base touch three files: packages/qwik-router/src/runtime/src/qwik-view-transition.css (the selector fix), `e2e/qwik-e2e/tests/qwikrouter/view-transition.e2e.ts…
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Full details: Linked Issues check

Explanation

[#9373] requires the navigation rule to set view-transition-name: none on html itself, not on a nested :root descendant selector. At the reviewed head, qwik-view-transition.css sets view-transition-name: none directly inside html:active-view-transition-type(qwik-navigation), inside the @supports selector(...) block and the qwik layer. The redundant nested :root block was removed in 9706e9c. The @supports not selector(...) fallback still sets :root { view-transition-name: none; }, which the issue does not require to change. The Playwright test excludes the document root during SPA view transitions samples getComputedStyle(document.documentElement).viewTransitionName at transition.ready after an SPA navigation and asserts none, which covers the reported behavior. The changeset adds a patch entry for @qwik.dev/router. Limit: the Chromium e2e run at the reviewed head could not launch the browser locally. The author reports passing results at def9050 and identical flattened CSS for the simplification, but this assessment did not execute the tests.

Full details: Out of Scope Changes check

Explanation

The whole-PR changes against the merge base touch three files: packages/qwik-router/src/runtime/src/qwik-view-transition.css (the selector fix), e2e/qwik-e2e/tests/qwikrouter/view-transition.e2e.ts (a regression test for the reported root computed style), and .changeset/quiet-roots-transition.md (a patch changeset for @qwik.dev/router). Each change supports [#9373]. The CSS diff is limited to the active-transition rule. The test checks the same root behavior as the issue. The changeset describes the same fix. No unrelated changes were found in the reviewed summary or in the inspected diffs.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files. (1 skipped: 1 unsupported.)

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b4327e34-d63b-408e-afee-5e5c2f9259e0
Commits

Reviewing files that changed from the base of the PR and between def9050 and 9706e9c.

Files selected for processing: 1

  • packages/qwik-router/src/runtime/src/qwik-view-transition.css
Files skipped from review as they are similar to previous changes: 1

  • packages/qwik-router/src/runtime/src/qwik-view-transition.css

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@Varixo Varixo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hello, thanks for this PR! Added small question

@Varixo Varixo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks

@pkg-pr-new

pkg-pr-new Bot commented Nov 5, 2026

Copy link
Copy Markdown

Open in StackBlitz

@qwik.dev/core

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/core@9375

@qwik.dev/router

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/router@9375

@qwik.dev/utils

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/utils@9375

eslint-plugin-qwik

npm i https://pkg.pr.new/QwikDev/qwik/eslint-plugin-qwik@9375

create-qwik

npm i https://pkg.pr.new/QwikDev/qwik/create-qwik@9375

@qwik.dev/optimizer

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/optimizer@9375

@qwik.dev/devtools

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/devtools@9375

@qwik.dev/mcp

npm i https://pkg.pr.new/QwikDev/qwik/@qwik.dev/mcp@9375

commit: 9706e9c

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

⚡ Docs preview for 9706e9c

@Varixo
Varixo merged commit 059745f into QwikDev:main Nov 6, 2026
55 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.

[🐞] View transition CSS doesn't un-capture :root in Chrome

2 participants