Skip to content

fix(timeline): keep clips visible during analysis#615

Merged
meiiie merged 1 commit into
mainfrom
fix/fresh-recording-auto-zoom-recovery
May 28, 2026
Merged

fix(timeline): keep clips visible during analysis#615
meiiie merged 1 commit into
mainfrom
fix/fresh-recording-auto-zoom-recovery

Conversation

@meiiie

@meiiie meiiie commented May 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Keep timeline clip blocks visible while background audio/cursor analysis is running.
  • Leave the loading state on the playhead/analysis indicator instead of replacing clips with an "Analyzing..." skeleton.

Why

Users reported that enabling Auto-apply fresh recording zooms could leave the timeline looking stuck on "Analyzing" with no usable clip. The clip itself is already available; background analysis should not hide or disable it.

Verification

  • npx biome check --formatter-enabled=false src/components/video-editor/timeline/components/viewport/TimelineCanvas.tsx
  • npx tsc --noEmit
  • git diff origin/main...HEAD --check

Summary by CodeRabbit

  • Refactor
    • Simplified the timeline clip interface by removing loading state indicators that appeared during clip analysis, streamlining the component structure.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

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: 7ce68355-abf6-4c12-8c16-ef12f6db7cf3

📥 Commits

Reviewing files that changed from the base of the PR and between f5e9603 and 0a0c938.

📒 Files selected for processing (1)
  • src/components/video-editor/timeline/components/viewport/TimelineCanvas.tsx
💤 Files with no reviewable changes (1)
  • src/components/video-editor/timeline/components/viewport/TimelineCanvas.tsx

📝 Walkthrough

Walkthrough

The TimelineCanvas component is simplified by removing the isLoading prop forwarding chain. The TimelineCanvasRowsProps interface no longer includes an optional isLoading field, the TimelineCanvasRows function signature is updated to remove the corresponding destructured default, clip Item components no longer receive the prop, and the top-level TimelineCanvasRows invocation removes the prop pass-through.

Changes

Remove isLoading Prop Drilling from Timeline Canvas

Layer / File(s) Summary
Remove isLoading prop drilling through component tree
src/components/video-editor/timeline/components/viewport/TimelineCanvas.tsx
TimelineCanvasRowsProps interface removes the optional isLoading field, TimelineCanvasRows function signature no longer destructures isLoading, clip Item components stop receiving isLoading and loadingLabel="Analyzing...", and the TimelineCanvasRows JSX invocation removes the isLoading prop pass-through.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • webadderallorg/Recordly#498: Modifies TimelineCanvasRowsProps interface and prop drilling in the same component, adding handler logic alongside prop interface changes.

Suggested labels

Checked

Poem

🐰 A prop that was passed through the timeline's long chain,
Now gently removed, like rain on a plain,
The component simplifies, lighter it flows,
One less thread tangled as the logic grows.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and directly describes the main change: removing the isLoading flag to keep clips visible during analysis, which is the core purpose of the changeset.
Description check ✅ Passed The description includes Summary, Why, and Verification sections addressing the change's purpose and testing. However, it lacks the formal template structure (Type of Change checkbox, Related Issue(s), Screenshots, Testing Guide, Checklist sections).
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 fix/fresh-recording-auto-zoom-recovery

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

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

@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Actionable comments posted: 0

@meiiie meiiie merged commit 4a1e50e into main May 28, 2026
3 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.

1 participant