You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Address design feedback on the v4 onboarding flow:
Left align StepHeader text — title and subtitle now align left instead of center
Fix video flash when toggling DuckPlayer — use requestVideoFrameCallback to wait for the first frame before switching visibility, preventing a flash of the previous frame
Stagger bubble fade-in — top bubble (title/subtitle) fades in 100ms before the bottom bubble (body content), sequencing the load-in to guide the user's eye
Low Risk
Low risk UI/animation tweaks confined to onboarding v4 components; main risk is minor timing/visibility regressions in bubble transitions or video toggling across browsers.
Overview
Improves onboarding v4 polish by staggering bubble entry animations, left-aligning StepHeader text, and fixing Duck Player video flashing during toggles.
Bubble now accepts an optional fadeInDelay (via a CSS var) and SingleStep uses it to delay the bottom bubble when a top bubble is present. DuckPlayerContent now waits for the first rendered video frame (requestVideoFrameCallback) before swapping visibility, switches hidden videos to opacity: 0/no pointer events, and updates the Playwright test to assert opacity instead of not.toBeVisible().
Written by Cursor Bugbot for commit b59639f. This will update automatically on new commits. Configure here.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
special-pages/pages/onboarding/app/v4/components/DuckPlayerContent.js (L95-L101) — severity: error video.requestVideoFrameCallback(...) is called unguarded. If this API is unavailable in any supported runtime/WebView build, play() rejects before state transition logic runs, causing toggle/ended handlers to fail (and likely emit unhandled promise rejections). This maps to Platform-Specific Breakage / Missing APIs.
special-pages/pages/onboarding/app/v4/components/DuckPlayerContent.js (L95-L101, L133-L144) — severity: warning frameReady is awaited even when video.play() throws. In failure modes where playback never starts (decode/network/autoplay edge cases), the frame callback may never fire, leaving toggle()/end() suspended and UI state stuck. This is a Timing/Race compatibility risk because transition completion now depends on a callback that may not resolve.
special-pages/pages/onboarding/app/v4/components/StepHeader.module.css (L19, L32), special-pages/pages/onboarding/app/v4/components/Bubble.js (L147), special-pages/pages/onboarding/app/v4/components/Bubble.module.css (L40) — severity: info
These visual-only changes (left alignment and staggered fade delay via CSS custom property) are low compatibility risk and do not alter browser API contracts.
Security Assessment
No direct security findings in this diff.
No changes to captured globals, wrapper utilities, message bridge, origin validation, or transport code.
No new postMessage, network fetch/XHR, dynamic code execution, or config-trust boundary changes.
Changed logic is isolated to onboarding special-page UI behavior.
Risk Level
Medium Risk — UI-only scope, but the new requestVideoFrameCallback dependency introduces a real cross-runtime compatibility failure mode that can block step transitions.
Recommendations
Add capability/error hardening around first-frame wait: guard requestVideoFrameCallback and ensure play() always resolves/rejects deterministically (do not await an unresolvable frame promise).
Add an integration test path that simulates missing requestVideoFrameCallback and a video.play() rejection, and assert the toggle flow still reaches a settled state.
Catch/handle async handler failures in toggle()/end() call paths to prevent unhandled promise rejection noise and silent stuck states.
Sent by Cursor Automation: Web compat and sec
Base automatically changed from
randerson/onboarding-v4-polish to
mainMarch 12, 2026 09:18
Use requestVideoFrameCallback to delay visibility switch until the
incoming video's first frame is painted. Use a state ref so the
onEnded handler always reads fresh state. Hide inactive video with
opacity: 0 instead of visibility: hidden so the compositor keeps
decoding frames.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When transitioning between steps, the top bubble (title/subtitle)
now fades in 100ms before the bottom bubble (body content). This
sequences the load-in to guide the user's eye and improve
comprehension.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
special-pages/pages/onboarding/app/v4/components/DuckPlayerContent.js (approx. lines 95-101) — severity: error video.requestVideoFrameCallback(...) is called unconditionally. This API is not universally available across all embedded WebView/Safari baselines, so play() can reject via a TypeError path and break the toggle flow on unsupported runtimes. This is a direct platform-specific missing API regression risk.
special-pages/pages/onboarding/app/v4/components/DuckPlayerContent.js (approx. lines 95-101, 131-145) — severity: warning frameReady is awaited even when video.play() fails. In failure/no-decode paths, the frame callback may never fire, so toggle() / end() can stall and leave UI state stuck. This is a timing/race contract issue: state transitions depend on an await that may never resolve.
Security Assessment
No security findings in this diff. Changes are isolated to onboarding special-page UI/CSS and do not modify message bridge, origin validation, captured globals, wrapper utilities, or injected feature shims.
Risk Level
High Risk — while scope is limited to onboarding, this introduces a new platform API dependency in the video transition path without capability checks, which can cause user-visible functional breakage on unsupported WebViews.
Recommendations
Guard requestVideoFrameCallback with capability detection before use, and keep state transition logic functional when unavailable.
Do not await frameReady after failed play(); ensure toggle/end always reach a deterministic state transition.
Add integration coverage for: missing requestVideoFrameCallback and rejected video.play() to verify no stuck phase/reverse state.
The reason will be displayed to describe this comment to others. Learn more.
Play failure causes indefinite hang via unresolved promise
Medium Severity
The play function registers requestVideoFrameCallback before calling video.play(), then unconditionally awaits the resulting frameReady promise after the try-catch. If video.play() rejects (caught and logged), and the seek to time 0 doesn't produce a new frame (e.g., already at position 0), requestVideoFrameCallback never fires and frameReady never resolves. This causes play() to hang indefinitely, blocking both toggle and end from ever calling setState, effectively freezing the DuckPlayer UI.
The reason will be displayed to describe this comment to others. Learn more.
Stale closure in async toggle after await
Low Severity
The toggle function reads state from its closure at the top, then awaits play() in the 'settled' branch before calling setState with absolute values. Meanwhile, end correctly reads from stateRef.current to avoid stale closures. The inconsistency means toggle could overwrite state changes that occurred during the await, though the practical window is small since no videos are playing in the 'settled' phase.
The reason will be displayed to describe this comment to others. Learn more.
Auto-play timer mismatched with new fade-in delay
Low Severity
The auto-play timeout of 667ms was computed from the original 400ms fade-in delay + 267ms animation duration, as the comment states. This PR changes the bottom bubble's fadeInDelay to 500ms (for the DuckPlayer step, which always has a topBubble), so the fade-in now runs from 500–767ms. But the hardcoded 667ms timer was not updated, causing the video to start playing ~100ms before the bubble is fully opaque — the exact kind of visual flash this PR aims to fix.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
special-pages/pages/onboarding/app/v4/components/DuckPlayerContent.js (lines 95-101, 113, 133-134, 143-144) — warning: video.requestVideoFrameCallback(...) is called unconditionally and outside the try/catch around video.play(). On engines/WebViews where this API is unavailable, play() rejects via thrown TypeError, and the callers (useEffect fire-and-forget + async UI handlers) can end up with unhandled promise rejection and broken video-state transitions. This maps to Missing APIs and Timing/Race risk categories.
special-pages/pages/onboarding/app/v4/components/StepHeader.module.css and special-pages/pages/onboarding/app/v4/components/Bubble* — info: visual-only alignment/timing changes; no browser API override or prototype-surface regression found.
Security Assessment
No direct security vulnerabilities found in changed lines (no message bridge/origin validation changes, no new network exfil paths, no dynamic code execution, no privileged API boundary changes).
Residual note: the unguarded requestVideoFrameCallback path can be used as a reliability DoS vector for the step UI on unsupported runtimes (functional breakage, not data exposure).
Risk Level
Medium Risk — PR is mostly UI/CSS, but it introduces a runtime dependency on requestVideoFrameCallback without capability checks/fallback, which can regress onboarding flow behavior on unsupported or edge WebView versions.
Recommendations
Add feature detection and fallback in play(video):
Gate on typeof video.requestVideoFrameCallback === 'function'.
Fallback to loadeddata/canplay + requestAnimationFrame, or bounded timeout, before swapping visibility.
Ensure no unhandled async failures from event handlers/effects:
In useEffect, call void play(...).catch(...).
Keep onEnded/onClick handlers resilient if play() fails.
Add an integration test path that stubs missing requestVideoFrameCallback and verifies toggle/ended transitions still complete.
The reason will be displayed to describe this comment to others. Learn more.
requestVideoFrameCallback is invoked unguarded here and outside the try/catch around video.play(). On runtimes lacking this API, this throws and can break state transitions via unhandled async rejection. Please add a capability check + fallback frame-readiness strategy.
@cursor Please make the 100ms delay between top bubble fade in and bottom bubble fade in configurable via url param. Similar to what we do for ?bubbleWidth.
Added a URL parameter bubbleFadeInDelay to configure the delay offset between the top and bottom bubble fade-in animations, following the same pattern as ?bubbleWidth.
Changes made:
Added bubbleFadeInDelay URL parameter parsing at the top of SingleStep.js
Updated the bottom bubble's fadeInDelay calculation to use: 400ms (default top delay) + configurable offset (default 100ms)
How it works:
Default behavior: Top bubble fades in at 400ms, bottom bubble at 500ms (100ms offset)
With ?bubbleFadeInDelay=200: Bottom bubble fades in at 600ms (200ms offset)
With ?bubbleFadeInDelay=0: Both bubbles fade in simultaneously at 400ms
Invalid values fall back to the default 100ms offset
The implementation matches the bubbleWidth override pattern and handles invalid input by falling back to the default.
Add ?bubbleFadeInDelay URL parameter to configure the delay offset between
top and bottom bubble fade-in animations. Default remains 100ms offset
(top: 400ms, bottom: 500ms). Follows the same pattern as ?bubbleWidth.
Co-authored-by: Robert Anderson <robert@noisysocks.com>
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
special-pages/pages/onboarding/app/v4/components/DuckPlayerContent.js (play() around requestVideoFrameCallback and await frameReady) — warning
Unconditional video.requestVideoFrameCallback(...) use is a cross-platform compatibility risk (Missing APIs category). If this API is absent in any supported WebView/Safari baseline, it throws synchronously before the try/catch, causing toggle/end async handlers to reject.
special-pages/pages/onboarding/app/v4/components/DuckPlayerContent.js (play() error path) — warning frameReady is awaited even when video.play() rejects. In rejection paths, no frame callback is guaranteed, so the promise can remain pending and stall state transitions (Timing and Race Conditions + return contract expectations for async handlers).
special-pages/pages/onboarding/app/v4/components/Bubble.js + Bubble.module.css + SingleStep.js (fade delay plumbing) — info
The staggered fade change is scoped to CSS custom-property timing and preserves component contracts. I do not see a site-compat regression pattern here.
special-pages/pages/onboarding/app/v4/components/StepHeader.module.css (text alignment) — info
Pure presentation change (text-align: left), no API/runtime compatibility concerns.
Security Assessment
No direct security findings in this diff.
The PR does not touch injected API shims, captured globals, messaging bridge, origin checks, or any network/data-exfiltration path. Changes are localized to onboarding special-page UI behavior.
Risk Level
Medium Risk — primarily due to the new first-frame synchronization path introducing potential runtime hangs/handler rejection on unsupported API or failed playback paths.
Recommendations
Gate requestVideoFrameCallback with feature detection and provide a bounded fallback path (for example, resolve on loadeddata/canplay or immediate resolve after successful play()).
Do not await frame readiness when play() fails; short-circuit to existing state transition logic so UI cannot deadlock.
Add an automated test case that stubs missing requestVideoFrameCallback and a play() rejection to verify toggle/end still settle correctly.
The "Hi there" and "Protections activated!" steps had centered
headings and body copy while the rest of the steps were left-aligned.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Web Compatibility Assessment
special-pages/pages/onboarding/app/v4/components/DuckPlayerContent.js (lines 95-101) — error requestVideoFrameCallback is called unguarded (video.requestVideoFrameCallback(...)). On engines where this API is absent, this throws synchronously before video.play() runs, causing the toggle/end async handlers to reject and leaving the step stuck. This maps to the Missing APIs / platform-specific breakage risk.
special-pages/pages/onboarding/app/v4/components/DuckPlayerContent.js (lines 95-101) — warning await frameReady runs even when video.play() rejects. If playback fails (decode/policy/media error), no frame callback is guaranteed, so the promise can remain pending and block state transitions. This creates a latent UI deadlock in the toggle flow.
Security Assessment
No security vulnerabilities found in this diff.
The changes are isolated to onboarding special-page UI/animation code and tests; no message bridge, transport/origin validation, captured-globals, network, or injected API-shimming paths were modified.
Risk Level
Medium Risk — scoped UI changes, but the new video-frame sync path introduces a real compatibility failure mode on unsupported or failed playback paths.
Recommendations
Guard requestVideoFrameCallback with feature detection and fall back to immediate resolve or a loadeddata/canplay-based wait.
Do not await frame readiness after failed play(); short-circuit to preserve state progress. A pragmatic alternative is Promise.race([frameReady, timeout]) with a short timeout.
Add an integration/unit test path that stubs HTMLVideoElement.prototype.requestVideoFrameCallback as undefined and asserts toggle still progresses without hanging.
The comparison table and actions now fade in 100ms after the title,
configurable via the bubbleFadeInDelay URL param.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The reason will be displayed to describe this comment to others. Learn more.
Web Compatibility Assessment
special-pages/pages/onboarding/app/v4/components/DuckPlayerContent.js (95-101) — error video.requestVideoFrameCallback(...) is called unconditionally. On platforms/engines where this API is missing, this throws and breaks toggle/end transitions for the Duck Player step. This is a Missing APIs / platform-specific breakage risk.
special-pages/pages/onboarding/app/v4/components/DuckPlayerContent.js (96-101, 133, 143) — error play() catches video.play() rejection but still awaits frameReady. If playback fails (or first frame is never produced), requestVideoFrameCallback may never resolve, causing state transitions to hang in async handlers (toggle/end). This is a timing/race/hang compatibility issue.
Security Assessment
No direct security vulnerabilities identified in this diff.
No changes to message bridge, origin validation, captured globals, wrapper utilities, or privileged transports.
New URL param usage (bubbleFadeInDelay) is parsed numerically before writing to CSS custom properties, so no direct script/CSS injection vector is introduced by these lines.
Risk Level
Medium Risk — UI-only scope, but the new video-frame synchronization path introduces cross-platform API availability and unresolved-promise failure modes that can break onboarding progression.
Recommendations
Guard requestVideoFrameCallback with feature detection and provide a fallback (loadeddata/canplay/requestAnimationFrame) before visibility swap.
Do not await frame readiness after video.play() rejection; fail fast to previous behavior (switch state without frame wait) or use a bounded timeout.
Add tests for degraded environments: missing requestVideoFrameCallback and rejected video.play() to ensure toggle/end never deadlock.
Sent by Cursor Automation: Web compat and sec
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
semver-patchBug fix / internal — no release needed
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.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
noisysocks
force-pushed
the
randerson/onboarding-v4-design-feedback-2
branch
from
Asana Task/Github Issue:
https://app.asana.com/1/137249556945/project/1209121419454298/task/1213627872900762?focus=true
https://app.asana.com/1/137249556945/project/1142021229838617/task/1213508111481481?focus=true
Description
Address design feedback on the v4 onboarding flow:
requestVideoFrameCallbackto wait for the first frame before switching visibility, preventing a flash of the previous frameTesting Steps
https://rawcdn.githack.com/duckduckgo/content-scope-scripts/1eecb0e5b9e282f4932c80ba63a7f79968f05830/build/integration/pages/onboarding/index.html?order=v4
Checklist
🤖 Generated with Claude Code
Note
Low Risk
Low risk UI/animation tweaks confined to onboarding v4 components; main risk is minor timing/visibility regressions in bubble transitions or video toggling across browsers.
Overview
Improves onboarding v4 polish by staggering bubble entry animations, left-aligning StepHeader text, and fixing Duck Player video flashing during toggles.
Bubblenow accepts an optionalfadeInDelay(via a CSS var) andSingleStepuses it to delay the bottom bubble when a top bubble is present.DuckPlayerContentnow waits for the first rendered video frame (requestVideoFrameCallback) before swapping visibility, switches hidden videos toopacity: 0/no pointer events, and updates the Playwright test to assert opacity instead ofnot.toBeVisible().Written by Cursor Bugbot for commit b59639f. This will update automatically on new commits. Configure here.