Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,93 @@ describe('notification attachment', () => {
});
});

describe('content-arrival rows', () => {
const hoursAgo = (h: number) => new Date(Date.now() - h * 3_600_000);
const sourcePost: NotificationItemProps = {
icon: NotificationIconType.Bell,
type: NotificationType.SourcePostAdded,
title: '<p>New post in <b>The New Stack</b></p>',
referenceId: 'source-post',
targetUrl: 'post url',
createdAt: hoursAgo(3),
avatars: [sampleNotificationAvatars[0]],
attachments: [
{
title: "Anthropic's Claude now has a browser of its own",
image: 'cover',
type: NotificationAttachmentType.Post,
},
],
};

it('should lead with the post headline, not the announcing sentence', async () => {
renderComponent(<NotificationItem {...sourcePost} />);
const headline = await screen.findByText(
"Anthropic's Claude now has a browser of its own",
);
expect(headline).toHaveClass('font-bold');
expect(screen.queryAllByText(/New post in/)).toHaveLength(0);
await screen.findByText('source');
});

it('should credit the person and the squad when both are present', async () => {
renderComponent(
<NotificationItem
{...sourcePost}
type={NotificationType.SquadPostAdded}
avatars={sampleNotificationAvatars}
/>,
);
await screen.findByText('user in source');
});

it('should keep the actor-first layout when there is no post to promote', async () => {
renderComponent(<NotificationItem {...sourcePost} attachments={[]} />);
// The source sits in a <b>, so the sentence spans several text nodes.
const matches = await screen.findAllByText(/New post in/);
expect(matches.length).toBeGreaterThan(0);
});
});

describe('notification timestamp', () => {
const hoursAgo = (h: number) => new Date(Date.now() - h * 3_600_000);

it('should follow the last grey line rather than the title', async () => {
renderComponent(
<NotificationItem {...sampleNotification} createdAt={hoursAgo(5)} />,
);
const time = await screen.findByText('5h');
expect(time.closest('div')).toHaveTextContent(/Sample attachment\s*·\s*5h/);
});

it('should never sit inside a clamped line, so a long title cannot hide it', async () => {
renderComponent(
<NotificationItem
{...sampleNotification}
description={undefined}
createdAt={hoursAgo(5)}
/>,
);
const time = await screen.findByText('5h');
expect(time.closest('.line-clamp-1, .line-clamp-2')).toBeNull();
expect(time.closest('.truncate')).toBeNull();
});

it('should take a line of its own when the row has no grey text', async () => {
renderComponent(
<NotificationItem
{...sampleNotification}
description={undefined}
attachments={[]}
createdAt={hoursAgo(9)}
/>,
);
const time = await screen.findByText('9h');
expect(time.closest('div')).toHaveTextContent('9h');
expect(time.closest('div')).not.toHaveTextContent('·');
});
});

describe('notification avatars', () => {
it('should display the avatar of the source', async () => {
const [source] = sampleNotificationAvatars;
Expand Down
115 changes: 91 additions & 24 deletions packages/shared/src/components/notifications/NotificationItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,12 @@ import { useObjectPurify } from '../../hooks/useDomPurify';
import NotificationItemAvatar from './NotificationItemAvatar';
import { NotificationItemLead } from './NotificationItemLead';
import { NotificationCategoryBadge } from './NotificationCategoryBadge';
import { getNotificationLeadAvatar } from './leadAvatar';
import {
getNotificationAttribution,
getNotificationLeadAvatar,
} from './leadAvatar';
import {
contentArrivalNotificationTypes,
getNotificationCategory,
NotificationFilterCategory,
notificationMutingCopy,
Expand Down Expand Up @@ -295,6 +299,42 @@ function NotificationItem(props: NotificationItemProps): ReactElement | null {
attachmentTitleNorm !== titleNorm &&
attachmentTitleNorm !== descriptionNorm;

const isContentArrival =
contentArrivalNotificationTypes.has(type) && !!attachmentTitle;
const attribution = isContentArrival
? getNotificationAttribution(filteredAvatars)
: undefined;
const showSubtitle = isContentArrival
? showDescription
: showDescription || showAttachmentTitle;
const showAttachmentLine =
!isContentArrival && showDescription && showAttachmentTitle;

let timeAnchor: 'attribution' | 'attachment' | 'subtitle' | null = null;
if (attribution) {
timeAnchor = 'attribution';
} else if (showAttachmentLine) {
timeAnchor = 'attachment';
} else if (showSubtitle) {
timeAnchor = 'subtitle';
}

const timeNode = timeText ? (
<Tooltip content={fullDate}>
<time className="relative z-1 whitespace-nowrap">{timeText}</time>
</Tooltip>
) : null;
const timeTail = timeNode && (
<>
<span aria-hidden>·</span>
{timeNode}
</>
);
// A global `* { flex-shrink: 0 }` means truncating text must opt back in with
// `shrink`, or it keeps max-content width and overflows the row.
const greyLine =
'flex items-baseline gap-1.5 text-text-tertiary typo-footnote';

return (
<div
className={classNames(
Expand Down Expand Up @@ -337,37 +377,64 @@ function NotificationItem(props: NotificationItemProps): ReactElement | null {
)}
</div>

{/* Headline (actor name bold, the rest regular for a scannable
hierarchy) with the relative time flowing inline right after it — so a
row reads "what happened" first and the time is a quiet suffix, not a
right-aligned column. Then the comment, then the post's title. */}
<div className="flex min-w-0 flex-1 flex-col gap-1 text-left">
<div className="break-words font-normal text-text-primary typo-callout [&_b]:font-bold [&_p]:m-0 [&_p]:inline [&_strong]:font-bold">
<span dangerouslySetInnerHTML={{ __html: memoizedTitle }} />
{timeText && (
<Tooltip content={fullDate}>
<time className="relative z-1 ml-1.5 whitespace-nowrap text-text-tertiary typo-footnote">
· {timeText}
</time>
</Tooltip>
)}
</div>
{(showDescription || showAttachmentTitle) && (
<div className="multi-truncate line-clamp-2 break-words text-text-tertiary typo-subhead [&_p]:m-0 [&_p]:inline">
{showDescription ? (
<span dangerouslySetInnerHTML={{ __html: memoizedDescription }} />
{isContentArrival ? (
<div className="multi-truncate line-clamp-2 break-words font-bold text-text-primary typo-callout">
{attachmentTitle}
</div>
) : (
<div className="break-words font-normal text-text-primary typo-callout [&_b]:font-bold [&_p]:m-0 [&_p]:inline [&_strong]:font-bold">
<span dangerouslySetInnerHTML={{ __html: memoizedTitle }} />
</div>
)}
{showSubtitle && (
<>
{timeAnchor === 'subtitle' ? (
// `multi-truncate` is `display: -webkit-box`, which blockifies
// as a flex item and loses both its clamp and its width — hence
// the separate single-line branch whenever the time rides here.
<div className="flex items-baseline gap-1.5 text-text-tertiary typo-subhead">
<span className="min-w-0 shrink truncate [&_p]:m-0 [&_p]:inline">
{showDescription ? (
<span
dangerouslySetInnerHTML={{ __html: memoizedDescription }}
/>
) : (
showAttachmentTitle && attachmentTitle
)}
</span>
<span className="flex shrink-0 items-baseline gap-1.5 typo-footnote">
{timeTail}
</span>
</div>
) : (
showAttachmentTitle && <span>{attachmentTitle}</span>
<div className="multi-truncate line-clamp-2 break-words text-text-tertiary typo-subhead [&_p]:m-0 [&_p]:inline">
{showDescription ? (
<span
dangerouslySetInnerHTML={{ __html: memoizedDescription }}
/>
) : (
showAttachmentTitle && <span>{attachmentTitle}</span>
)}
</div>
)}
</div>
</>
)}
{/* When there's both a comment and a post, name the post on its own
line so it's clear which article it's about. */}
{showDescription && showAttachmentTitle && (
<div className="multi-truncate line-clamp-1 break-words text-text-tertiary typo-footnote">
{attachmentTitle}
{showAttachmentLine && (
<div className={greyLine}>
<span className="min-w-0 shrink truncate">{attachmentTitle}</span>
{timeAnchor === 'attachment' && timeTail}
</div>
)}
{attribution && (
<div className={greyLine}>
<span className="min-w-0 shrink truncate">{attribution}</span>
{timeAnchor === 'attribution' && timeTail}
</div>
)}
{!timeAnchor && timeNode && <div className={greyLine}>{timeNode}</div>}
{type === NotificationType.UserFollow && (
<span className="relative z-1">
<NotificationFollowUserButton {...props} />
Expand Down
15 changes: 15 additions & 0 deletions packages/shared/src/components/notifications/leadAvatar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,18 @@ export const getNotificationLeadAvatar = (
): NotificationAvatar | undefined =>
avatars.find((avatar) => avatar.type === NotificationAvatarType.User) ??
avatars[0];

export const getNotificationAttribution = (
avatars: NotificationAvatar[] = [],
): string | undefined => {
const source = avatars.find(
(avatar) => avatar.type === NotificationAvatarType.Source,
);
const user = avatars.find(
(avatar) => avatar.type === NotificationAvatarType.User,
);
if (source?.name && user?.name) {
return `${user.name} in ${source.name}`;
}
return source?.name ?? user?.name;
};
6 changes: 6 additions & 0 deletions packages/shared/src/components/notifications/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,12 @@ export const notificationTypeTheme: Partial<Record<NotificationType, string>> =
[NotificationType.UserFollow]: 'text-brand-default',
};

export const contentArrivalNotificationTypes = new Set<NotificationType>([
NotificationType.SourcePostAdded,
NotificationType.SquadPostAdded,
NotificationType.UserPostAdded,
]);

export const notificationTypeNotClickable: Partial<
Record<NotificationType, boolean>
> = {
Expand Down
Loading
Loading