From 77f633ffb46bd58d0d1e8ce4fdaffda039402ae8 Mon Sep 17 00:00:00 2001 From: udaykiranchenna2 Date: Sun, 18 Oct 2026 16:59:54 +0530 Subject: [PATCH 1/2] fix: show an image count for image-only commits in notifications Co-Authored-By: Claude Opus 5.5 --- DESIGN.md | 2 +- .../NotificationGroupController.php | 2 +- app/Queries/NotificationGroupsQuery.php | 9 ++-- resources/js/pages/notification/index.tsx | 50 +++++++++++++++++-- resources/js/types/notification.ts | 8 ++- .../NotificationControllerTest.php | 20 ++++++++ tests/Unit/NotificationInbox.test.tsx | 1 + tests/Unit/NotificationIndex.test.tsx | 38 +++++++++++++- 8 files changed, 116 insertions(+), 14 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index c8af7aeb4..7652cb873 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -317,7 +317,7 @@ Under the graph, a mono 13px strip prints the streaks, `longest streak 12 days Hovering an avatar, name, or handle (mouse only, 450ms delay, 200ms grace to move into it) opens a 304px Popover preview below the trigger, flipping above near the viewport bottom: a 56px avatar ringed in the user's lane, Follow / Following (Unfollow on hover) top right, name, mono handle with lane dot and a "Follows you" chip, a 3-line bio, and mono counts linking to following and followers. Follow toggles optimistically and rolls back on error. Closes on scroll, Escape, or leaving. ### Notifications -The inbox is a log too: every row hangs on a Rail Gray rail whose node is a 36px hollow circle holding the type icon (person-plus, reply, quote, star, at, check, cross). A sticky header carries the "Notifications" title over a row of page tabs (All, Replies, Quotes, Stars, Mentions, Follows, Moderation) with the sliding Commit Blue underline; on phones the row scrolls sideways with the current filter kept in view. Day markers ("Today", "Yesterday", "Wed, Oct 14") split the rows the way they split the log. A row opens with a line of stacked 32px avatars (up to three, then a mono `+N` disc) and a mono relative time pushed right, then the sentence: the actor's name (600), their mono handle when they act alone, otherwise "and N others", and the action in plain prose. Words someone else wrote (a reply, quote, or mention) follow at 15px like a commit body; your own commit (a star, an approval, a rejection) sits underneath as a hairline-bordered, muted block clamped to three lines. Moderation rows replace the avatars with a mono `approved` or `rejected` chip, and a rejection prints a mono `reason:` key before the moderator's words. Groups expand in place from a Commit Blue toggle ("5 stars" / "Hide stars"), listing one compact row per event behind a 1px Rail Gray guide line, each follower with their own Follow button. Rows the inbox marked read on this visit ring their node in Commit Blue and sit above a `HEAD@{last visit}` marker. While you read, polling adds new rows above without moving the ones on screen, and a Commit Blue pill ("↑ 1 new notification") takes you to them. Empty filters speak in git's voice (`$ cat /dev/notifications | grep stars`) with a link back to all notifications; the end of the list is a hollow root node: "That's the oldest notification. There's nothing before it." +The inbox is a log too: every row hangs on a Rail Gray rail whose node is a 36px hollow circle holding the type icon (person-plus, reply, quote, star, at, check, cross). A sticky header carries the "Notifications" title over a row of page tabs (All, Replies, Quotes, Stars, Mentions, Follows, Moderation) with the sliding Commit Blue underline; on phones the row scrolls sideways with the current filter kept in view. Day markers ("Today", "Yesterday", "Wed, Oct 14") split the rows the way they split the log. A row opens with a line of stacked 32px avatars (up to three, then a mono `+N` disc) and a mono relative time pushed right, then the sentence: the actor's name (600), their mono handle when they act alone, otherwise "and N others", and the action in plain prose. Words someone else wrote (a reply, quote, or mention) follow at 15px like a commit body; your own commit (a star, an approval, a rejection) sits underneath as a hairline-bordered, muted block clamped to three lines. A commit of images alone has no words to show, so an image icon and "Image" or "N images" stand in for them. Moderation rows replace the avatars with a mono `approved` or `rejected` chip, and a rejection prints a mono `reason:` key before the moderator's words. Groups expand in place from a Commit Blue toggle ("5 stars" / "Hide stars"), listing one compact row per event behind a 1px Rail Gray guide line, each follower with their own Follow button. Rows the inbox marked read on this visit ring their node in Commit Blue and sit above a `HEAD@{last visit}` marker. While you read, polling adds new rows above without moving the ones on screen, and a Commit Blue pill ("↑ 1 new notification") takes you to them. Empty filters speak in git's voice (`$ cat /dev/notifications | grep stars`) with a link back to all notifications; the end of the list is a hollow root node: "That's the oldest notification. There's nothing before it." ### Reply Threads Under a commit's body, a quiet action row: "Reply" (muted, opens an inline bordered composer with its own counter, Cancel, and Reply; ⌘/Ctrl+Enter submits, Escape cancels) "N replies" in Commit Blue, which expands the thread in place, and a muted copy-link icon. Replies are compact rows: 24px (phone) / 28px avatar, name, mono handle (hidden on phones), time, status badge, then the body. Children render below their parent at the parent's left edge, not inside its text column, behind a 1px Rail Gray guide line; each level costs 19px (phone) / 27px. Indentation stops at three levels: deeper replies stay at that depth and carry a mono "↳ replying to @handle" label. Bodies wrap anywhere (`overflow-wrap: anywhere`) so long URLs and tokens never overflow. diff --git a/app/Http/Controllers/NotificationGroupController.php b/app/Http/Controllers/NotificationGroupController.php index 6f55d5a3b..d6bc4d923 100644 --- a/app/Http/Controllers/NotificationGroupController.php +++ b/app/Http/Controllers/NotificationGroupController.php @@ -16,6 +16,6 @@ public function show(ShowNotificationGroupRequest $request, #[CurrentUser] User $user, Notification $notification, NotificationGroupsQuery $query): JsonResponse { return response()->json($query->members($user, $notification, $request->type()) - ->with('commit')->latest()->latest('id')->cursorPaginate(20)->withQueryString()); + ->latest()->latest('id')->cursorPaginate(20)->withQueryString()); } } diff --git a/app/Queries/NotificationGroupsQuery.php b/app/Queries/NotificationGroupsQuery.php index cde8aad67..06e69bd5a 100644 --- a/app/Queries/NotificationGroupsQuery.php +++ b/app/Queries/NotificationGroupsQuery.php @@ -35,7 +35,7 @@ public function paginate(User $user, string $type = 'all', ?CarbonImmutable $thr ->with([ 'actor' => fn (BelongsTo $query): BelongsTo => $query ->withExists(['followers as is_followed' => fn (Builder $query): Builder => $query->whereKey($user->id)]), - 'commit', + 'commit' => fn (BelongsTo $query): BelongsTo => $query->withCount('images'), ]) ->latest()->latest('id'); $perPage = $through instanceof CarbonImmutable @@ -65,8 +65,11 @@ public function unreadCount(User $user): int public function members(User $user, Notification $notification, string $type = 'all'): NotificationBuilder { return $this->group($this->visible($user), $type, $notification) - ->with(['actor' => fn (BelongsTo $query): BelongsTo => $query - ->withExists(['followers as is_followed' => fn (Builder $query): Builder => $query->whereKey($user->id)])]); + ->with([ + 'actor' => fn (BelongsTo $query): BelongsTo => $query + ->withExists(['followers as is_followed' => fn (Builder $query): Builder => $query->whereKey($user->id)]), + 'commit' => fn (BelongsTo $query): BelongsTo => $query->withCount('images'), + ]); } public function events(User $user, string $type = 'all'): NotificationBuilder diff --git a/resources/js/pages/notification/index.tsx b/resources/js/pages/notification/index.tsx index 87d6f6c21..b1a78b247 100644 --- a/resources/js/pages/notification/index.tsx +++ b/resources/js/pages/notification/index.tsx @@ -5,6 +5,7 @@ import { AtSign, Check, ChevronDown, + ImageIcon, MessageSquareReply, Quote, Star, @@ -40,7 +41,12 @@ import { localDayKey, } from '@/lib/utils'; import { show } from '@/routes/user'; -import type { CursorPaginated, Notification, NotificationGroup } from '@/types'; +import type { + CursorPaginated, + Notification, + NotificationCommit, + NotificationGroup, +} from '@/types'; type Filters = { type: @@ -757,14 +763,14 @@ function NotificationContent({ const excerpt = notification.type === 'followed' ? null : isOwnCommit ? (
-
) : ( - ); @@ -925,4 +931,38 @@ function NotificationContent({ ); } +/** + * A commit of images alone has no body to excerpt, so it says how many images it holds instead. + */ +function NotificationCommitBody({ + commit, + className, +}: { + commit: NotificationCommit; + className: string; +}) { + if (commit.body_segments.length > 0) { + return ( + + ); + } + + return ( +

+ + {commit.images_count === 1 + ? 'Image' + : `${commit.images_count} images`} +

+ ); +} + Index.layout = AppLayout; diff --git a/resources/js/types/notification.ts b/resources/js/types/notification.ts index 60fe44839..29e0f39d9 100644 --- a/resources/js/types/notification.ts +++ b/resources/js/types/notification.ts @@ -12,6 +12,10 @@ type BaseNotification = { reason: string | null; }; +export type NotificationCommit = Omit & { + images_count: number; +}; + export type NotificationGroup = Notification & { group_id: string; event_count: number; @@ -31,11 +35,11 @@ export type Notification = type: 'replied' | 'starred'; commit_id: string; subject_id: string; - commit: Omit; + commit: NotificationCommit; }) | (BaseNotification & { type: 'quoted' | 'mentioned' | 'approved' | 'rejected'; commit_id: string; subject_id: null; - commit: Omit; + commit: NotificationCommit; }); diff --git a/tests/Feature/Controllers/NotificationControllerTest.php b/tests/Feature/Controllers/NotificationControllerTest.php index 6679d6e61..86e0fd379 100644 --- a/tests/Feature/Controllers/NotificationControllerTest.php +++ b/tests/Feature/Controllers/NotificationControllerTest.php @@ -5,6 +5,7 @@ use App\Actions\CreateCommitStar; use App\Actions\DeleteUserFollow; use App\Models\Commit; +use App\Models\CommitImage; use App\Models\Notification; use App\Models\User; @@ -128,6 +129,25 @@ expect($user->notifications()->unread()->exists())->toBeFalse(); }); +it('counts the images of each commit so one without a body still has something to show', function (): void { + $user = User::factory()->create(); + $commit = Commit::factory()->for($user)->create(['body' => '']); + CommitImage::factory()->count(2)->for($commit)->create(); + $starred = Notification::factory()->for($user, 'recipient')->starred($commit)->create(); + + $this->actingAs($user)->get(route('notification.index')) + ->assertOk() + ->assertInertia(fn ($page) => $page + ->where('notifications.data.0.commit.id', $commit->id) + ->where('notifications.data.0.commit.body_segments', []) + ->where('notifications.data.0.commit.images_count', 2)); + + $this->getJson(route('notification-group.show', $starred)) + ->assertOk() + ->assertJsonPath('data.0.commit.id', $commit->id) + ->assertJsonPath('data.0.commit.images_count', 2); +}); + it('says whether the user follows each new follower back', function (): void { $user = User::factory()->create(); $followedBack = Notification::factory()->for($user, 'recipient')->create(['created_at' => now()->subMinute()]); diff --git a/tests/Unit/NotificationInbox.test.tsx b/tests/Unit/NotificationInbox.test.tsx index d667290d7..7221fd228 100644 --- a/tests/Unit/NotificationInbox.test.tsx +++ b/tests/Unit/NotificationInbox.test.tsx @@ -54,6 +54,7 @@ const quote: NotificationGroup = { stars_count: 0, starred: false, quoted_commit_id: 'quoted-commit', + images_count: 0, }, }; diff --git a/tests/Unit/NotificationIndex.test.tsx b/tests/Unit/NotificationIndex.test.tsx index 49668c7a5..3a24c6b85 100644 --- a/tests/Unit/NotificationIndex.test.tsx +++ b/tests/Unit/NotificationIndex.test.tsx @@ -5,7 +5,11 @@ import { test } from 'node:test'; import { createInertiaApp } from '@inertiajs/react'; import { renderToString } from 'react-dom/server'; import Index from '../../resources/js/pages/notification/index'; -import type { Commit, NotificationGroup, User } from '../../resources/js/types'; +import type { + NotificationCommit, + NotificationGroup, + User, +} from '../../resources/js/types'; const actor = { id: 'actor', @@ -38,7 +42,8 @@ const notification = { html: 'return round($score, 6);', }, ], - } as Omit, + images_count: 0, + } as NotificationCommit, } as NotificationGroup; async function render(notifications: NotificationGroup[]): Promise { @@ -90,6 +95,35 @@ void test('renders a code block highlighted in a notification', async () => { assert.match(body, /]*tabindex="-1"/i); }); +for (const [imagesCount, label] of [ + [1, 'Image'], + [3, '3 images'], +] as const) { + void test(`says "${label}" for a starred commit of images alone instead of an empty excerpt`, async () => { + const starred = { + ...notification, + type: 'starred', + commit_id: 'images', + subject_id: 'images', + commit: { + id: 'images', + body: '', + body_segments: [], + images_count: imagesCount, + } as NotificationCommit, + } as NotificationGroup; + const body = await render([starred]); + + assert.match( + body, + new RegExp( + `class="mt-2 rounded-lg border[^"]*">

]*>(?:(?!).)*${label}

`, + 's', + ), + ); + }); +} + for (const isFollowing of [false, true]) { void test(`keeps ${isFollowing ? 'unfollow' : 'follow-back'} separate from highlighted notification links`, async () => { const followed = { From f34fd58a5220ed0236fbc5acb07792229e193bed Mon Sep 17 00:00:00 2001 From: udaykiranchenna2 Date: Mon, 19 Oct 2026 00:20:30 +0530 Subject: [PATCH 2/2] test: run the notification page tests in CI Moves NotificationIndex and NotificationInbox to vite-plus/test and lists them in tests/draft-images.config.ts, which SeriesStash.test.ts runs in CI. The image-count test now checks the label text inside the bordered block instead of its exact markup. Co-Authored-By: Claude Opus 5.5 --- tests/Unit/NotificationInbox.test.tsx | 22 +++++++++------------- tests/Unit/NotificationIndex.test.tsx | 19 ++++++++----------- tests/draft-images.config.ts | 2 ++ 3 files changed, 19 insertions(+), 24 deletions(-) diff --git a/tests/Unit/NotificationInbox.test.tsx b/tests/Unit/NotificationInbox.test.tsx index d671d18e6..4f10bf230 100644 --- a/tests/Unit/NotificationInbox.test.tsx +++ b/tests/Unit/NotificationInbox.test.tsx @@ -1,9 +1,9 @@ /// import assert from 'node:assert/strict'; -import { test } from 'node:test'; import { createInertiaApp } from '@inertiajs/react'; import { renderToString } from 'react-dom/server'; +import { test } from 'vite-plus/test'; import Index from '../../resources/js/pages/notification/index'; import type { NotificationGroup, User } from '../../resources/js/types'; @@ -96,23 +96,20 @@ async function renderInbox(notifications: NotificationGroup[], type = 'all') { return result.body.replaceAll('', ''); } -void test('renders quote notifications with their label and quoting commit destination', async () => { +test('renders quote notifications with their label and quoting commit destination', async () => { const html = await renderInbox([quote]); assert.match(html, /quoted your commit/); assert.match(html, /href="\/@quoter\/commits\/quote-commit"/); assert.match(html, /My quote commentary/); - assert.match( - html, - /]*aria-pressed="true"[^>]*>Quotes<\/button>/, - ); + assert.match(html, /]*aria-pressed="true"[^>]*>Quotes<\/button>/); }); -void test('includes quotes in the unfiltered empty inbox message', async () => { +test('includes quotes in the unfiltered empty inbox message', async () => { assert.match(await renderInbox([]), /replies, quotes, mentions/); }); -void test('renders a follow-back control outside the follower profile link', async () => { +test('renders a follow-back control outside the follower profile link', async () => { const follow: NotificationGroup = { ...quote, type: 'followed', @@ -136,7 +133,7 @@ void test('renders a follow-back control outside the follower profile link', asy ); }); -void test('renders notification code without altering emoji shortcodes or nesting interactive controls', async () => { +test('renders notification code without altering emoji shortcodes or nesting interactive controls', async () => { const html = await renderInbox([ { ...quote, @@ -152,7 +149,7 @@ void test('renders notification code without altering emoji shortcodes or nestin assert.doesNotMatch(html, /Copy code/); }); -void test('renders highlighted notification blocks without a nested copy button', async () => { +test('renders highlighted notification blocks without a nested copy button', async () => { const html = await renderInbox([ { ...quote, @@ -177,14 +174,13 @@ void test('renders highlighted notification blocks without a nested copy button' assert.doesNotMatch(html, /Copy code/); }); - -void test('renders all notification categories as enabled toggle buttons by default', async () => { +test('renders all notification categories as enabled toggle buttons by default', async () => { const body = await renderInbox([]); assert.equal((body.match(/aria-pressed="true"/g) ?? []).length, 6); assert.doesNotMatch(body, /aria-current="page"|role="tab"/); }); -void test('renders independent category selections and the all-off empty state', async () => { +test('renders independent category selections and the all-off empty state', async () => { const selected = await renderInbox([], 'replies,mentions'); assert.equal((selected.match(/aria-pressed="true"/g) ?? []).length, 2); assert.equal((selected.match(/aria-pressed="false"/g) ?? []).length, 4); diff --git a/tests/Unit/NotificationIndex.test.tsx b/tests/Unit/NotificationIndex.test.tsx index 3a24c6b85..c29178a4a 100644 --- a/tests/Unit/NotificationIndex.test.tsx +++ b/tests/Unit/NotificationIndex.test.tsx @@ -1,9 +1,9 @@ /// import assert from 'node:assert/strict'; -import { test } from 'node:test'; import { createInertiaApp } from '@inertiajs/react'; import { renderToString } from 'react-dom/server'; +import { test } from 'vite-plus/test'; import Index from '../../resources/js/pages/notification/index'; import type { NotificationCommit, @@ -85,7 +85,7 @@ async function render(notifications: NotificationGroup[]): Promise { return result.body.slice(result.body.indexOf(' { +test('renders a code block highlighted in a notification', async () => { const body = await render([notification]); assert.match(body, /]*>return<\/span>/); @@ -99,7 +99,7 @@ for (const [imagesCount, label] of [ [1, 'Image'], [3, '3 images'], ] as const) { - void test(`says "${label}" for a starred commit of images alone instead of an empty excerpt`, async () => { + test(`says "${label}" for a starred commit of images alone instead of an empty excerpt`, async () => { const starred = { ...notification, type: 'starred', @@ -113,19 +113,16 @@ for (const [imagesCount, label] of [ } as NotificationCommit, } as NotificationGroup; const body = await render([starred]); - - assert.match( - body, - new RegExp( - `class="mt-2 rounded-lg border[^"]*">

]*>(?:(?!).)*${label}

`, - 's', - ), + const block = body.match( + /
(.*?)<\/div>/s, ); + + assert.equal(block?.[1].replace(/<[^>]+>/g, ''), label); }); } for (const isFollowing of [false, true]) { - void 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', diff --git a/tests/draft-images.config.ts b/tests/draft-images.config.ts index 81c1aaadb..e98db5909 100644 --- a/tests/draft-images.config.ts +++ b/tests/draft-images.config.ts @@ -16,6 +16,8 @@ export default defineConfig({ 'tests/Unit/EmojiSearch.test.tsx', 'tests/Unit/EmojiShortcodes.test.tsx', 'tests/Unit/QuoteReply.test.tsx', + 'tests/Unit/NotificationIndex.test.tsx', + 'tests/Unit/NotificationInbox.test.tsx', ], }, });