Add VideoEventTimeline component#1672
Conversation
…ions' into horatiu-lig-9805-import-activitynet-style-event-annotations-3
…ent-annotations-3
…ent-annotations-3
Previously, filtering and counting video annotations only considered annotations attached to a video's frames. Videos can also carry annotations directly (e.g. ActivityNet-style event/classification labels on the whole video). This makes both the video filter and the annotation counter consider these direct video annotations in addition to frame annotations.
…ions-3' into horatiu-lig-9806-filter-videos-by-event-metadata
Mirrors a <video> element's playback state into reactive fields and exposes intent callbacks (seek, play/pause, mute, fullscreen) for a custom control bar. Listening via native events keeps it composable with handlers on the element. Covered by a harness-driven unit test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Compose useVideoPlayback + VideoControls, forcing native <video controls> off so the full-width scrubber can host aligned timeline overlays. Adds a startTimeS prop (null waits for a deep-link timestamp) and a region ref for fullscreen. Updates tests and stories accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Hand the deep-linked frame's timestamp to VideoPlayer via startTimeS instead of seeking after load, and remount on frame-number change with a composite #key. Align the annotation overlay to the <video> box and fix min-h-0 layout so the player fills its card. Adds a VideoDetails test covering the frame-load branch and deep-link handoff. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Full-width scrubber + transport buttons that own no playback state and call back on user intent. Includes helpers (formatTime, clampPercent, timeFromClientX) and unit tests. Exported from the components barrel. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Interactive Playground plus paused / near-end states, on a dark backdrop matching how the bar overlays a video. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Hand the deep-linked frame's timestamp to VideoPlayer via startTimeS instead of seeking after load, and remount on frame-number change with a composite #key. Align the annotation overlay to the <video> box and fix min-h-0 layout so the player fills its card. Adds a VideoDetails test covering the frame-load branch and deep-link handoff. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Compose useVideoPlayback + VideoControls, forcing native <video controls> off so the full-width scrubber can host aligned timeline overlays. Adds a startTimeS prop (null waits for a deep-link timestamp) and a region ref for fullscreen. Updates tests and stories accordingly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mirrors a <video> element's playback state into reactive fields and exposes intent callbacks (seek, play/pause, mute, fullscreen) for a custom control bar. Listening via native events keeps it composable with handlers on the element. Covered by a harness-driven unit test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Interactive Playground plus paused / near-end states, on a dark backdrop matching how the bar overlays a video.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe pull request adds a reusable ChangesVideo event timeline
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
…ts-model' into horatiu-lig-9807-show-event-bars-on-the-timeline-3-events-timeline
|
/review |
…o horatiu-lig-9807-show-event-bars-on-the-timeline.b
…o horatiu-lig-9807-show-event-bars-on-the-timeline.c
…o horatiu-lig-9807-show-event-bars-on-the-timeline.d
…o horatiu-lig-9807-show-event-bars-on-the-timeline-3-events-model
…ts-model' into horatiu-lig-9807-show-event-bars-on-the-timeline-3-events-timeline
…line-3-events-timeline
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@lightly_studio_view/src/lib/components/VideoEventTimeline/VideoEventTimeline.svelte`:
- Line 111: Update the event button styling in VideoEventTimeline to explicitly
set its text color to event.contrastColor alongside the existing background and
border colors, ensuring labels remain readable regardless of event.color or
inherited theme styles.
- Around line 96-100: Update the event-bar calculations in the timeline markup
around leftPercent and widthPercent so the computed bar width cannot extend past
100%; clamp leftPercent against the available track space after applying
MIN_BAR_WIDTH_PERCENT, while preserving the existing minimum-width behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ad3acef1-31d9-4909-a4f4-2fed8f46ab7e
📒 Files selected for processing (4)
lightly_studio_view/src/lib/components/VideoEventTimeline/VideoEventTimeline.stories.sveltelightly_studio_view/src/lib/components/VideoEventTimeline/VideoEventTimeline.sveltelightly_studio_view/src/lib/components/VideoEventTimeline/VideoEventTimeline.test.tslightly_studio_view/src/lib/components/index.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lightly_studio_view/src/lib/components/VideoEventTimeline/VideoEventTimeline.stories.svelte (1)
53-53: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove extraneous
asChildprop.The
<Story>component from@storybook/addon-svelte-csfdoes not use anasChildprop and simply renders its default slot. Including it might cause an unexpected prop warning or TypeScript error.♻️ Proposed fix
- <Story name="Playground" asChild> + <Story name="Playground">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lightly_studio_view/src/lib/components/VideoEventTimeline/VideoEventTimeline.stories.svelte` at line 53, Remove the extraneous asChild prop from the Playground Story declaration in VideoEventTimeline.stories.svelte, leaving the Story component to render its default slot without additional props.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@lightly_studio_view/src/lib/components/VideoEventTimeline/VideoEventTimeline.stories.svelte`:
- Line 53: Remove the extraneous asChild prop from the Playground Story
declaration in VideoEventTimeline.stories.svelte, leaving the Story component to
render its default slot without additional props.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 179aea7a-15ae-4281-a858-310e0b3fdf0a
📒 Files selected for processing (1)
lightly_studio_view/src/lib/components/VideoEventTimeline/VideoEventTimeline.stories.svelte
What has changed and why?
Adds a presentation-only VideoEventTimeline component that renders VideoEvents as clickable bars positioned by start/end time, stacked into lanes, with an optional playhead and header. Clicking a bar calls onSeek; very short events keep a minimum clickable width. It takes already-derived events (from PR #1671), holds no playback state, and is exported for reuse. Includes Storybook stories.
How has it been tested?
Component tests in VideoEventTimeline.test.ts plus Storybook stories for the empty, single-lane, and overlapping-events cases.
Did you update CHANGELOG.md?
Summary by CodeRabbit
VideoEventTimelinecomponent to display time-based events as clickable, lane-stacked bars on a timeline.