Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 21 additions & 6 deletions static/app/components/timeline/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export interface TimelineItemProps {
'data-index'?: number;
icon?: React.ReactNode;
isActive?: boolean;
marker?: React.ReactNode;
onClick?: React.MouseEventHandler<HTMLDivElement>;
onMouseEnter?: React.MouseEventHandler<HTMLDivElement>;
onMouseLeave?: React.MouseEventHandler<HTMLDivElement>;
Expand All @@ -33,6 +34,7 @@ function Item({
title,
children,
icon,
marker,
colorConfig,
timestamp,
isActive = false,
Expand All @@ -42,9 +44,11 @@ function Item({
}: TimelineItemProps) {
const theme = useTheme();
const config = colorConfig ?? makeDefaultColorConfig(theme);
const hasMarker = marker !== undefined;

return (
<Row ref={ref} {...props}>
<Row ref={ref} hasMarker={hasMarker} {...props}>
{hasMarker && <MarkerWrapper>{marker}</MarkerWrapper>}
{icon ? (
<IconWrapper
style={{
Expand All @@ -64,7 +68,7 @@ function Item({
</Flex>
{timestamp ?? <div />}
<Container justifySelf="center" width="0" height="100%" column="span 1" />
<Content>{children}</Content>
<Content hasMarker={hasMarker}>{children}</Content>
</Row>
);
}
Expand All @@ -77,12 +81,14 @@ function makeDefaultColorConfig(theme: Theme) {
};
}

const Row = styled('div')<{showLastLine?: boolean}>`
const Row = styled('div')<{hasMarker: boolean; showLastLine?: boolean}>`
position: relative;
color: ${p => p.theme.tokens.content.secondary};
display: grid;
align-items: start;
grid-template: auto auto / 22px 1fr auto;
grid-template-rows: auto auto;
grid-template-columns: ${p =>
p.hasMarker ? '22px 22px minmax(50px, 1fr) auto' : '22px minmax(50px, 1fr) auto'};
grid-column-gap: ${p => p.theme.space.md};
margin: ${p => p.theme.space.md} 0;
&:first-child {
Expand All @@ -96,6 +102,15 @@ const Row = styled('div')<{showLastLine?: boolean}>`
}
`;

const MarkerWrapper = styled('div')`
grid-column: span 1;
z-index: 10;
display: grid;
place-items: center;
min-width: 22px;
min-height: 22px;
`;

const IconWrapper = styled('div')`
grid-column: span 1;
border-radius: 100%;
Expand All @@ -115,9 +130,9 @@ const Title = styled('div')`
font-size: ${p => p.theme.font.size.md};
`;

const Content = styled('div')`
const Content = styled('div')<{hasMarker: boolean}>`
text-align: left;
grid-column: span 2;
grid-column: ${p => (p.hasMarker ? '3 / -1' : 'span 2')};
color: ${p => p.theme.tokens.content.secondary};
margin: ${p => p.theme.space['2xs']} 0 0;
font-size: ${p => p.theme.font.size.sm};
Expand Down
103 changes: 103 additions & 0 deletions static/app/views/issueDetails/activitySection/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,109 @@ describe('ActivitySection', () => {
expect(screen.queryByText('Test Note')).not.toBeInTheDocument();
});

it('renders activity actor markers', async () => {
const activityGroup = GroupFixture({
id: '1338',
activity: [
{
type: GroupActivityType.NOTE,
id: 'note-1',
data: {text: 'User note'},
dateCreated: '2020-01-01T00:00:00',
user,
},
{
type: GroupActivityType.SET_RESOLVED,
id: 'resolved-1',
data: {},
dateCreated: '2020-01-02T00:00:00',
user: null,
},
],
project,
});

render(<ActivitySection group={activityGroup} />, {
organization: OrganizationFixture({features: ['issue-activity-feed-v2']}),
});

expect(await screen.findByText('User note')).toBeInTheDocument();
expect(screen.getByTestId('user-activity-marker')).toBeInTheDocument();
expect(screen.getByTestId('sentry-activity-marker')).toBeInTheDocument();
});

it('does not render activity actor markers when the feature is disabled', async () => {
const activityGroup = GroupFixture({
id: '1338',
activity: [
{
type: GroupActivityType.NOTE,
id: 'note-1',
data: {text: 'User note'},
dateCreated: '2020-01-01T00:00:00',
user,
},
],
project,
});

render(<ActivitySection group={activityGroup} />);

expect(await screen.findByText('User note')).toBeInTheDocument();
expect(screen.queryByTestId('user-activity-marker')).not.toBeInTheDocument();
});

it('does not render user avatar as icon for notes in two-column layout', async () => {
const activityGroup = GroupFixture({
id: '1338',
activity: [
{
type: GroupActivityType.NOTE,
id: 'note-1',
data: {text: 'User note'},
dateCreated: '2020-01-01T00:00:00',
user,
},
],
project,
});

render(<ActivitySection group={activityGroup} />, {
organization: OrganizationFixture({features: ['issue-activity-feed-v2']}),
});

expect(await screen.findByText('User note')).toBeInTheDocument();
expect(screen.getByTestId('user-activity-marker')).toBeInTheDocument();
expect(screen.queryByTestId('letter_avatar-avatar')).not.toBeInTheDocument();
});

it('renders provider-specific icon for create issue in two-column layout', async () => {
const createIssueGroup = GroupFixture({
id: '1345',
activity: [
{
type: GroupActivityType.CREATE_ISSUE,
id: 'create-issue-1',
dateCreated: '2020-01-01T00:00:00',
data: {
provider: 'GitHub',
location: 'https://github.com/org/repo/issues/1',
title: 'Test Issue',
},
user,
},
],
project,
});

render(<ActivitySection group={createIssueGroup} />, {
organization: OrganizationFixture({features: ['issue-activity-feed-v2']}),
});

expect(await screen.findByText('Test Issue')).toBeInTheDocument();
expect(screen.queryByTestId('icon-add')).not.toBeInTheDocument();
});

it('renders note and allows for edit', async () => {
jest.spyOn(indicators, 'addSuccessMessage');

Expand Down
120 changes: 116 additions & 4 deletions static/app/views/issueDetails/activitySection/index.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import {Fragment, useCallback, useState} from 'react';
import {useTheme} from '@emotion/react';
import {useTheme, type Theme} from '@emotion/react';
import styled from '@emotion/styled';

import {SentryAppAvatar, UserAvatar} from '@sentry/scraps/avatar';
import {LinkButton} from '@sentry/scraps/button';
import {Flex, Grid} from '@sentry/scraps/layout';
import {Text} from '@sentry/scraps/text';
Expand Down Expand Up @@ -47,6 +48,74 @@ function getAuthorName(item: GroupActivity) {
return 'Sentry';
}

function getActivityMarker(item: GroupActivity, color: string) {
if (item.sentry_app) {
return (
<AvatarMarker color={color}>
<SentryAppAvatar
data-test-id="sentry-app-activity-marker"
sentryApp={item.sentry_app}
size={22}
/>
</AvatarMarker>
);
}
if (item.user) {
return (
<AvatarMarker color={color}>
<UserAvatar data-test-id="user-activity-marker" user={item.user} size={22} />
</AvatarMarker>
);
}
return <SentryMarker color={color} data-test-id="sentry-activity-marker" />;
}

function getActivityColorConfig(theme: Theme, type: GroupActivityType) {
const defaultConfig = {
title: theme.tokens.content.primary,
icon: theme.tokens.content.secondary,
iconBorder: theme.tokens.content.secondary,
};

switch (type) {
case GroupActivityType.SET_RESOLVED:
case GroupActivityType.SET_RESOLVED_BY_AGE:
case GroupActivityType.SET_RESOLVED_IN_RELEASE:
case GroupActivityType.SET_RESOLVED_IN_COMMIT:
case GroupActivityType.SET_RESOLVED_IN_PULL_REQUEST:
case GroupActivityType.MARK_REVIEWED:
case GroupActivityType.SEER_RCA_COMPLETED:
case GroupActivityType.SEER_SOLUTION_COMPLETED:
case GroupActivityType.SEER_CODING_COMPLETED:
case GroupActivityType.SEER_PR_CREATED:
return {
...defaultConfig,
icon: theme.tokens.graphics.success.vibrant,
iconBorder: theme.tokens.border.success.vibrant,
};
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.

SET_RESOLVED_IN_PULL_REQUEST missing from success color config

Medium Severity

The getActivityColorConfig switch statement includes all SET_RESOLVED_* activity types (SET_RESOLVED, SET_RESOLVED_BY_AGE, SET_RESOLVED_IN_RELEASE, SET_RESOLVED_IN_COMMIT) in the success/green color group — but omits GroupActivityType.SET_RESOLVED_IN_PULL_REQUEST. This means resolved-by-PR activities won't receive the green icon and border that all other resolved activity types get, resulting in an inconsistent visual treatment.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit fde9e8e. Configure here.

case GroupActivityType.SET_UNRESOLVED:
case GroupActivityType.SET_REGRESSION:
return {
...defaultConfig,
icon: theme.tokens.graphics.danger.vibrant,
iconBorder: theme.tokens.border.danger.vibrant,
};
case GroupActivityType.SET_ESCALATING:
case GroupActivityType.SET_PRIORITY:
return {
...defaultConfig,
icon: theme.tokens.graphics.warning.vibrant,
iconBorder: theme.tokens.border.warning.vibrant,
};
case GroupActivityType.SET_IGNORED:
return {
...defaultConfig,
};
default:
return defaultConfig;
}
}

function TimelineItem({
item,
handleDelete,
Expand All @@ -65,8 +134,11 @@ function TimelineItem({
teams: Team[];
}) {
const organization = useOrganization();
const theme = useTheme();
const [editing, setEditing] = useState(false);
const useTwoColumnLayout = organization.features.includes('issue-activity-feed-v2');
const authorName = getAuthorName(item);
const colorConfig = getActivityColorConfig(theme, item.type);
const {title, message} = getGroupActivityItem(
item,
organization,
Expand All @@ -77,8 +149,12 @@ function TimelineItem({
);

const iconMapping = groupActivityTypeIconMapping[item.type];
const Icon = iconMapping?.componentFunction
? iconMapping.componentFunction({
const componentFunction =
useTwoColumnLayout && item.type === GroupActivityType.NOTE
? undefined
: iconMapping?.componentFunction;
const Icon = componentFunction
? componentFunction({
data: item.data,
user: item.user,
sentry_app: item.sentry_app,
Expand All @@ -102,6 +178,8 @@ function TimelineItem({
</Flex>
}
timestamp={<Timestamp date={item.dateCreated} tooltipProps={{skipWrapper: true}} />}
marker={useTwoColumnLayout ? getActivityMarker(item, colorConfig.icon) : undefined}
colorConfig={useTwoColumnLayout ? colorConfig : undefined}
icon={
Icon && (
<Icon
Expand Down Expand Up @@ -302,6 +380,7 @@ export function ActivitySection({
item => !filterComments || item.type === GroupActivityType.NOTE
);
const inputVariant = variant === 'sidebar' ? 'compact' : 'full';
const useTwoColumnLayout = organization.features.includes('issue-activity-feed-v2');

const renderActivityItem = (item: GroupActivity) => (
<TimelineItem
Expand Down Expand Up @@ -379,6 +458,7 @@ export function ActivitySection({
{t('View %s more', filteredActivities.length - 3)}
</LinkButton>
}
marker={useTwoColumnLayout ? <span /> : undefined}
icon={<RotatedEllipsisIcon direction="up" />}
/>
</Fragment>
Expand All @@ -398,7 +478,6 @@ const TitleTooltip = styled(Tooltip)`

const ActivityTimelineItem = styled(Timeline.Item)`
align-items: center;
grid-template-columns: 22px minmax(50px, 1fr) auto;
`;

const Timestamp = styled(TimeSince)`
Expand All @@ -418,3 +497,36 @@ const NoteWrapper = styled('div')<{size: 'sm' | 'md'}>`
const ActivityInputFrame = styled('div')`
color: ${p => p.theme.tokens.content.primary};
`;

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.

Sidebar "View more" row misaligned with marker rows

Medium Severity

When the issue-activity-feed-v2 feature flag is enabled, activity items in the sidebar render with a 4-column grid (22px 22px minmax(50px,1fr) auto) via the marker prop, but the "View X more" ActivityTimelineItem doesn't pass a marker, so it gets a 3-column grid. This causes its icon and title to be horizontally misaligned with the activity rows above it — the icon sits where markers are, and the title text shifts left by one column.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit fde9e8e. Configure here.

const AvatarMarker = styled('span')<{color: string}>`
display: block;
position: relative;
border-radius: 100%;
line-height: 0;

&::after {
content: '';
position: absolute;
inset: 0;
border-radius: 100%;
box-shadow: inset 0 0 0 2px ${p => p.color};
pointer-events: none;
}
`;

const SentryMarker = styled('span')<{color: string}>`
width: 12px;
height: 12px;
border-radius: 100%;
background: ${p => p.theme.tokens.background.primary};
display: grid;
place-items: center;

&::after {
content: '';
width: 6px;
height: 6px;
border-radius: 100%;
background: ${p => p.color};
}
`;
Loading