feat(activity-feed-v2): emit timestamped comment markers to video player by kduncanhsu · Pull Request #4651 · box/box-ui-elements

kduncanhsu · 2026-06-24T23:47:34Z

Summary

ActivityFeedV2 now extracts timestamped comments and frame annotations from filteredItems and emits them to the video viewer via the commentmarkers event
Listens for commentmarkerselect events from the viewer to scroll the sidebar to the selected item when a scrubber marker is clicked
Threads the existing getViewer prop from SidebarPanels → ActivitySidebar → ActivityFeedV2

Modified files

ActivityFeedV2.tsx — Added useEffect to build markers array and emit to viewer, registers/cleans up commentmarkerselect listener
types.ts — Added getViewer prop to ActivityFeedV2Props
ActivitySidebar.js — Added getViewer to props type, passes it to ActivityFeedV2
SidebarPanels.js — Passes getViewer to LoadableActivitySidebar
ActivityFeedV2.test.tsx — 10 new tests for marker emission, listener lifecycle, and filtering

Depends on

box-content-preview PR that adds the commentmarkers event listener and commentmarkerselect event emission

Test plan

Open a video file with timestamped comments — verify markers appear on scrubber
Open a video with frame annotations — verify annotation markers appear
Verify non-timestamped comments and page-based annotations do not produce markers
Click a marker on the scrubber — verify the sidebar scrolls to the corresponding feed item
Post a new timestamped comment — verify markers update
Filter the feed (e.g., "only mentions me") — verify markers update to match filtered items
Switch to a non-video file — verify no markers are emitted

Summary by CodeRabbit

New Features
- Video activity feeds can now synchronize comment/annotation “markers” with the viewer when available via an optional viewer callback passed through the activity sidebar.
- Selecting a viewer marker scrolls directly to the corresponding activity item.
Bug Fixes
- Marker emission is now correctly gated to video content and timestamped/frame-based annotations, with proper viewer event setup and cleanup.
Tests
- Added test coverage for marker payload accuracy, viewer event wiring, filtering behavior, and unmount cleanup.

coderabbitai · 2026-06-24T23:47:49Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

Walkthrough

Activity sidebar routing now passes an optional viewer getter into ActivityFeedV2. The feed emits video comment markers from filtered items, listens for marker selection, and scrolls matching activity entries into view. Tests cover marker payloads, filtering, and listener cleanup.

Changes

Activity feed viewer markers

Layer / File(s)	Summary
Viewer prop plumbing `src/elements/content-sidebar/activity-feed-v2/types.ts`, `src/elements/content-sidebar/SidebarPanels.js`, `src/elements/content-sidebar/ActivitySidebar.js`, `src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx`	`getViewer` is added to the feed props contract and threaded through the activity sidebar route into `ActivityFeedV2`.
Comment marker effect `src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx`	`ActivityFeedV2` builds marker payloads from filtered video items, emits them to the viewer, and scrolls the matching activity element when `comment_marker_select` fires.
Comment marker tests `src/elements/content-sidebar/activity-feed-v2/__tests__/ActivityFeedV2.test.tsx`	The test suite covers marker emission, listener registration and cleanup, and filtering for timestamped versus non-timestamped items.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

box/box-ui-elements#4611: Introduces timestamp-derived feed item data that the new marker payloads consume.
box/box-ui-elements#4620: Extends ActivityFeedV2 around timestamped video comments and related item shaping.

Suggested labels

ready-to-merge

Suggested reviewers

jackiejou
JChan106
ahorowitz123

Poem

A rabbit met markers in sidebar light,
They hopped from the feed and lined up just right.
One viewer said “select,” and the comments would roam,
Boing back to their spots, like they’d found their home.
🐇✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: emitting timestamped comment markers from ActivityFeedV2 to the video player.
Description check	✅ Passed	The description includes a clear summary, modified files, dependency, and test plan, so it is sufficiently complete.
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.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/elements/content-sidebar/activity-feed-v2/__tests__/ActivityFeedV2.test.tsx (1)
1141-1145: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert that cleanup removes the same callback instance.

This test still passes if removeListener gets a different function than the one registered in addListener. Capture the listener passed at registration time and assert cleanup removes that exact reference, so leaks in this path are caught.
🤖 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
`@src/elements/content-sidebar/activity-feed-v2/__tests__/ActivityFeedV2.test.tsx`
around lines 1141 - 1145, The cleanup test for the commentmarkerselect listener
is too weak because it only checks that removeListener is called with some
function, not the exact one that was registered. Update the ActivityFeedV2 test
to capture the callback passed to mockViewer.addListener during
renderComponentWithMarkers and assert unmount calls mockViewer.removeListener
with that same function reference. Use the existing addListener/removeListener
behavior in ActivityFeedV2 to verify the registered listener is the one cleaned
up.

🤖 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 `@src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx`:
- Around line 350-360: Clear the viewer’s marker state during teardown because
the current cleanup in ActivityFeedV2 only removes the commentmarkerselect
listener and leaves the last commentmarkers payload behind. Update the marker
lifecycle around viewer.emit('commentmarkers', markers) and the returned cleanup
so unmounting or stopping the feed also emits an empty marker set (or equivalent
reset) before detaching the listener. Keep marker emission separate from the
listener setup in handleMarkerSelect so normal filter updates don’t briefly
clear and re-add markers.

---

Nitpick comments:
In
`@src/elements/content-sidebar/activity-feed-v2/__tests__/ActivityFeedV2.test.tsx`:
- Around line 1141-1145: The cleanup test for the commentmarkerselect listener
is too weak because it only checks that removeListener is called with some
function, not the exact one that was registered. Update the ActivityFeedV2 test
to capture the callback passed to mockViewer.addListener during
renderComponentWithMarkers and assert unmount calls mockViewer.removeListener
with that same function reference. Use the existing addListener/removeListener
behavior in ActivityFeedV2 to verify the registered listener is the one cleaned
up.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 68221e7c-15c9-4207-a9a4-cb2b9741a893

📥 Commits

Reviewing files that changed from the base of the PR and between c833428 and 7d5c5f2.

📒 Files selected for processing (5)

src/elements/content-sidebar/ActivitySidebar.js
src/elements/content-sidebar/SidebarPanels.js
src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx
src/elements/content-sidebar/activity-feed-v2/__tests__/ActivityFeedV2.test.tsx
src/elements/content-sidebar/activity-feed-v2/types.ts

jmcbgaston

awesome

coderabbitai

🧹 Nitpick comments (1)

src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx (1)
359-374: 🚀 Performance & Scalability | 🔵 Trivial

Confirm getViewer identity stability to prevent effect churn

The effect at lines 359–374 depends on getViewer. Tracing the component tree in SidebarPanels and ActivitySidebar shows getViewer is passed down as a prop (e.g., line 1429 in ActivitySidebar) rather than defined inline. If the upstream parent recreates the getViewer function on every render (e.g., inline () => this.viewer.getViewer()), this effect will tear down and re-attach the listener on every update. The cleanup function emits commentmarkers: [] during this churn, potentially causing scrubber marker flicker. Consider moving the effect inside a child wrapper that receives a stable useCallback factory or useMemo-ized getViewer.
🤖 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 `@src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx` around
lines 359 - 374, The ActivityFeedV2 effect that registers the
`commentmarkerselect` listener is churn-prone because it depends on `getViewer`,
which may be recreated by upstream props from `ActivitySidebar`/`SidebarPanels`.
Make `getViewer` stable before it reaches `ActivityFeedV2` (for example by
wrapping the factory in `useCallback` or memoizing the prop) so the
`React.useEffect` in `ActivityFeedV2` only reattaches when the actual viewer
changes. Keep the listener setup/cleanup in sync with the stable viewer identity
to avoid repeated `commentmarkers` clears and marker flicker.

🤖 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 `@src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx`:
- Around line 359-374: The ActivityFeedV2 effect that registers the
`commentmarkerselect` listener is churn-prone because it depends on `getViewer`,
which may be recreated by upstream props from `ActivitySidebar`/`SidebarPanels`.
Make `getViewer` stable before it reaches `ActivityFeedV2` (for example by
wrapping the factory in `useCallback` or memoizing the prop) so the
`React.useEffect` in `ActivityFeedV2` only reattaches when the actual viewer
changes. Keep the listener setup/cleanup in sync with the stable viewer identity
to avoid repeated `commentmarkers` clears and marker flicker.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8f500df1-0788-4f60-bfb4-ab2deee7bfae

📥 Commits

Reviewing files that changed from the base of the PR and between 3539750 and d5d15d0.

📒 Files selected for processing (3)

src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx
src/elements/content-sidebar/activity-feed-v2/__tests__/ActivityFeedV2.test.tsx
src/elements/content-sidebar/activity-feed-v2/types.ts

🚧 Files skipped from review as they are similar to previous changes (1)

src/elements/content-sidebar/activity-feed-v2/tests/ActivityFeedV2.test.tsx

mergify · 2026-06-26T07:03:46Z

Queued — the merge queue status continues in this comment ↓.

mergify · 2026-06-26T07:03:52Z

kduncanhsu requested review from a team as code owners June 24, 2026 23:47

coderabbitai Bot reviewed Jun 24, 2026

View reviewed changes

Comment thread src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx Outdated

jmcbgaston reviewed Jun 25, 2026

View reviewed changes

Comment thread src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx

jmcbgaston reviewed Jun 25, 2026

View reviewed changes

Comment thread src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx Outdated

jmcbgaston reviewed Jun 25, 2026

View reviewed changes

Comment thread src/elements/content-sidebar/activity-feed-v2/types.ts Outdated

kduncanhsu force-pushed the kduncanhsu/annotations-in-scrubber-bar-plumbing branch from 0cbfbee to 3539750 Compare June 25, 2026 16:37

jmcbgaston previously approved these changes Jun 25, 2026

View reviewed changes

coderabbitai Bot reviewed Jun 25, 2026

View reviewed changes

kduncanhsu dismissed jmcbgaston’s stale review via 3401384 June 25, 2026 19:06

kduncanhsu added 4 commits June 25, 2026 12:09

feat(activity-feed-v2): emit timestamped comment markers to video player

15e9707

fix(scroll): fix scroll to comment on marker click

6c3dfd2

refactor(activity-feed-v2): address PR comments

cab3120

refactor(activity-feed-v2): revert some changes and change event names

928ca74

kduncanhsu force-pushed the kduncanhsu/annotations-in-scrubber-bar-plumbing branch from 3401384 to 928ca74 Compare June 25, 2026 19:12

tjiang-box previously approved these changes Jun 25, 2026

View reviewed changes

refactor(activity-feed-v2): revert back to single useEffect

71af51c

kduncanhsu dismissed tjiang-box’s stale review via 71af51c June 25, 2026 20:39

mrscobbler approved these changes Jun 25, 2026

View reviewed changes

jmcbgaston approved these changes Jun 25, 2026

View reviewed changes

tjiang-box approved these changes Jun 26, 2026

View reviewed changes

kduncanhsu added the ready-to-merge label Jun 26, 2026

mergify Bot added the queued label Jun 26, 2026

mergify Bot merged commit 51720a1 into box:master Jun 26, 2026
9 of 10 checks passed

mergify Bot removed the queued label Jun 26, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(activity-feed-v2): emit timestamped comment markers to video player#4651

feat(activity-feed-v2): emit timestamped comment markers to video player#4651
mergify[bot] merged 5 commits into
box:masterfrom
kduncanhsu:kduncanhsu/annotations-in-scrubber-bar-plumbing

kduncanhsu commented Jun 24, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 24, 2026 •

edited

Loading

Reviews paused

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jmcbgaston left a comment

Uh oh!

coderabbitai Bot left a comment

Uh oh!

mergify Bot commented Jun 26, 2026 •

edited

Loading

Uh oh!

mergify Bot commented Jun 26, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

kduncanhsu commented Jun 24, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Modified files

Depends on

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jmcbgaston left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

mergify Bot commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mergify Bot commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge Queue Status

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kduncanhsu commented Jun 24, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 24, 2026 •

edited

Loading

mergify Bot commented Jun 26, 2026 •

edited

Loading

mergify Bot commented Jun 26, 2026 •

edited

Loading