Skip to content

fix(timeline): copy a whole pill, not the fragment clicked - #1245

Merged
EtienneLescot merged 1 commit into
mainfrom
fix/copy-whole-pills
Oct 16, 2026
Merged

EtienneLescot merged 1 commit into
mainfrom
fix/copy-whole-pills

Conversation

@EtienneLescot

@EtienneLescot EtienneLescot commented Oct 15, 2026 •

Copy link
Copy Markdown
Member

Summary

Copying a zoom, annotation, speed or Full Camera region that a clip boundary cuts took only the fragment clicked, so the paste came out shorter than the pill on screen.

  • wholePillOf (timelineMap.ts) gives the pill a fragment belongs to as one region over its whole span, through the same coalesceRegionsForRuler the lanes draw with.
  • handleCopyRegion copies that. Trims and audio already collapsed their pills.

The shader effect copy from #1210, which had its own version of this fix, now uses wholePillOf too.

Related issue

None.

Type of change

  • Bug fix

Release impact

  • Patch

Desktop impact

  • Not platform-specific

Testing

  • timelineMap.test.ts: wholePillOf returns a zoom cut at a clip boundary over its whole span, a lone region as is, and null for an unknown id.
  • npx tsc --noEmit, npx tsc -p tsconfig.test.json --noEmit, Biome: clean.
  • npm run test:changed: 2377 passed.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Copying a shader effect or other time-based region now copies the full merged region shown on the timeline, rather than only the selected fragment. Shader-effect copies include the effect definition for the complete region. Regions that are not part of a merged span continue to copy as their own region.

@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 7f71efed-5ba4-422c-80cb-e8863420fe09
📥 Commits

Reviewing files that changed from the base of the PR and between 639f0bb and bc4e7e3.

📒 Files selected for processing: 1
  • src/components/ai-edition/NewEditorShell.tsx

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

📝 Walkthrough

The copy handler now resolves a selected region to its complete merged pill span. For shader effects, it also resolves the effect definition from the complete pill. Tests cover merged spans, regions with different properties, and unknown IDs.

Changes: Copy complete timeline pills

Layer / File(s) Summary
Resolve and copy the full pill
src/lib/ai-edition/timeline/timelineMap.ts, src/components/ai-edition/NewEditorShell.tsx, src/lib/ai-edition/timeline/timelineMap.test.ts
wholePillOf returns the merged span for a region ID, or null if no pill contains the ID. The copy handler uses this helper for shader effects and other region kinds. Shader-effect copies resolve the effect definition from the complete pill. Tests cover merged spans, regions with different properties, and unknown IDs.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: my-denia

Merge Risk: ⚪ Minimal · up to bc4e7

The complete-pill copy behavior is supported by the implementation and focused tests, with no merge-blocking risk identified.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
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.
Title check Passed The title clearly and concisely describes the primary fix: copying the complete timeline pill instead of only the clicked fragment.
Description check Passed The description covers the change, issue status, change type, release impact, desktop impact, and testing results. The screenshots section is omitted, but the change is not described as a visual UI ch…
  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

A zoom, annotation, speed or Full Camera region cut by a clip boundary is several fragments drawn as one pill. Copying it took the fragment clicked, so the paste came out shorter than the region on screen. wholePillOf gives the pill as one region over its whole span; the shader effect copy, which had its own version of this, uses it too.
@EtienneLescot
EtienneLescot merged commit bb75494 into main Oct 16, 2026
20 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