fix(video-annotations): Show timestamp badge and support deep-link#4575
Conversation
Activity Feed v2 rendered video annotations as "Page 4623", treating the millisecond frame offset as a page number. The annotation badge component in @box/threaded-annotations already supports an AnnotationBadgeType.Frame variant that renders a timestamp string, but the BUIE transformer never emitted it. - annotationTargetToBadge now detects target.location.type === "frame" and emits a Frame badge with the value formatted as M:SS or H:MM:SS. - Added convertMillisecondsToTimestamp util alongside the existing HMMSS helper, omitting the hour field for sub-hour durations. - Added a Frame Flow type and widened TargetDrawing and TargetRegion location to Frame | Page. Highlight and Point stay Page-only. - Wired ContentPreview startAt to support frame deep-link by mapping frame to seconds and converting the millisecond value, so clicking a cross-version video annotation seeks the player to the right time.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds a ChangesFrame Annotation Support with Display Formatting
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Merge Queue Status
This pull request spent 10 minutes 30 seconds in the queue, including 10 minutes 6 seconds running CI. Required conditions to merge
|
Summary
Activity Feed v2 rendered video annotations as "Page 4623", treating the millisecond frame offset as a page number. Also wires up
startAtdeep-link so clicking a cross-version video annotation seeks the player to the right time instead of starting at 0:00.annotationTargetToBadgenow detectstarget.location.type === 'frame'and emits a Frame badge with the value formatted asM:SS(sub-hour) orH:MM:SS. The downstream@box/threaded-annotationsalready supportsAnnotationBadgeType.Framewith atimestampstring.convertMillisecondsToTimestamputil alongside the existingconvertMillisecondsToHMMSS, dropping the hour field for sub-hour durations.FrameFlow type.TargetDrawing.locationandTargetRegion.locationwidened toFrame | Page. Highlight and Point stayPage-only (text selection / point coordinates are document-only primitives).ContentPreview.handleAnnotationSelectmapsframe→secondsand converts ms → seconds before passing tosetState({ startAt }), enabling video deep-link across versions.Test plan
0:04) instead of "Page 4623".M:SSand over-an-hour asH:MM:SS.Page Nbadges and deep-link correctly.yarn flow checkpasses.yarn tsc --noEmitpasses.transformers,timestamp, andContentPreview.Summary by CodeRabbit
New Features
Tests