feat(activity-feed): add new v2 placeholder#4460
Merged
Merged
Conversation
Contributor
WalkthroughThis pull request introduces a new ActivityFeedV2 component with feature flag support. The ActivitySidebar now conditionally renders ActivityFeedV2 based on the threadedRepliesV2 feature flag, falling back to the existing ActivityFeed when disabled. Corresponding tests verify both rendering paths. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/elements/content-sidebar/__tests__/ActivitySidebar.rtl.test.js (1)
517-539: Add one precedence test for overlapping feed flags.Consider adding a case where both
activityFeed.newThreadedReplies.enabledandactivityFeed.threadedRepliesV2.enabledaretrue, and assert V2 still wins. This locks intended behavior if both rollouts are active together.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/elements/content-sidebar/__tests__/ActivitySidebar.rtl.test.js` around lines 517 - 539, Add a new test in the describe('render() - threadedRepliesV2 feature gate') suite that calls renderActivitySidebar with features set so both activityFeed.newThreadedReplies.enabled and activityFeed.threadedRepliesV2.enabled are true, then assert that ActivityFeedV2 is rendered (expect getByTestId('activity-feed-adapter-v2') toBeInTheDocument()) and that the legacy feed is not (expect queryByTestId('activity-feed-mock') not.toBeInTheDocument()); place this alongside the existing tests so renderActivitySidebar and the same test ids are reused to lock V2 precedence when both flags overlap.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/elements/content-sidebar/ActivitySidebar.js`:
- Around line 1317-1328: The V2 feature branch renders SidebarContent without
the existing actions and full-layout class, causing loss of filter/task actions
and layout parity; update the isThreadedRepliesV2Enabled branch in
ActivitySidebar.js so the SidebarContent includes actions={this.renderActions()}
and preserves the conditional "bcs-activity--full" CSS class (same logic used in
the non-V2 branch), ensuring the V2 path mirrors the original SidebarContent
props and classes.
---
Nitpick comments:
In `@src/elements/content-sidebar/__tests__/ActivitySidebar.rtl.test.js`:
- Around line 517-539: Add a new test in the describe('render() -
threadedRepliesV2 feature gate') suite that calls renderActivitySidebar with
features set so both activityFeed.newThreadedReplies.enabled and
activityFeed.threadedRepliesV2.enabled are true, then assert that ActivityFeedV2
is rendered (expect getByTestId('activity-feed-adapter-v2') toBeInTheDocument())
and that the legacy feed is not (expect queryByTestId('activity-feed-mock')
not.toBeInTheDocument()); place this alongside the existing tests so
renderActivitySidebar and the same test ids are reused to lock V2 precedence
when both flags overlap.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/elements/content-sidebar/ActivitySidebar.jssrc/elements/content-sidebar/__tests__/ActivitySidebar.rtl.test.jssrc/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsxsrc/elements/content-sidebar/activity-feed-v2/__tests__/ActivityFeedV2.test.tsxsrc/elements/content-sidebar/activity-feed-v2/index.ts
jackiejou
force-pushed
the
activity-feed-stub
branch
from
7852f24 to
0d6ef56
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/elements/content-sidebar/ActivitySidebar.js (1)
1317-1328:⚠️ Potential issue | 🟠 MajorV2 branch regresses SidebarContent chrome parity (actions + full layout class).
Line 1319 renders
SidebarContentwithoutactions={this.renderActions()}, and Line 1320 drops the conditionalbcs-activity--fullclass. That changes behavior/UI when the V2 flag is enabled.Proposed parity fix
if (isThreadedRepliesV2Enabled) { return ( <SidebarContent - className="bcs-activity" + actions={this.renderActions()} + className={classNames('bcs-activity', { 'bcs-activity--full': hasReplies })} elementId={elementId} sidebarView={SIDEBAR_VIEW_ACTIVITY} title={this.renderTitle()} > <ActivityFeedV2 /> </SidebarContent> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/elements/content-sidebar/ActivitySidebar.js` around lines 1317 - 1328, The V2 branch omits the existing SidebarContent chrome: when isThreadedRepliesV2Enabled returns true, render SidebarContent with the same props as the V1 branch—include actions={this.renderActions()} and preserve the conditional full-layout className (e.g., include "bcs-activity--full" when appropriate) so the V2 path around SidebarContent (wrapping ActivityFeedV2) matches the original chrome and layout used for SIDEBAR_VIEW_ACTIVITY, elementId, and title={this.renderTitle()}.
🧹 Nitpick comments (1)
src/elements/content-sidebar/__tests__/ActivitySidebar.rtl.test.js (1)
517-540: Nice gate-path coverage; add one parity assertion for SidebarContent props.These tests correctly verify feed switching. Recommend adding a V2-enabled assertion for sidebar chrome parity (
actionspresent andbcs-activity--fullwhenhasRepliesis true) to prevent regressions like the current one.Suggested test addition
+ test('should preserve SidebarContent chrome when threadedRepliesV2 is enabled', () => { + renderActivitySidebar({ + hasReplies: true, + features: { activityFeed: { threadedRepliesV2: { enabled: true } } }, + }); + + expect(screen.getByTestId('sidebar-content-mock')).toHaveClass('bcs-activity--full'); + expect(screen.getByTestId('sidebar-actions')).not.toBeEmptyDOMElement(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/elements/content-sidebar/__tests__/ActivitySidebar.rtl.test.js` around lines 517 - 540, Add a V2-enabled parity assertion to the existing threadedRepliesV2 tests by updating the test that enables threadedRepliesV2 in the describe block: after calling renderActivitySidebar({ features: { activityFeed: { threadedRepliesV2: { enabled: true } } } }), assert that the sidebar chrome props match the non-V2 behavior — verify that the actions element is present (using the same test-id/class used for actions) and that the sidebar container has the bcs-activity--full class when hasReplies is true; use the existing renderActivitySidebar helper and query utilities (screen.getByTestId / getByClassName equivalent) so the new assertions sit right after the existing ActivityFeedV2 presence checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/elements/content-sidebar/ActivitySidebar.js`:
- Around line 1317-1328: The V2 branch omits the existing SidebarContent chrome:
when isThreadedRepliesV2Enabled returns true, render SidebarContent with the
same props as the V1 branch—include actions={this.renderActions()} and preserve
the conditional full-layout className (e.g., include "bcs-activity--full" when
appropriate) so the V2 path around SidebarContent (wrapping ActivityFeedV2)
matches the original chrome and layout used for SIDEBAR_VIEW_ACTIVITY,
elementId, and title={this.renderTitle()}.
---
Nitpick comments:
In `@src/elements/content-sidebar/__tests__/ActivitySidebar.rtl.test.js`:
- Around line 517-540: Add a V2-enabled parity assertion to the existing
threadedRepliesV2 tests by updating the test that enables threadedRepliesV2 in
the describe block: after calling renderActivitySidebar({ features: {
activityFeed: { threadedRepliesV2: { enabled: true } } } }), assert that the
sidebar chrome props match the non-V2 behavior — verify that the actions element
is present (using the same test-id/class used for actions) and that the sidebar
container has the bcs-activity--full class when hasReplies is true; use the
existing renderActivitySidebar helper and query utilities (screen.getByTestId /
getByClassName equivalent) so the new assertions sit right after the existing
ActivityFeedV2 presence checks.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/elements/content-sidebar/ActivitySidebar.jssrc/elements/content-sidebar/__tests__/ActivitySidebar.rtl.test.jssrc/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsxsrc/elements/content-sidebar/activity-feed-v2/__tests__/ActivityFeedV2.test.tsxsrc/elements/content-sidebar/activity-feed-v2/index.js
🚧 Files skipped from review as they are similar to previous changes (2)
- src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx
- src/elements/content-sidebar/activity-feed-v2/tests/ActivityFeedV2.test.tsx
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/elements/content-sidebar/ActivitySidebar.js (1)
1318-1327:⚠️ Potential issue | 🟠 MajorPreserve SidebarContent parity in the V2 flag path.
Line 1320 omits
actions={this.renderActions()}, and Line 1321 hardcodesbcs-activitywithout the existingbcs-activity--fullcondition. WhenactivityFeed.threadedRepliesV2.enabledis on, this drops filter/task actions and breaks layout parity.💡 Suggested parity fix
if (isThreadedRepliesV2Enabled) { return ( <SidebarContent - className="bcs-activity" + actions={this.renderActions()} + className={classNames('bcs-activity', { 'bcs-activity--full': hasReplies })} elementId={elementId} sidebarView={SIDEBAR_VIEW_ACTIVITY} title={this.renderTitle()} > <ActivityFeedV2 /> </SidebarContent> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/elements/content-sidebar/ActivitySidebar.js` around lines 1318 - 1327, The V2 flag branch for isThreadedRepliesV2Enabled is missing the actions prop and the full-width class; update the SidebarContent JSX inside that branch to pass actions={this.renderActions()} and compute className the same way as the non-V2 branch (preserving "bcs-activity" and conditionally adding "bcs-activity--full" when appropriate) so ActivityFeedV2 renders with the same actions and layout parity; look for SidebarContent, isThreadedRepliesV2Enabled, and this.renderActions() to apply the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/elements/content-sidebar/ActivitySidebar.js`:
- Around line 1318-1327: The V2 flag branch for isThreadedRepliesV2Enabled is
missing the actions prop and the full-width class; update the SidebarContent JSX
inside that branch to pass actions={this.renderActions()} and compute className
the same way as the non-V2 branch (preserving "bcs-activity" and conditionally
adding "bcs-activity--full" when appropriate) so ActivityFeedV2 renders with the
same actions and layout parity; look for SidebarContent,
isThreadedRepliesV2Enabled, and this.renderActions() to apply the change.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/elements/content-sidebar/ActivitySidebar.jssrc/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx
ahorowitz123
approved these changes
Mar 3, 2026
Contributor
Merge Queue StatusRule:
This pull request spent 5 seconds in the queue, with no time running CI. Required conditions to merge
|
6 tasks
9 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ActivityFeedV2placeholder component in a newactivity-feed-v2/directoryActivitySidebarbehindactivityFeed.threadedRepliesV2.enabledfeature keyActivityFeedV2insideSidebarContentinstead of the existingActivityFeedthreadedRepliesV2key separate from existingnewThreadedRepliesto avoid interferenceTest plan
ActivitySidebar.rtl.test.jscovering feature gate enabled, disabled, and unsetActivityFeedV2componentSummary by CodeRabbit
New Features
Tests