Skip to content

feat(content-sidebar): focused state for threaded-replies in activity-feed-v2 #4650

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mrscobbler wants to merge 3 commits into from
Closed

feat(content-sidebar): focused state for threaded-replies in activity-feed-v2 #4650

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
54659e7
feat(content-sidebar): focused state for threaded-replies in activity…
mrscobbler Jun 24, 2026
1f47211
refactor(content-sidebar): address review feedback on threaded-replie…
mrscobbler Jun 29, 2026
6438ad2
Merge branch 'master' into feature/threaded-replies-focus
mrscobbler Jun 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,16 @@
&-mentionEmpty {
padding: var(--bp-space-030) var(--bp-space-040);
}

// Layout-transparent hook that paints focus chrome onto the vendor card (CSS-modules-hashed, so match a substring).
&-threadRow {
display: contents;

&.is-focused [class*='threadedAnnotations'] {
border-color: var(--bp-box-blue-80);
background-color: var(--bp-box-blue-opacity-04);
}
Comment on lines +51 to +54

@tjuanitas tjuanitas Jun 30, 2026

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.

I think these styles should live in the shared feature, not the UI Element. That way the styles are located in one codebase instead of spread across repos and then other consumers like notes won't need to duplicate this.

The other issue is that the threadedAnnotations wildcard is fragile since it relies on an internal classname that is not guaranteed by the component.

you could have ActivityFeed.List.ThreadedAnnotation receive an isFocused prop that forces the style on and then still set it with item.id === activeFeedEntryId

@mrscobbler mrscobbler Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We can't have an actual focus state on the threaded annotations component itself due to the internal components also requiring focus state (@jackiejou keep me honest here). If we added an isFocused state it could be misleading? Open to moving to this strategy though if we're ok with this treatment not actually being used for accessibility related focus state-like behavior.

@mrscobbler mrscobbler Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Did a little more analysis:

The threaded annotations component already owns real focus semantics (:focus-within, root outline). This styling in this PR is a passive selected highlight, not focus — isFocused would collide and could get wired to outlines/aria later.

Proposal: add isActive (or isSelected) to the shared component, docs as selection-only (border + tint, no outline), scoped :not(.annotations) so the inline drawing/region popup is untouched.

The shared component is used across two surfaces -- the sidebar + annotations in a note/document/video/image -- so we'll have to get specific with the css on when the active state is activated vs not.

@tjuanitas thoughts ?

@tjuanitas tjuanitas Jul 1, 2026

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.

yeah an isSelected or isHighlighted prop in ActivityFeed.List.ThreadedAnnotation could work

something like this:

// ActivityFeedV2.tsx
filteredItems.map(item => (
  <FeedItemRow
    key={item.id}
    isSelected={activeFeedEntryId === item.id} // calculate the condition here instead of in FeedItemRow
    ...
  />

// FeedItemRow.tsx
<ActivityFeed.List.ThreadedAnnotation
  isSelected={isSelected} // passthrough prop
  ...
/>

// Handle the styles in ThreadedAnnotationsV2

@mrscobbler mrscobbler Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok -- we're taking that approach! Will close out this PR.

}
}

// Containing block for the absolutely positioned .bcs-NewActivityFeed child,
Expand Down
1 change: 1 addition & 0 deletions src/elements/content-sidebar/activity-feed-v2/ActivityFeedV2.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,7 @@ const ActivityFeedV2 = ({
{filteredItems.map(item => (
<FeedItemRow
key={item.id}
activeFeedEntryId={activeFeedEntryId}
currentUserId={currentUserId}
fps={fps}
isDisabled={isDisabled}
Expand Down
101 changes: 55 additions & 46 deletions src/elements/content-sidebar/activity-feed-v2/FeedItemRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

import * as React from 'react';
import classNames from 'classnames';
import noop from 'lodash/noop';

import { ActivityFeed } from '@box/activity-feed';
Expand All @@ -30,6 +31,7 @@ import {
} from '../../../constants';

type FeedItemRowProps = {
activeFeedEntryId?: string;
currentUserId?: string;
fps: number;
isDisabled: boolean;
Expand Down Expand Up @@ -81,6 +83,7 @@ const buildReplyPost =
};

const FeedItemRow = ({
activeFeedEntryId,
currentUserId,
fps,
isDisabled,
Expand All @@ -104,6 +107,10 @@ const FeedItemRow = ({
timeFormat,
userSelectorProps,
}: FeedItemRowProps) => {
const threadRowClassName = classNames('bcs-NewActivityFeed-threadRow', {
'is-focused': item.id === activeFeedEntryId,
Comment thread
mrscobbler marked this conversation as resolved.
});

switch (item.type) {
case 'comment': {
const { permissions } = item;
Expand Down Expand Up @@ -145,27 +152,28 @@ const FeedItemRow = ({
? { ...item.annotationTarget, timestamp: formatByTimeFormat(timestampMs, timeFormat, fps) }
: item.annotationTarget;
return (
<ActivityFeed.List.ThreadedAnnotation
key={item.id}
Comment thread
mickr marked this conversation as resolved.
annotationTarget={commentAnnotationTarget}
isAnnotations={false}
isEditDisabled={isDisabled || item.isResolved}
isResolved={item.isResolved}
messages={item.messages}
onAnnotationBadgeClick={handleBadgeClick}
onAvatarClick={noop}
onCopyLink={onCommentCopyLink ? (id: string) => onCommentCopyLink({ id }) : undefined}
onDelete={handleDelete}
onEdit={handleEdit}
onEditError={logEditError}
onPost={buildReplyPost(item.id, FEED_ITEM_TYPE_COMMENT, isDisabled, onReplyCreate)}
onResolve={handleStatusChange('resolved')}
onThreadDelete={() => handleDelete(item.id)}
onUnresolve={handleStatusChange('open')}
resolvedAt={item.resolvedAt}
resolvedBy={item.resolvedBy}
userSelectorProps={userSelectorProps}
/>
<div className={threadRowClassName}>
<ActivityFeed.List.ThreadedAnnotation
annotationTarget={commentAnnotationTarget}
isAnnotations={false}
isEditDisabled={isDisabled || item.isResolved}
isResolved={item.isResolved}
messages={item.messages}
onAnnotationBadgeClick={handleBadgeClick}
onAvatarClick={noop}
onCopyLink={onCommentCopyLink ? (id: string) => onCommentCopyLink({ id }) : undefined}
onDelete={handleDelete}
onEdit={handleEdit}
onEditError={logEditError}
onPost={buildReplyPost(item.id, FEED_ITEM_TYPE_COMMENT, isDisabled, onReplyCreate)}
onResolve={handleStatusChange('resolved')}
onThreadDelete={() => handleDelete(item.id)}
onUnresolve={handleStatusChange('open')}
resolvedAt={item.resolvedAt}
resolvedBy={item.resolvedBy}
userSelectorProps={userSelectorProps}
/>
</div>
);
}

Expand Down Expand Up @@ -209,31 +217,32 @@ const FeedItemRow = ({
}
: badgeTarget;
return (
<ActivityFeed.List.ThreadedAnnotation
key={item.id}
annotationTarget={annotationBadgeTarget}
isAnnotations={false}
isEditDisabled={isDisabled || item.isResolved}
isResolved={item.isResolved}
messages={item.messages}
onAnnotationBadgeClick={() => onAnnotationSelect?.(item.annotation)}
onAvatarClick={noop}
onCopyLink={
onAnnotationCopyLink && fileVersionId
? () => onAnnotationCopyLink({ annotationId: item.id, fileVersionId })
: undefined
}
onDelete={handleDelete}
onEdit={handleEdit}
onEditError={logEditError}
onPost={buildReplyPost(item.id, FEED_ITEM_TYPE_ANNOTATION, isDisabled, onReplyCreate)}
onResolve={handleStatusChange('resolved')}
onThreadDelete={() => handleDelete(item.id)}
onUnresolve={handleStatusChange('open')}
resolvedAt={item.resolvedAt}
resolvedBy={item.resolvedBy}
userSelectorProps={userSelectorProps}
/>
<div className={threadRowClassName}>
<ActivityFeed.List.ThreadedAnnotation
annotationTarget={annotationBadgeTarget}
isAnnotations={false}
isEditDisabled={isDisabled || item.isResolved}
isResolved={item.isResolved}
messages={item.messages}
onAnnotationBadgeClick={() => onAnnotationSelect?.(item.annotation)}
onAvatarClick={noop}
onCopyLink={
onAnnotationCopyLink && fileVersionId
? () => onAnnotationCopyLink({ annotationId: item.id, fileVersionId })
: undefined
}
onDelete={handleDelete}
onEdit={handleEdit}
onEditError={logEditError}
onPost={buildReplyPost(item.id, FEED_ITEM_TYPE_ANNOTATION, isDisabled, onReplyCreate)}
onResolve={handleStatusChange('resolved')}
onThreadDelete={() => handleDelete(item.id)}
onUnresolve={handleStatusChange('open')}
resolvedAt={item.resolvedAt}
resolvedBy={item.resolvedBy}
userSelectorProps={userSelectorProps}
/>
</div>
);
}

Expand Down
36 changes: 36 additions & 0 deletions src/elements/content-sidebar/activity-feed-v2/__tests__/FeedItemRow.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1033,4 +1033,40 @@ describe('elements/content-sidebar/activity-feed-v2/FeedItemRow', () => {
});
});
});

describe('focus state', () => {
const getThreadRow = () => screen.getByRole('article', { name: 'threaded annotation' }).parentElement;

test('should wrap a comment thread in a threadRow div without breaking rendering', () => {
render(<FeedItemRow {...defaultProps} item={mockComment} />);

expect(screen.getByRole('article', { name: 'threaded annotation' })).toBeVisible();
expect(getThreadRow()).toHaveClass('bcs-NewActivityFeed-threadRow');
});

test('should mark the comment row focused when activeFeedEntryId matches the item id', () => {
render(<FeedItemRow {...defaultProps} activeFeedEntryId="comment-1" item={mockComment} />);
expect(getThreadRow()).toHaveClass('is-focused');
});

test('should not mark the comment row focused when activeFeedEntryId does not match', () => {
render(<FeedItemRow {...defaultProps} activeFeedEntryId="comment-2" item={mockComment} />);
expect(getThreadRow()).not.toHaveClass('is-focused');
});

test('should not mark the comment row focused when activeFeedEntryId is undefined', () => {
render(<FeedItemRow {...defaultProps} item={mockComment} />);
expect(getThreadRow()).not.toHaveClass('is-focused');
});

test('should mark the annotation row focused when activeFeedEntryId matches the item id', () => {
render(<FeedItemRow {...defaultProps} activeFeedEntryId="annotation-1" item={mockAnnotation} />);
expect(getThreadRow()).toHaveClass('bcs-NewActivityFeed-threadRow', 'is-focused');
});

test('should not mark the annotation row focused when activeFeedEntryId does not match', () => {
render(<FeedItemRow {...defaultProps} activeFeedEntryId="annotation-2" item={mockAnnotation} />);
expect(getThreadRow()).not.toHaveClass('is-focused');
});
});
});
Loading