Skip to content

feat(timeline): tint video and image clips by element type#796

Open
avillagran wants to merge 1 commit into
OpenCut-app:mainfrom
avillagran:feat/timeline-clip-media-type-color
Open

feat(timeline): tint video and image clips by element type#796
avillagran wants to merge 1 commit into
OpenCut-app:mainfrom
avillagran:feat/timeline-clip-media-type-color

Conversation

@avillagran
Copy link
Copy Markdown

@avillagran avillagran commented May 17, 2026

Summary

Video clips on the timeline previously had a transparent background (`TIMELINE_TRACK_THEME.video.elementClassName = "transparent"`), so they blended into the timeline surface and were only visible once selected. Image clips share the same track type as video, so they had the exact same problem.

This PR adds a subtle per-element-type tint:

element type background
video `bg-sky-500/20` (subtle sky / celeste)
image `bg-amber-500/20` (subtle amber)
audio / text / sticker / graphic / effect unchanged (existing track-level theme)

How

  • Introduces `TIMELINE_ELEMENT_THEME`, a `Partial<Record<ElementType, …>>` map that lets specific element types override the track-level theme.
  • Adds `getTimelineElementClassNameForElement({ elementType, trackType })` which prefers the element-level theme and falls back to `TIMELINE_TRACK_THEME` when no override exists.
  • The single render site in `timeline-element.tsx` now calls the new helper, so the legacy `getTimelineElementClassName` is left intact for any other consumer (none exist today, but the surface stays stable).

The colour values are intentionally low-alpha so the selected-state and hover styling are still clearly the strongest visual signal on a clip.

Test plan

  • Unit tests cover the element-level override, the track-level fallback, and that video and image now produce different class names. `bun test ./apps/web/src/timeline/components/tests/theme.test.ts` → 4 pass.
  • Manual: imported a video and an image into the same track. Both clips are now clearly visible against the timeline background and distinguishable from each other, while audio/text/sticker clips look identical to before.

Notes

  • Configurability (user-customisable colours or a per-project theme) is intentionally out of scope to keep this PR small. If maintainers want it, happy to follow up with a dedicated PR that turns the whole theme into a configurable surface, not just the two new entries.

Summary by CodeRabbit

  • New Features

    • Timeline elements now support individual theme customization alongside track-level theming, with automatic fallback when element-specific theming is not configured.
  • Tests

    • Added comprehensive test suite for timeline theme configuration and styling application.

Review Change Stack

Video clips on the timeline previously had a transparent background,
which made them blend into the timeline surface and only become
visible when selected. Image clips share the same track type as
video and therefore had the same problem.

This adds a per-element-type tint layer that applies a subtle
background colour to clips on the timeline:

- video clips: sky tint (\`bg-sky-500/20\`)
- image clips: amber tint (\`bg-amber-500/20\`)
- audio / text / sticker / graphic / effect: unchanged, still
  driven by \`TIMELINE_TRACK_THEME\`

The new \`TIMELINE_ELEMENT_THEME\` map is a Partial<Record<ElementType, …>>
so the element-level theme can override per type while keeping the
existing track-level theme as the fallback. A small unit test covers
the override and fallback behaviour.
@vercel
Copy link
Copy Markdown

vercel Bot commented May 17, 2026

@avillagran is attempting to deploy a commit to the OpenCut OSS Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 17, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 853cd7fe-fdcc-4a0f-bc5e-a48bed5c5ad9

📥 Commits

Reviewing files that changed from the base of the PR and between fbe3db7 and f1d537e.

📒 Files selected for processing (3)
  • apps/web/src/timeline/components/__tests__/theme.test.ts
  • apps/web/src/timeline/components/theme.ts
  • apps/web/src/timeline/components/timeline-element.tsx

📝 Walkthrough

Walkthrough

This PR adds a per-element-type theming layer to timeline elements. The changes introduce TIMELINE_ELEMENT_THEME, a mapping for element-specific styling, and a helper function that applies element overrides while falling back to track-level defaults. The integration updates timeline element rendering, and tests validate the override and fallback behavior.

Changes

Element-level theming with fallback

Layer / File(s) Summary
Element theme contract and helper
apps/web/src/timeline/components/theme.ts
Imports ElementType, introduces TIMELINE_ELEMENT_THEME constant mapping element types to optional class names with documented fallback strategy, and exports getTimelineElementClassNameForElement() that returns element-specific classes with track-level fallback.
Timeline element styling integration
apps/web/src/timeline/components/timeline-element.tsx
Updates imports to use getTimelineElementClassNameForElement instead of getTimelineElementClassName, and modifies ElementInner to compute CSS classes by passing both elementType and trackType to the new helper.
Element theming validation
apps/web/src/timeline/components/__tests__/theme.test.ts
Test suite validates that track-type mapping works, element-level overrides are preferred when present, elements are distinguished even with shared track types, and fallback to track-level theming occurs when no override exists.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A timeline adorned with element hues,
Each type a choice, a theme to choose,
But if one stumbles on empty ground,
The track-level default waits around—
Fallback wisdom, tested true!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is comprehensive and detailed, covering the problem, solution, implementation approach, test coverage, and design rationale. However, the template requires a checklist for PRs and indicates feature PRs should not be submitted without prior discussion. The PR description should acknowledge the repository's policy stated in the template: feature PRs require prior issue discussion and maintainer approval. Confirm this PR meets those requirements or adjust submission approach.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding element-type-based tinting for video and image clips on the timeline.
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 unit tests (beta)
  • Create PR with unit tests

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.

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