Skip to content

fix(video-annotations): Show timestamp badge and support deep-link#4575

Merged
mergify[bot] merged 2 commits into
box:masterfrom
jackiejou:fix/video-annotation-frame-support
May 21, 2026
Merged

fix(video-annotations): Show timestamp badge and support deep-link#4575
mergify[bot] merged 2 commits into
box:masterfrom
jackiejou:fix/video-annotation-frame-support

Conversation

@jackiejou
Copy link
Copy Markdown
Contributor

@jackiejou jackiejou commented May 20, 2026

Summary

Activity Feed v2 rendered video annotations as "Page 4623", treating the millisecond frame offset as a page number. Also wires up startAt deep-link so clicking a cross-version video annotation seeks the player to the right time instead of starting at 0:00.

  • annotationTargetToBadge now detects target.location.type === 'frame' and emits a Frame badge with the value formatted as M:SS (sub-hour) or H:MM:SS. The downstream @box/threaded-annotations already supports AnnotationBadgeType.Frame with a timestamp string.
  • New convertMillisecondsToTimestamp util alongside the existing convertMillisecondsToHMMSS, dropping the hour field for sub-hour durations.
  • New Frame Flow type. TargetDrawing.location and TargetRegion.location widened to Frame | Page. Highlight and Point stay Page-only (text selection / point coordinates are document-only primitives).
  • ContentPreview.handleAnnotationSelect maps frameseconds and converts ms → seconds before passing to setState({ startAt }), enabling video deep-link across versions.

Test plan

  • Open a video file with frame annotations and confirm the activity feed shows a timestamp badge (e.g. 0:04) instead of "Page 4623".
  • Confirm sub-hour timestamps render as M:SS and over-an-hour as H:MM:SS.
  • Click a video annotation that points to a different file version; verify the preview reloads and seeks to the annotated time rather than starting at 0:00.
  • Click a video annotation on the current version; verify scroll/highlight still works as before.
  • Verify document (page) annotations still render Page N badges and deep-link correctly.
  • yarn flow check passes.
  • yarn tsc --noEmit passes.
  • Unit tests pass for transformers, timestamp, and ContentPreview.

Summary by CodeRabbit

  • New Features

    • Support for frame-based annotations with numeric timestamps.
    • Frame annotations show formatted timestamps (M:SS or H:MM:SS) and default to 0:00 when missing.
    • Content preview uses frame timestamps to position playback accurately.
  • Tests

    • Added/updated tests covering frame timestamp formatting and preview start-time behavior.

Review Change Stack

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.
@jackiejou jackiejou requested review from a team as code owners May 20, 2026 20:13
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 20, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9e4f2d0b-08ff-4ab0-9d55-a2b778990c49

📥 Commits

Reviewing files that changed from the base of the PR and between 2fac51e and 6b5a48f.

📒 Files selected for processing (2)
  • src/elements/content-sidebar/activity-feed-v2/__tests__/transformers.test.ts
  • src/elements/content-sidebar/activity-feed-v2/transformers.ts

Walkthrough

Adds a Frame annotation type, a millisecond→timestamp formatter, and updates content preview and activity-feed badge logic/tests to handle frame-based annotation locations and formatted timestamps.

Changes

Frame Annotation Support with Display Formatting

Layer / File(s) Summary
Frame type and annotation target contracts
src/common/types/annotations.js
Adds exported Frame type with numeric value; updates TargetDrawing.location and TargetRegion.location to Frame | Page.
Millisecond-to-timestamp converter utility
src/utils/timestamp.ts, src/utils/timestamp.js.flow, src/utils/__tests__/timestamp.test.js
Adds and exports convertMillisecondsToTimestamp(timestampInMilliseconds: number): string, updates Flow declaration and export list, and adds tests for sub-hour, hour-or-longer, and invalid inputs.
Activity feed frame annotation badge display
src/elements/content-sidebar/activity-feed-v2/transformers.ts, src/elements/content-sidebar/activity-feed-v2/__tests__/transformers.test.ts
Extends annotationTargetToBadge to handle location.type === 'frame', returning a Frame badge with timestamp formatted via convertMillisecondsToTimestamp; tests cover formatting and missing-value fallback.
Content preview frame annotation state handling
src/elements/content-preview/ContentPreview.js, src/elements/content-preview/__tests__/ContentPreview.test.js
Imports convertTimestampToSeconds, constrains startAtTypes with $ReadOnly, and converts frame location.value (ms) to seconds when setting startAt; tests adjusted/added for cross-version frame/page behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • box/box-ui-elements#4380: Related adjustments to ContentPreview.handleAnnotationSelect for frame annotation handling.
  • box/box-ui-elements#4318: Related work adding support for location.type === 'frame' and timestamp formatting in sidebar/activity flows.

Suggested reviewers

  • ahorowitz123
  • jfox-box
  • JChan106

Poem

🐰
I hopped through frames and counted time,
Milliseconds shaped into rhyme,
Badges gleam and previews start—
A tiny change, a joyous heart.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main changes: fixing video annotation handling by showing timestamp badges and supporting deep-linking via the frame feature.
Description check ✅ Passed The description comprehensively covers the objectives, implementation details, and test plan, clearly explaining the new Frame type, timestamp formatting, and deep-link functionality.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented May 21, 2026

Merge Queue Status

  • Entered queue2026-05-21 17:05 UTC · Rule: Automatic strict merge
  • Checks passed · in-place
  • Merged2026-05-21 17:16 UTC · at 6b5a48fc13f32aecf5eed8265155423094885b4a · squash

This pull request spent 10 minutes 30 seconds in the queue, including 10 minutes 6 seconds running CI.

Required conditions to merge

@mergify mergify Bot merged commit 490df9e into box:master May 21, 2026
10 of 11 checks passed
@mergify mergify Bot removed the queued label May 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants