Keep click effects visible when cursor is hidden

[webadderall]

Summary

• make cursor click effects render even when the cursor sprite is hidden
• preserve cursor-smoothed hook behavior while cursor is hidden
• keep native static-layout fallback logic aligned for active click effects
• add regression coverage for hidden-cursor click effects

Validation

• npm exec -- tsc --noEmit --pretty false
• npx vitest run src/lib/exporter/modernVideoExporter.nativeStaticLayout.test.ts

Summary by CodeRabbit

• Bug Fixes
  • Improved cursor rendering behavior and visibility state management in video playback
  • Enhanced cursor click effect compatibility with export functionality when cursor rendering is disabled
  • Refined cursor overlay state reset and timing synchronization

[webadderall]

@coderabbitai review

@copilot review

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a4697d00-d427-477f-a662-38241112805e

📥 Commits

Reviewing files that changed from the base of the PR and between 8641491 and b5a01a3.

📒 Files selected for processing (3)

• src/components/video-editor/videoPlayback/cursorRenderer.ts
• src/lib/exporter/modernVideoExporter.nativeStaticLayout.test.ts
• src/lib/exporter/modernVideoExporter.ts

💤 Files with no reviewable changes (1)

• src/lib/exporter/modernVideoExporter.ts

---

📝 Walkthrough

Walkthrough

This PR decouples cursor sprite rendering from click-effect rendering by introducing explicit visibility state in PixiCursorOverlay. The cursor renderer now computes sprite and click-effect visibility independently, allowing click effects to draw even when the cursor sprite is hidden. The exporter's native static-layout skip logic is updated to detect cursor click effects regardless of the showCursor setting, with a new test validating this behavior.

Changes

Cursor Visibility and Click-Effect Decoupling

Layer / File(s)	Summary
Cursor visibility state foundation src/components/video-editor/videoPlayback/cursorRenderer.ts	PixiCursorOverlay adds a cursorVisible private field to track sprite visibility independently of Pixi container visibility. The field is initialized, gated in getSmoothedCursorSnapshot(), and reset in the reset() method.
Cursor sprite and click-effect visibility separation src/components/video-editor/videoPlayback/cursorRenderer.ts	update() method is refactored to compute shouldShowCursorSprite and shouldDrawClickEffect independently. Early-exit paths are tightened with explicit state cleanup; sprite rendering branches enforce the new visibility gate; and lastRenderedPoint is set to null when sprite is not shown, preventing stale positions from affecting motion-blur calculations.
Exporter skip reason and test src/lib/exporter/modernVideoExporter.ts, src/lib/exporter/modernVideoExporter.nativeStaticLayout.test.ts	getNativeStaticLayoutSkipReasons() removes the showCursor === true guard from the cursor click-effect check, allowing the unsupported-cursor-click-effect skip reason to trigger based solely on cursor telemetry and click-effect presence. A new test case validates the updated skip logic when cursor rendering is disabled.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• webadderallorg/Recordly#630: Both PRs touch the cursor click-effects pipeline—cursorRenderer.ts visibility/state gating and click-effect rendering, plus modernVideoExporter eligibility/skip logic for unsupported-cursor-click-effect.
• webadderallorg/Recordly#607: Both PRs touch the cursor click-effect pipeline—Add click effects and fix auto zoom and FPS bitrate behavior #607 adds cursor click-effect configuration/rendering, and the main PR further refines PixiCursorOverlay visibility/click-effect drawing and exporter native static-layout skip logic.
• webadderallorg/Recordly#589: Both PRs change cursor-related state management: the retrieved PR resets cursor telemetry on source reload, which affects the telemetry-driven cursor visibility/reset logic and subsequent cursor-click-effect skip behavior in the main PR.

Suggested labels

Checked

Poem

🐰 A cursor takes two paths now,
One sprite, one spark of click,
No longer locked together—
They dance when needed, swift and quick!
✨ Freedom for effects to flow...

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	The description is structured but does not follow the provided template; it lacks required sections like Motivation, Type of Change, and Related Issue(s), though it provides a clear summary and validation steps.	Restructure the description to follow the template: add Motivation section explaining why click effects should render when cursor is hidden, specify Type of Change, and link related issue #632.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: enabling click effect visibility when the cursor is hidden, matching the core objective of the PR.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/click-effects-auto-zoom-fps-fixes

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get the PR diff
gh pr view 632 --repo webadderallorg/Recordly --json title,body,files,additions,deletions

# Get the PR diff
gh pr diff 632 --repo webadderallorg/Recordly 2>&1 | head -800

Length of output: 10268

---

---

@webadderall: Here's my review of PR #632.

Overall Assessment

The core logic is sound — decoupling container.visible from cursor sprite visibility is the right approach, and the modernVideoExporter.ts one-liner fix is correct and well-tested. A few things are worth addressing before merge.

---

🔴 Potential Bug — clickEffectPx/clickEffectPy fallback uses raw position instead of smoothed

File: src/components/video-editor/videoPlayback/cursorRenderer.ts, ~Line 1355

The old fallback used the smoothed spring position:

// Before
: px;  // smoothed via this.state.x

The new fallback uses the raw projected position:

// After
: viewport.x + projectedTarget.cx * viewport.width;

When the cursor is visible but projectedClickSample is not available, the click ring now anchors to the raw (unsmoothed) cursor position instead of the smoothed one. This introduces a visual inconsistency — the cursor sprite moves smoothly but the click ring can jump to the raw position during that frame. Consider using px (computed from this.state.x) for the fallback when shouldShowCursorSprite is true:

const clickEffectPx =
    projectedClickSample && projectedClickSample.visible
        ? viewport.x + projectedClickSample.cx * viewport.width
        : shouldShowCursorSprite
            ? px  // use smoothed position when cursor is visible
            : viewport.x + projectedTarget.cx * viewport.width;

---

🟡 Potential Visual Glitch — spring state not updated while cursor is hidden

When shouldShowCursorSprite is false, this.state.update() / this.state.snapTo() are never called, but this.lastRenderedTimeMs = timeMs is still set every frame. When the cursor becomes visible again:

• hasTimeDiscontinuity is false (timestamps were tracked continuously)
• freeze is typically false
• → shouldFreezeCursorMotion = false → this.state.update() is called from the spring's stale internal position

This can cause the cursor to visibly drift from its stale spring position to the actual position over several frames. If intentional (smooth re-entry), it's fine — but it's worth a comment. If not, you may want to call this.state.snapTo() on the first frame the cursor becomes visible again (e.g., by checking !this.cursorVisible before the update):

if (shouldShowCursorSprite) {
    // ...
    const justBecameVisible = !this.cursorVisible; // was false last frame
    shouldFreezeCursorMotion = freeze || hasTimeDiscontinuity || justBecameVisible;
    // ...
}

---

🟡 Code Duplication — sprite-hiding block appears 3+ times

The following block is copy-pasted at least three times (Lines ~1323, ~1375, ~1407):

this.customCursorShadowSprite.visible = false;
this.customCursorSprite.visible = false;
for (const shadowSprite of Object.values(this.cursorShadowSprites)) {
    shadowSprite.visible = false;
}
for (const sprite of Object.values(this.cursorSprites)) {
    sprite.visible = false;
}

Consider extracting this into a private helper:

private hideCursorSprites(): void {
    this.customCursorShadowSprite.visible = false;
    this.customCursorSprite.visible = false;
    for (const shadowSprite of Object.values(this.cursorShadowSprites)) {
        shadowSprite.visible = false;
    }
    for (const sprite of Object.values(this.cursorSprites)) {
        sprite.visible = false;
    }
}

---

✅ Positives

• cursorVisible field cleanly tracks sprite-level visibility separately from container visibility — correct approach.
• getSmoothedPosition guard updated to !this.container.visible || !this.cursorVisible — correct; prevents returning a position when only the click ring is rendering.
• lastRenderedPoint = shouldShowCursorSprite ? { px, py } : null — correctly prevents stale position data from bleeding into the next visible frame's discontinuity check.
• reset() now initialises cursorVisible = false — consistent.
• The modernVideoExporter.ts fix is minimal and correct; the regression test in nativeStaticLayout.test.ts covers the exact scenario.