Skip to content

feat(studio): add TimelineClip and thumbnail components#62

Closed
miguel-heygen wants to merge 2 commits intomainfrom
studio/2-timeline-components
Closed

feat(studio): add TimelineClip and thumbnail components#62
miguel-heygen wants to merge 2 commits intomainfrom
studio/2-timeline-components

Conversation

@miguel-heygen
Copy link
Copy Markdown
Collaborator

@miguel-heygen miguel-heygen commented Mar 26, 2026 •
edited
Loading

Summary

  • TimelineClip: Extracted clip rendering component with drag, resize, selection, and context menu
  • CompositionThumbnail: Film-strip of server-rendered JPEG thumbnails for HTML compositions
  • VideoThumbnail: Client-side film-strip via <video> + canvas for video clips
  • Add playerStore unit tests (265 lines)
  • Add time formatting utilities with tests

🤖 Generated with Claude Code

@miguel-heygen miguel-heygen mentioned this pull request Mar 26, 2026
Merged
@miguel-heygen Graphite App
Copy link
Copy Markdown
Collaborator Author

miguel-heygen commented Mar 26, 2026 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

This was referenced Mar 26, 2026
Merged
Merged
Merged
@miguel-heygen miguel-heygen force-pushed the studio/2-timeline-components branch from e745317 to 5c3dfeb Compare March 26, 2026 18:29
@miguel-heygen miguel-heygen force-pushed the studio/1-player-store branch from 9631ccd to 2c6db38 Compare March 26, 2026 18:29
@miguel-heygen miguel-heygen force-pushed the studio/2-timeline-components branch from 5c3dfeb to 83c21e2 Compare March 26, 2026 18:37
@miguel-heygen miguel-heygen force-pushed the studio/1-player-store branch from 2c6db38 to 4baafb7 Compare March 26, 2026 18:44
@miguel-heygen miguel-heygen force-pushed the studio/2-timeline-components branch 4 times, most recently from 5bf4c98 to 3af8a9f Compare March 26, 2026 20:05
@miguel-heygen miguel-heygen changed the title feat(studio): add TimelineClip, CompositionThumbnail, and VideoThumbnail feat(studio): add TimelineClip and thumbnail components Mar 26, 2026
@vanceingalls
Copy link
Copy Markdown
Collaborator

vanceingalls commented Mar 26, 2026

Review: PR #62 — TimelineClip and Thumbnails

Clean component extraction with good lazy-loading patterns.

Issues

[Important] VideoThumbnail leaks data URLs. Each canvas.toDataURL() creates a string stored in state. On unmount, these are never cleaned up. For long timelines with many video clips, this accumulates memory.

[Important] CompositionThumbnail DOM walk can overshoot. The setRef callback walks parentElement looking for [data-clip]. If rendered outside that context (tests, storybook), it walks to <html> and measures the wrong element.

[Suggestion] VideoThumbnail extractingRef is never reset. If videoSrc changes, the effect re-runs but extractingRef.current is already true, blocking re-extraction. The component can't update when the video source changes.

**[Suggestion] time.test.ts documents formatTime(-1) → "-1:-1" and formatTime(NaN) → "NaN:NaN" as expected. These are user-visible strings that look broken. Consider guarding.

What's well done

  • IntersectionObserver for lazy thumbnail loading, ResizeObserver for responsive frame counts
  • Progressive video frame extraction via seeked event — streams frames as ready
  • Clean memo-ized presentational component in TimelineClip
  • Store tests are thorough with edge cases

@miguel-heygen miguel-heygen force-pushed the studio/2-timeline-components branch from 3af8a9f to b641529 Compare March 26, 2026 20:48
@miguel-heygen miguel-heygen force-pushed the studio/1-player-store branch from 4baafb7 to 7bc8e48 Compare March 26, 2026 20:48
@miguel-heygen
Copy link
Copy Markdown
Collaborator Author

miguel-heygen commented Mar 27, 2026

Thanks Vance!

Addressed:

  • ✅ VideoThumbnail extractingRef is now reset in the cleanup function, so re-extraction works when videoSrc changes

Not addressed (tracked for follow-up):

  • Data URL memory leak — the toDataURL strings accumulate. Will add cleanup via URL.revokeObjectURL pattern or limit retained frames in a follow-up
  • CompositionThumbnail DOM walk overshoot — valid concern for storybook/test contexts, will add a guard
  • formatTime(-1) / formatTime(NaN) returning broken strings — will add input guards to the shared utility

@miguel-heygen miguel-heygen force-pushed the studio/2-timeline-components branch 2 times, most recently from 10ac97e to 63a634d Compare March 27, 2026 01:45
This was referenced Mar 27, 2026
Merged
Merged
@miguel-heygen miguel-heygen force-pushed the studio/2-timeline-components branch from 63a634d to 318fe84 Compare March 27, 2026 19:36
@miguel-heygen miguel-heygen force-pushed the studio/1-player-store branch from 7bc8e48 to 1442902 Compare March 27, 2026 19:36
This was referenced Mar 28, 2026
Merged
Merged
Merged
@miguel-heygen miguel-heygen mentioned this pull request Mar 28, 2026
Merged
7 tasks
@miguel-heygen Graphite App
Copy link
Copy Markdown
Collaborator Author

miguel-heygen commented Mar 28, 2026 •
edited
Loading

Merge activity

  • Mar 28, 7:59 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Mar 28, 8:00 AM UTC: Graphite couldn't merge this PR because it had merge conflicts.
  • Mar 28, 5:19 PM UTC: A user started a stack merge that includes this pull request via Graphite.

@miguel-heygen miguel-heygen changed the base branch from studio/1-player-store to graphite-base/62 March 28, 2026 07:59
@miguel-heygen miguel-heygen changed the base branch from graphite-base/62 to main March 28, 2026 07:59
@miguel-heygen
refactor(studio): improve player store with zoom, element updates, an…
9ccd869 
…d tests

- Add zoom state (zoomMode, pixelsPerSecond) to player store
- Add updateElementDuration, updateElementTrack, updateElement actions
- Remove agent activity tracking (activeEdits, agentId, agentColor)
- Add comprehensive store tests (265 lines)
- Add time utility tests
@miguel-heygen miguel-heygen force-pushed the studio/2-timeline-components branch from 318fe84 to 2826d84 Compare March 28, 2026 17:19
@miguel-heygen
feat(studio): add TimelineClip, CompositionThumbnail, and VideoThumbnail
6d5c925 
- TimelineClip: extracted clip rendering with drag, resize, and selection
- CompositionThumbnail: renders composition preview as timeline thumbnail
- VideoThumbnail: extracts and displays video frame thumbnails via canvas
@miguel-heygen miguel-heygen force-pushed the studio/2-timeline-components branch from 2826d84 to 6d5c925 Compare March 28, 2026 17:42
@miguel-heygen
Copy link
Copy Markdown
Collaborator Author

miguel-heygen commented Mar 28, 2026

Closing — all changes were already merged into main via the stacked PRs above (#63, #64, #65, #94).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vanceingalls vanceingalls vanceingalls approved these changes

@jrusso1020 jrusso1020 Awaiting requested review from jrusso1020

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@miguel-heygen @vanceingalls