Skip to content

fix(activity-feed-v2): improve deeplink scroll/highlight and bump activity-feed to 3.0.0 - #4765

Merged
mergify[bot] merged 2 commits into
box:masterfrom
abhishek1128:fix/activity-feed-v2-deeplink-scroll
Aug 10, 2026
Merged

fix(activity-feed-v2): improve deeplink scroll/highlight and bump activity-feed to 3.0.0#4765
mergify[bot] merged 2 commits into
box:masterfrom
abhishek1128:fix/activity-feed-v2-deeplink-scroll

Conversation

@abhishek1128

@abhishek1128 abhishek1128 commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Reply and root deeplinks scroll the parent thread card into view with block: 'start' (aligned with activity-feed fade-mask clearance). Thread card in view is enough, no reply expand / scrollToMessage.
  • Deep-link highlight holds for 2s then clears so the card can fade even while activeFeedEntryId stays set; same-thread id changes restart the hold.
  • Bumps @box/activity-feed to 3.0.0 and @box/threaded-annotations to 4.8.7.

Description

Bumps @box/activity-feed to 3.0.0 (scroll alignment / pin-to-bottom) and @box/threaded-annotations to 4.8.7 (highlight fade), then wires Activity Feed v2 deeplink behavior to that API.

Package Before After
@box/activity-feed ^2.4.0 ^3.0.0
@box/threaded-annotations ^4.8.4 ^4.8.7

Ranges updated in both devDependencies and peerDependencies; lockfile updated so each bumped package resolves to a single version.

Other AF/TA peer mins (e.g. user-selector@^3) are unchanged in this PR — AF/TA only need the headless selector APIs already present on ^2.2.22.

Testing

  • Unit tests for activity-feed-v2 (helpers match/resolve, reply → parent scroll, highlight hold + restart)
  • Manual: deeplink to a comment / annotation / reply — thread card scrolls into view under fade mask and highlight fades after ~2s

Summary by CodeRabbit

  • Bug Fixes

    • Improved deep links to replies and annotations by opening and scrolling to the correct parent thread.
    • Added clearer highlighting for linked activity entries, including automatic expiration and refresh when the active entry changes.
    • Improved scrolling behavior to position linked entries at the top of the activity feed.
  • Tests

    • Expanded coverage for deep-link matching, scrolling, and activity-entry highlighting.

@abhishek1128
abhishek1128 requested review from a team as code owners August 10, 2026 21:19
@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Activity-feed dependencies were updated. Deep-link helpers now resolve reply IDs to parent feed rows. Scrolling uses start alignment, and matched rows highlight for two seconds with timer resets on active-entry changes.

Changes

Activity Feed Deep-Link Handling

Layer / File(s) Summary
Deep-link matching and dependency alignment
package.json, src/elements/content-sidebar/activity-feed-v2/helpers.ts, src/elements/content-sidebar/activity-feed-v2/__tests__/helpers.test.ts
Updated activity-feed dependencies. Added matching and resolution for root, comment-reply, and annotation-reply IDs.
Resolved deep-link scrolling
src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx, src/elements/content-sidebar/activity-feed-v2/__tests__/ActivityFeedV2.test.tsx
Activity-feed scrolling resolves reply entries to parent rows and uses { block: 'start' } for initial and retry scrolling.
Timed feed-row highlighting
src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx, src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx
Matched rows highlight for two seconds. The timer clears inactive highlights and restarts when the active entry changes.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ActiveEntry
  participant ActivityFeedV2
  participant resolveFeedItemIdForEntry
  participant FeedItemRow
  ActiveEntry->>ActivityFeedV2: set activeFeedEntryId
  ActivityFeedV2->>resolveFeedItemIdForEntry: resolve root feed-row ID
  resolveFeedItemIdForEntry-->>ActivityFeedV2: return matching row ID
  ActivityFeedV2->>FeedItemRow: scrollTo({ block: 'start' })
  ActiveEntry->>FeedItemRow: update active entry
  FeedItemRow->>FeedItemRow: hold highlight for 2 seconds
Loading

Possibly related PRs

Suggested labels: ready-to-merge, dependencies

Suggested reviewers: ahorowitz123, jmcbgaston, tjuanitas

Poem

A rabbit found a reply in the feed,
And traced it to the thread in need.
The row then glowed,
For two seconds showed,
While scroll and tests hopped with speed.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the deeplink behavior changes and dependency upgrade.
Description check ✅ Passed The description covers the changes, dependency versions, implementation details, and planned testing.
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.
✨ 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.

@socket-security

socket-security Bot commented Aug 10, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updatednpm/​@​box/​activity-feed@​2.4.0 ⏵ 3.0.083 +710087 +19950
Updatednpm/​@​box/​threaded-annotations@​4.8.4 ⏵ 4.8.781 +6100879950

View full report

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx (1)

252-271: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Test the timer reset before the original deadline.

Line 259 expires the first timer before activeFeedEntryId changes. The test cannot detect a stale timer that clears the new highlight at the original deadline. Advance part of the hold duration before rerender, then assert that the highlight remains active after the original deadline and clears only after the restarted deadline.

  • src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx#L252-L271: change the comment-thread test to rerender before the first two-second timer expires.
  • src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx#L540-L561: apply the same timing sequence to the annotation-thread test.
Proposed test timing
- act(() => {
-     jest.advanceTimersByTime(2000);
- });
- expect(lastThreadedAnnotationProps.isHighlighted).toBe(false);
+ act(() => {
+     jest.advanceTimersByTime(1000);
+ });
 
  rerender(<FeedItemRow {...defaultProps} activeFeedEntryId="reply-1" item={mockComment} />);
  expect(lastThreadedAnnotationProps.isHighlighted).toBe(true);
 
  act(() => {
-     jest.advanceTimersByTime(2000);
+     jest.advanceTimersByTime(1000);
  });
+ expect(lastThreadedAnnotationProps.isHighlighted).toBe(true);
 
+ act(() => {
+     jest.advanceTimersByTime(1000);
+ });
  expect(lastThreadedAnnotationProps.isHighlighted).toBe(false);
🤖 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__/FeedItemRow.test.tsx`
around lines 252 - 271, The tests at
src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx#L252-L271
and `#L540-L561` must rerender before the initial two-second timer expires. In
both the comment-thread and annotation-thread tests, advance only part of the
hold duration, change activeFeedEntryId, verify the highlight remains active
past the original deadline, then advance through the restarted deadline and
verify it clears.
🤖 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/__tests__/FeedItemRow.test.tsx`:
- Around line 252-271: The tests at
src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx#L252-L271
and `#L540-L561` must rerender before the initial two-second timer expires. In
both the comment-thread and annotation-thread tests, advance only part of the
hold duration, change activeFeedEntryId, verify the highlight remains active
past the original deadline, then advance through the restarted deadline and
verify it clears.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 587196ea-d656-443a-9ac2-acfc2cf42f33

📥 Commits

Reviewing files that changed from the base of the PR and between 84b90a7 and 9677964.

⛔ Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
📒 Files selected for processing (7)
  • package.json
  • src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx
  • src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx
  • src/elements/content-sidebar/activity-feed-v2/__tests__/ActivityFeedV2.test.tsx
  • src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx
  • src/elements/content-sidebar/activity-feed-v2/__tests__/helpers.test.ts
  • src/elements/content-sidebar/activity-feed-v2/helpers.ts

@abhishek1128 abhishek1128 added dependencies Pull requests that update a dependency file javascript Pull requests that update javascript code labels Aug 10, 2026
…t fade

Resolve reply deeplinks to the parent thread card, scroll with block start
for fade-mask clearance, and time out the highlight so it can fade while
activeFeedEntryId stays set.
@abhishek1128
abhishek1128 force-pushed the fix/activity-feed-v2-deeplink-scroll branch from 9677964 to e1a20ac Compare August 10, 2026 21:37
@mergify

mergify Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • Entered queue2026-08-10 22:56 UTC · Rule: Automatic strict merge · triggered by rule Automatic merge queue
  • Checks skipped · PR is already up-to-date
  • Merged2026-08-10 22:56 UTC · at e1a20ac29a8eb18158dd3ccecfe0a3995d161b22 · squash

This pull request spent 13 seconds in the queue, including 3 seconds running CI.

Required conditions to merge
  • github-review-approved [🛡 GitHub branch protection]
  • any of [🛡 GitHub branch protection]:
    • check-success = Summary
    • check-neutral = Summary
    • check-skipped = Summary
  • any of [🛡 GitHub branch protection]:
    • check-success = lint_test_build
    • check-neutral = lint_test_build
    • check-skipped = lint_test_build
  • any of [🛡 GitHub branch protection]:
    • check-success = license/cla
    • check-neutral = license/cla
    • check-skipped = license/cla
  • any of [🛡 GitHub branch protection]:
    • check-success = lint_pull_request
    • check-neutral = lint_pull_request
    • check-skipped = lint_pull_request

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

Labels

dependencies Pull requests that update a dependency file javascript Pull requests that update javascript code ready-to-merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants