Skip to content

feat: name both people when two follow you or star your commit #1332

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

Merged
xHeaven merged 2 commits into from
Oct 31, 2026
+82 −23
Merged

feat: name both people when two follow you or star your commit #1332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
42f7363
feat: name both people when two follow you or star your commit
richardkeep Oct 30, 2026
9a6a6bd
Merge branch 'production' into feat/name-both-notification-actors
xHeaven Oct 31, 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
38 changes: 30 additions & 8 deletions resources/js/pages/notification/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,9 @@ export function NotificationContent({
notification.type === 'followed' || notification.type === 'starred'
? actorsCount - 1
: 0;
// Two people are both named; "and N others" is kept for three or more.
const secondActor =
otherActors === 1 && actors[1]?.id !== actor.id ? actors[1] : undefined;
const grouped =
summary &&
(actorsCount > 1 ||
Expand Down Expand Up @@ -1140,14 +1143,33 @@ export function NotificationContent({
>
{actor.name}
</Link>{' '}
{otherActors > 0 && (
<span
title={exactCountTitle(
otherActors,
)}
>
{`and ${formatCount(otherActors)} ${otherActors === 1 ? 'other' : 'others'} `}
</span>
{secondActor ? (
<>
and{' '}
<Link
href={show(
secondActor,
)}
className="relative z-10 inline-block max-w-full truncate align-bottom font-semibold hover:underline"
title={
secondActor.name
}
>
{
secondActor.name
}
</Link>{' '}
</>
) : (
otherActors > 0 && (
<span
title={exactCountTitle(
otherActors,
)}
>
{`and ${formatCount(otherActors)} ${otherActors === 1 ? 'other' : 'others'} `}
</span>
)
)}
</>
) : (
Expand Down
67 changes: 52 additions & 15 deletions tests/Unit/NotificationIndex.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ const previewedNames = (body: string): number =>

const other = { ...actor, id: 'other', name: 'Other', handle: 'other' };

test('separates last-visit activity without a border under the preceding notification', async () => {
test('separates last-visit activity without a border under the preceding notification', async () => {
const body = await render([
{ ...notification, read_at: '2026-10-16T12:00:00.000000Z' },
{
Expand All @@ -136,7 +136,7 @@ const other = { ...actor, id: 'other', name: 'Other', handle: 'other' };
assert.match(after, /class="min-w-0 flex-1 py-3 pr-4 border-b"/);
});

test('colors each notification rail by actor and ends it in a dot', async () => {
test('colors each notification rail by actor and ends it in a dot', async () => {
const nextActor = { ...other, lane: 5, is_followed: false };
const body = await render([
notification,
Expand All @@ -156,7 +156,7 @@ const other = { ...actor, id: 'other', name: 'Other', handle: 'other' };
assert.match(body, /border-\(--lane\)/);
});

test('renders a code block highlighted in a notification', async () => {
test('renders a code block highlighted in a notification', async () => {
const body = await render([notification]);

assert.match(body, /<pre[^>]*><span class="hl-keyword">return<\/span>/);
Expand All @@ -167,7 +167,7 @@ const other = { ...actor, id: 'other', name: 'Other', handle: 'other' };
});

for (const isFollowing of [false, true]) {
test(`keeps ${isFollowing ? 'unfollow' : 'follow-back'} separate from highlighted notification links`, async () => {
test(`keeps ${isFollowing ? 'unfollow' : 'follow-back'} separate from highlighted notification links`, async () => {
const followed = {
...notification,
id: 'follow-notification',
Expand Down Expand Up @@ -195,7 +195,7 @@ for (const isFollowing of [false, true]) {
});
}

test('shows the image of a starred image-only commit beside who starred it', async () => {
test('shows the image of a starred image-only commit beside who starred it', async () => {
const starred = {
...notification,
type: 'starred',
Expand Down Expand Up @@ -225,7 +225,44 @@ for (const isFollowing of [false, true]) {
assert.doesNotMatch(body, /<div[^>]*>(?:<div[^>]*>)*(?:<\/div>)+<\/a>/);
});

test('links the actor to their profile apart from the commit link', async () => {
for (const type of ['followed', 'starred']) {
test(`names both people of a ${type} group of two`, async () => {
const body = (
await render([
{
...notification,
type,
actors: [actor, other],
actors_count: 2,
} as NotificationGroup,
])
).replaceAll('<!-- -->', '');

assert.match(
body,
/<\/a> and <a\b[^>]*href="\/@other"[^>]*>Other<\/a> /,
);
assert.doesNotMatch(body, /1 other/);
});

test(`counts the others of a ${type} group of three`, async () => {
const body = (
await render([
{
...notification,
type,
actors: [actor, other, { ...other, id: 'third' }],
actors_count: 3,
} as NotificationGroup,
])
).replaceAll('<!-- -->', '');

assert.match(body, /and 2 others /);
assert.doesNotMatch(body, /href="\/@other"[^>]*>Other<\/a>/);
});
}

test('links the actor to their profile apart from the commit link', async () => {
const starred = { ...notification, type: 'starred' } as NotificationGroup;
const body = await render([starred]);
const links = [...body.matchAll(/<a\b[^>]*>/g)].map((match) => match[0]);
Expand All @@ -241,14 +278,14 @@ for (const isFollowing of [false, true]) {
assert.match(body, /starred your commit<\/a>/);
});

test('previews the person of a single reply from the avatar and the name', async () => {
test('previews the person of a single reply from the avatar and the name', async () => {
const body = await render([notification]);

assert.equal(previewedAvatars(body), 1);
assert.equal(previewedNames(body), 1);
});

test('previews the person of a single star from the avatar and the name', async () => {
test('previews the person of a single star from the avatar and the name', async () => {
const body = await render([
{ ...notification, type: 'starred' } as NotificationGroup,
]);
Expand All @@ -258,7 +295,7 @@ for (const isFollowing of [false, true]) {
});

for (const type of ['replied', 'starred']) {
test(`leaves the header of a group of ${type} notifications without previews`, async () => {
test(`leaves the header of a group of ${type} notifications without previews`, async () => {
const body = await render([
{
...notification,
Expand All @@ -275,7 +312,7 @@ for (const type of ['replied', 'starred']) {
}

for (const type of ['replied', 'starred']) {
test(`previews the person of an expanded ${type} notification`, async () => {
test(`previews the person of an expanded ${type} notification`, async () => {
const body = await renderItem({
...notification,
type,
Expand All @@ -292,7 +329,7 @@ const avatarLink =
const nestedLink = /<a\b(?:(?!<\/a>).)*<a\b/s;

for (const type of ['replied', 'starred']) {
test(`links the avatar of a single ${type} notification to the profile`, async () => {
test(`links the avatar of a single ${type} notification to the profile`, async () => {
const body = await render([
{ ...notification, type } as NotificationGroup,
]);
Expand All @@ -302,7 +339,7 @@ for (const type of ['replied', 'starred']) {
});
}

test('leaves the avatar of a single follow unlinked, as the row already opens the profile', async () => {
test('leaves the avatar of a single follow unlinked, as the row already opens the profile', async () => {
const body = await render([
{
...notification,
Expand All @@ -317,7 +354,7 @@ for (const type of ['replied', 'starred']) {
assert.doesNotMatch(body, avatarLink);
});

test('links an expanded reply to its author and to the reply', async () => {
test('links an expanded reply to its author and to the reply', async () => {
const body = await renderItem(notification);

assert.match(body, avatarLink);
Expand All @@ -336,7 +373,7 @@ for (const type of ['replied', 'starred']) {
assert.doesNotMatch(body, nestedLink);
});

test('keeps an expanded star as one link to the profile', async () => {
test('keeps an expanded star as one link to the profile', async () => {
const body = await renderItem({
...notification,
type: 'starred',
Expand All @@ -346,7 +383,7 @@ for (const type of ['replied', 'starred']) {
assert.match(body, /<a [^>]*href="\/@mnapoli"/);
});

test('says which level and rank a level-up reached, linking to the profile', async () => {
test('says which level and rank a level-up reached, linking to the profile', async () => {
const leveledUp = {
...notification,
id: 'level',
Expand Down
Loading