From 6f6395c328afd8729341888e051e505d8d4004b5 Mon Sep 17 00:00:00 2001 From: Mark Date: Sat, 24 Oct 2026 02:31:34 +0200 Subject: [PATCH 1/5] fix: retain author avatars through automatic feed loading --- KNOWLEDGE_BASE.md | 2 +- resources/js/pages/log/show.tsx | 120 ++++++++++++++++--------------- tests/Unit/LogInsertion.test.tsx | 61 ++++++++++++---- 3 files changed, 110 insertions(+), 73 deletions(-) diff --git a/KNOWLEDGE_BASE.md b/KNOWLEDGE_BASE.md index c0fd446eb..dbbd4767b 100644 --- a/KNOWLEDGE_BASE.md +++ b/KNOWLEDGE_BASE.md @@ -153,7 +153,7 @@ Sources: [follow import](app/Actions/CreateUserGitHubFollows.php), [suggestions] Logs are the app's timelines. **Following** contains your commits and those of developers you follow, newest first; it requires sign-in. **Latest** contains the newest public commits across the app. **Trending** favors recent public commits that attract engagement from distinct other developers across the thread, with replies and quotes weighing more than stars and diminishing returns from additional engagement. Pastéis don't affect Trending. On the web, Trending reshuffles each time you open it: higher-scoring commits are more likely to appear first, but the order varies between visits and the same draw is used while you scroll. The signed API does not redraw on each visit. -Logs list top-level commits, including quotes, rather than every reply, 20 at a time as you scroll, and show your own pending commits to you. Following and Latest check for new commits every 30 seconds and mark where you left off. The new-commit control previews up to five distinct authors, with a +N badge for additional authors. Following loads the new commits when you select it. Latest loads new commits automatically: at the top they animate into view, while a reader who has scrolled keeps their place and can scroll up to see them. On touch screens, swipe sideways to move between logs. An empty Following log suggests three recent commits from people to follow. Muted and blocked people are filtered for signed-in viewers. The last log you opened is remembered. +Logs list top-level commits, including quotes, rather than every reply, 20 at a time as you scroll, and show your own pending commits to you. Following and Latest check for new commits every 30 seconds and mark where you left off. The new-commit control previews up to five distinct authors, with a +N circle for additional authors. The avatars stay visible while Latest fetches new commits and while its loaded prompt remains on screen. Following loads the new commits when you select it. Latest loads new commits automatically: at the top they animate into view, while a reader who has scrolled keeps their place and can scroll up to see them. On touch screens, swipe sideways to move between logs. An empty Following log suggests three recent commits from people to follow. Muted and blocked people are filtered for signed-in viewers. The last log you opened is remembered. Browser Back and Forward restore expanded replies and loaded thread continuations. Offscreen commits are measured before scroll restoration so returning to a log or discussion keeps the content in place. diff --git a/resources/js/pages/log/show.tsx b/resources/js/pages/log/show.tsx index 0e50957f7..590d8a438 100644 --- a/resources/js/pages/log/show.tsx +++ b/resources/js/pages/log/show.tsx @@ -42,6 +42,7 @@ export default function Show({ const autoLoads = log === 'latest'; const [isFetching, setIsFetching] = useState(false); const [announced, setAnnounced] = useState(0); + const [announcedAuthors, setAnnouncedAuthors] = useState(newCommitAuthors); const [loaded, setLoaded] = useState<'shown' | 'leaving' | null>(null); const [isAway, setIsAway] = useState(false); const [stateLog, setStateLog] = useState(log); @@ -58,6 +59,7 @@ export default function Show({ setIsFetching(false); setLoaded(null); setWaiting(0); + setAnnouncedAuthors({ users: [], total: 0 }); } const commitDialogClosedCount = useCommitDialogClosedCount(); @@ -335,6 +337,7 @@ export default function Show({ const timer = setTimeout(() => { const count = newCommitsCount; setAnnounced(count); + setAnnouncedAuthors(newCommitAuthors); setIsFetching(true); loadingNew.current = true; @@ -369,7 +372,23 @@ export default function Show({ return () => { clearTimeout(timer); }; - }, [autoLoads, newCommitsCount, isFetching, oldestLoadedId]); + }, [ + autoLoads, + newCommitsCount, + newCommitAuthors, + isFetching, + oldestLoadedId, + ]); + + const preview = + isFetching || newCommitsCount === 0 + ? announcedAuthors + : newCommitAuthors; + const announcementLabel = isFetching + ? `Fetching ${announced} new ${announced === 1 ? 'commit' : 'commits'}` + : newCommitsCount > 0 + ? `Show ${newCommitsCount} new ${newCommitsCount === 1 ? 'commit' : 'commits'}` + : `${waiting} new ${waiting === 1 ? 'commit' : 'commits'} loaded. Scroll to top`; return ( <> @@ -428,73 +447,60 @@ export default function Show({ : reloadNewestFirst } disabled={isFetching} - aria-label={ - isFetching - ? `Fetching ${announced} new commits` - : newCommitsCount > 0 - ? `Show ${newCommitsCount} new ${newCommitsCount === 1 ? 'commit' : 'commits'}` - : `${waiting} new ${waiting === 1 ? 'commit' : 'commits'} loaded. Scroll to top` - } + aria-label={announcementLabel} + title={announcementLabel} className={cn( loaded === 'leaving' && newCommitsCount === 0 && '-translate-y-2 opacity-0', - 'pointer-events-auto mt-3 grid min-h-11 max-w-[calc(100vw-2rem)] items-center gap-1.5 rounded-md bg-primary px-4 py-1.5 text-sm font-medium text-primary-foreground shadow-[0_4px_14px_-4px_rgb(0_0_0/0.35)] transition-colors hover:bg-primary-hover motion-safe:animate-in motion-safe:transition-[opacity,translate,background-color] motion-safe:duration-450 motion-safe:fade-in motion-safe:slide-in-from-top-2', + 'pointer-events-auto mt-3 flex min-h-11 max-w-[calc(100vw-2rem)] items-center gap-1.5 rounded-md bg-primary px-4 py-1.5 text-sm font-medium text-primary-foreground shadow-[0_4px_14px_-4px_rgb(0_0_0/0.35)] transition-colors hover:bg-primary-hover motion-safe:animate-in motion-safe:transition-[opacity,translate,background-color] motion-safe:duration-450 motion-safe:fade-in motion-safe:slide-in-from-top-2', )} > + {isFetching ? ( + + ) : ( + + )} - - {`${Math.max(newCommitsCount, announced, waiting)} new commits loaded · Click to scroll up`} - - - {isFetching ? ( - - ) : ( - - )} - {isFetching ? ( - `Fetching ${announced === 1 ? 'commit' : `${announced} commits`}…` - ) : newCommitsCount === 0 ? ( - `${waiting === 1 ? '1 new commit' : `${waiting} new commits`} loaded${isAway ? ` · ${window.matchMedia('(pointer: coarse)').matches ? 'Tap' : 'Click'} to scroll up` : ''}` - ) : ( - <> - - {newCommitAuthors.users.map( - (author) => ( - - ), - )} - {newCommitAuthors.total > 5 && ( - - + - {Math.min( - newCommitAuthors.total - 5, - 99, - )} - {newCommitAuthors.total - 5 > 99 - ? '+' - : ''} - - )} - - committed - + {preview.users.map((author) => ( + + ))} + {preview.total > 5 && ( + + {preview.total - 5 > 99 + ? '99+' + : `+${preview.total - 5}`} + )} + + + loaded · Scroll up + + + {isFetching + ? 'fetching…' + : newCommitsCount > 0 + ? 'committed' + : isAway + ? 'loaded · Scroll up' + : 'loaded'} + + )} diff --git a/tests/Unit/LogInsertion.test.tsx b/tests/Unit/LogInsertion.test.tsx index 815aa51e6..e7038d457 100644 --- a/tests/Unit/LogInsertion.test.tsx +++ b/tests/Unit/LogInsertion.test.tsx @@ -2,7 +2,7 @@ import assert from 'node:assert/strict'; import * as React from 'react'; import type { ReactElement, ReactNode } from 'react'; import { afterEach, test, vi } from 'vite-plus/test'; -import type { Commit, CursorPaginated } from '../../resources/js/types'; +import type { Commit, CursorPaginated, User } from '../../resources/js/types'; let slots: unknown[]; let cursor: number; @@ -70,7 +70,7 @@ afterEach(() => { vi.unstubAllGlobals(); }); -function insertAt(offset: number, reduced = false) { +function insertAt(offset: number, reduced = false, authors: User[] = []) { slots = []; listeners.clear(); reload.mockClear(); @@ -130,6 +130,10 @@ function insertAt(offset: number, reduced = false) { commits, isRanked: false, newCommitsCount: count, + newCommitAuthors: { + users: count > 0 ? authors : [], + total: count > 0 ? authors.length : 0, + }, suggestions: [], }), ); @@ -144,9 +148,12 @@ function insertAt(offset: number, reduced = false) { } for (const effect of layouts) effect(); for (const effect of effects) effect(); + return nodes; }; - render(commits(['existing']), 3); - vi.advanceTimersByTime(1700); + const pending = render(commits(['existing']), 3); + vi.advanceTimersByTime(1000); + const fetching = render(commits(['existing']), 3); + vi.advanceTimersByTime(700); const incoming = commits(['one', 'two', 'three', 'existing']); listeners.get('beforeUpdate')!({ detail: { @@ -162,9 +169,11 @@ function insertAt(offset: number, reduced = false) { render(incoming); const finish = () => { reload.mock.calls.at(-1)![0].onFinish(); - render(incoming); + return render(incoming); }; return { + pending, + fetching, animate, top: existing.getBoundingClientRect().top, scrollY, @@ -208,7 +217,14 @@ test('keeps the loaded control while new commits are above a shallowly scrolled assert.equal(vi.getTimerCount(), 0); }); -for (const total of [1, 5, 6, 104, 10000]) { +for (const [total, label] of [ + [1, undefined], + [5, undefined], + [6, '+1'], + [7, '+2'], + [104, '+99'], + [10000, '99+'], +] as const) { test(`previews five authors and caps the overflow badge for ${total} authors`, () => { slots = []; cursor = 0; @@ -244,16 +260,9 @@ for (const total of [1, 5, 6, 104, 10000]) { const badge = nodes.find( (node) => node.props.title === `${total - 5} more authors`, ); - assert.equal(Boolean(badge), total > 5); + assert.equal(Boolean(badge), label !== undefined); if (badge) { - assert.deepEqual( - React.Children.toArray(badge.props.children as ReactNode), - [ - '+', - Math.min(total - 5, 99), - total - 5 > 99 ? '+' : '', - ], - ); + assert.equal(badge.props.children, label); } reload.mockClear(); (button.props.onClick as () => void)(); @@ -264,3 +273,25 @@ for (const total of [1, 5, 6, 104, 10000]) { ]); }); } + +test('keeps the announced avatars while fetching and after the author prop clears', () => { + const authors = [ + { id: 'author', name: 'Demo author', avatar_url: null }, + ] as User[]; + const result = insertAt(500, false, authors); + const loaded = result.finish(); + + for (const nodes of [result.pending, result.fetching, loaded]) { + const previews = nodes.filter((node) => node.props.user !== undefined); + assert.equal(previews.length, 1); + assert.equal((previews[0].props.user as User).id, 'author'); + } + assert.equal( + result.fetching.find((node) => node.type === 'button')?.props.disabled, + true, + ); + assert.equal( + loaded.find((node) => node.type === 'button')?.props['aria-label'], + '3 new commits loaded. Scroll to top', + ); +}); From ac910897237ac688302c647bdd161c3f44fe9154 Mon Sep 17 00:00:00 2001 From: Mark Date: Sat, 24 Oct 2026 02:50:39 +0200 Subject: [PATCH 2/5] fix: tighten new commit prompt spacing --- resources/js/pages/log/show.tsx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/resources/js/pages/log/show.tsx b/resources/js/pages/log/show.tsx index 590d8a438..1dcc465e5 100644 --- a/resources/js/pages/log/show.tsx +++ b/resources/js/pages/log/show.tsx @@ -476,7 +476,7 @@ export default function Show({ {preview.total > 5 && ( {preview.total - 5 > 99 ? '99+' @@ -489,16 +489,14 @@ export default function Show({ aria-hidden className="invisible col-start-1 row-start-1" > - loaded · Scroll up + committed {isFetching ? 'fetching…' : newCommitsCount > 0 ? 'committed' - : isAway - ? 'loaded · Scroll up' - : 'loaded'} + : 'loaded'} From b91fdd33964fd2d273ac269fa247fc5327dc680c Mon Sep 17 00:00:00 2001 From: Mark Date: Sat, 24 Oct 2026 03:16:24 +0200 Subject: [PATCH 3/5] fix: simplify automatic log updates --- KNOWLEDGE_BASE.md | 2 +- app/Http/Controllers/LogController.php | 5 - app/Queries/LatestLogCommitsQuery.php | 20 - resources/js/pages/log/show.tsx | 281 ++++++------- .../Feature/Controllers/LogControllerTest.php | 15 +- ...thorsTest.php => LatestLogCommitsTest.php} | 34 +- tests/Unit/LogInsertion.test.tsx | 391 ++++++++++++++---- 7 files changed, 446 insertions(+), 302 deletions(-) rename tests/Feature/{LatestLogAuthorsTest.php => LatestLogCommitsTest.php} (57%) diff --git a/KNOWLEDGE_BASE.md b/KNOWLEDGE_BASE.md index dbbd4767b..e15ed71e4 100644 --- a/KNOWLEDGE_BASE.md +++ b/KNOWLEDGE_BASE.md @@ -153,7 +153,7 @@ Sources: [follow import](app/Actions/CreateUserGitHubFollows.php), [suggestions] Logs are the app's timelines. **Following** contains your commits and those of developers you follow, newest first; it requires sign-in. **Latest** contains the newest public commits across the app. **Trending** favors recent public commits that attract engagement from distinct other developers across the thread, with replies and quotes weighing more than stars and diminishing returns from additional engagement. Pastéis don't affect Trending. On the web, Trending reshuffles each time you open it: higher-scoring commits are more likely to appear first, but the order varies between visits and the same draw is used while you scroll. The signed API does not redraw on each visit. -Logs list top-level commits, including quotes, rather than every reply, 20 at a time as you scroll, and show your own pending commits to you. Following and Latest check for new commits every 30 seconds and mark where you left off. The new-commit control previews up to five distinct authors, with a +N circle for additional authors. The avatars stay visible while Latest fetches new commits and while its loaded prompt remains on screen. Following loads the new commits when you select it. Latest loads new commits automatically: at the top they animate into view, while a reader who has scrolled keeps their place and can scroll up to see them. On touch screens, swipe sideways to move between logs. An empty Following log suggests three recent commits from people to follow. Muted and blocked people are filtered for signed-in viewers. The last log you opened is remembered. +Logs list top-level commits, including quotes, rather than every reply, 20 at a time as you scroll, and show your own pending commits to you. Following and Latest check for new commits every 30 seconds and mark where you left off. Following and Latest load new commits automatically. At the top, they animate into view without a notification. If you have scrolled down, your reading position stays still and a **New commits** control appears only after new commits are loaded above you. It previews up to three distinct authors from the latest batch, with a +N circle for additional authors. Select it to scroll to the new commits; it disappears when you reach them. On touch screens, swipe sideways to move between logs. An empty Following log suggests three recent commits from people to follow. Muted and blocked people are filtered for signed-in viewers. The last log you opened is remembered. Browser Back and Forward restore expanded replies and loaded thread continuations. Offscreen commits are measured before scroll restoration so returning to a log or discussion keeps the content in place. diff --git a/app/Http/Controllers/LogController.php b/app/Http/Controllers/LogController.php index 878f767eb..35b718e24 100644 --- a/app/Http/Controllers/LogController.php +++ b/app/Http/Controllers/LogController.php @@ -54,11 +54,6 @@ public function show(ShowLogRequest $request, #[CurrentUser] ?User $user, Log $l return $seen instanceof Commit ? $latest->count($log, $user, $seen) : 0; }), - 'newCommitAuthors' => Inertia::optional(function () use ($request, $user, $log, $latest): array { - $seen = $request->since(); - - return $seen instanceof Commit ? $latest->authors($log, $user, $seen) : ['users' => [], 'total' => 0]; - }), 'suggestions' => fn (): Collection => $user instanceof User && $log === Log::Following && ! $user->following()->exists() ? $suggested->get($user) : new Collection(), diff --git a/app/Queries/LatestLogCommitsQuery.php b/app/Queries/LatestLogCommitsQuery.php index 43a72b3c7..26893bfa9 100644 --- a/app/Queries/LatestLogCommitsQuery.php +++ b/app/Queries/LatestLogCommitsQuery.php @@ -7,7 +7,6 @@ use App\Enums\Log; use App\Models\Commit; use App\Models\User; -use Illuminate\Support\Collection; /** * A ranked log does not put new commits on top, so it does not offer them either: they belong to the Latest log. The @@ -26,23 +25,4 @@ public function count(Log $log, ?User $user, Commit $since): int ->when($user, fn (CommitBuilder $query, User $user): CommitBuilder => $query->where($query->qualifyColumn('user_id'), '!=', $user->id)) ->count(); } - - /** - * @return array{users: Collection, total: int} - */ - public function authors(Log $log, ?User $user, Commit $since): array - { - if ($log->isRanked()) { - return ['users' => collect(), 'total' => 0]; - } - - $query = $log->query()->builder($user) - ->newerThan($since) - ->when($user, fn (CommitBuilder $query, User $user): CommitBuilder => $query->where($query->qualifyColumn('user_id'), '!=', $user->id)) - ->reorder()->toBase(); - $total = (clone $query)->distinct()->count('commits.user_id'); - $ids = $query->select('commits.user_id')->distinct()->orderBy('commits.user_id')->limit(5)->pluck('user_id'); - - return ['users' => User::query()->whereIn('id', $ids)->orderBy('id')->get()->toBase(), 'total' => $total]; - } } diff --git a/resources/js/pages/log/show.tsx b/resources/js/pages/log/show.tsx index 1dcc465e5..51dc1cd85 100644 --- a/resources/js/pages/log/show.tsx +++ b/resources/js/pages/log/show.tsx @@ -1,6 +1,13 @@ +import type { CancelToken } from '@inertiajs/core'; import { Head, Link, router, usePage, usePoll } from '@inertiajs/react'; -import { ArrowUp, LoaderCircle } from 'lucide-react'; -import { useEffect, useLayoutEffect, useRef, useState } from 'react'; +import { ArrowUp } from 'lucide-react'; +import { + useCallback, + useEffect, + useLayoutEffect, + useRef, + useState, +} from 'react'; import LogController from '@/actions/App/Http/Controllers/LogController'; import { CommitComposer } from '@/components/commit-composer'; import { useCommitDialogClosedCount } from '@/components/commit-composer-dialog'; @@ -24,42 +31,34 @@ export default function Show({ log, commits, isRanked, - newCommitsCount = 0, - newCommitAuthors = { users: [], total: 0 }, suggestions, }: { log: Log; commits: CursorPaginated; isRanked: boolean; - newCommitsCount?: number; - newCommitAuthors?: { users: User[]; total: number }; suggestions: Commit[]; }) { const { props: { auth }, } = usePage(); const user = auth.user; - const autoLoads = log === 'latest'; - const [isFetching, setIsFetching] = useState(false); - const [announced, setAnnounced] = useState(0); - const [announcedAuthors, setAnnouncedAuthors] = useState(newCommitAuthors); - const [loaded, setLoaded] = useState<'shown' | 'leaving' | null>(null); - const [isAway, setIsAway] = useState(false); + const [notice, setNotice] = useState<{ + newestId: string; + users: User[]; + total: number; + count: number; + leaving: boolean; + } | null>(null); const [stateLog, setStateLog] = useState(log); - const [waiting, setWaiting] = useState(0); - const loadedTimers = useRef[]>([]); - const seenFresh = useRef<() => void>(() => undefined); - const fetchTimer = useRef>(undefined); + const noticeTimers = useRef[]>([]); + const loadRequest = useRef(null); const loadingNew = useRef(false); const freshIds = useRef>(new Set()); // Another log has its own commits to announce, so nothing of the last one's pill carries over. if (stateLog !== log) { setStateLog(log); - setIsFetching(false); - setLoaded(null); - setWaiting(0); - setAnnouncedAuthors({ users: [], total: 0 }); + setNotice(null); } const commitDialogClosedCount = useCommitDialogClosedCount(); @@ -231,7 +230,6 @@ export default function Show({ if (visible.size > 0 && sightedAt === 0) { sightedAt = Date.now(); timer = setTimeout(settle, 7000); - seenFresh.current(); } }, { threshold: 0.6 }, @@ -256,12 +254,67 @@ export default function Show({ const firstSeenId = useLastVisit(viewerId, log, commits.data); + const loadNewest = (count: number) => { + if (isRanked || !count || loadingNew.current) { + return; + } + + const known = new Set(commits.data.map((commit) => commit.id)); + loadingNew.current = true; + router.reload( + backgroundPollOptions({ + only: ['commits', 'newCommitsCount'], + reset: ['commits'], + data: { since: '' }, + // Retain the loaded range so inserting new commits preserves the reader's row. + headers: { 'X-Log-Through': oldestLoadedId ?? '' }, + preserveUrl: true, + onCancelToken: (token) => { + loadRequest.current = token; + }, + onSuccess: (page) => { + const incoming = page.props + .commits as CursorPaginated; + const arrivals = incoming.data.filter( + (commit) => + !known.has(commit.id) && + commit.user_id !== user?.id, + ); + if (window.scrollY <= 0 || arrivals.length === 0) return; + + const authors = [ + ...new Map( + arrivals.map((commit) => [ + commit.user.id, + commit.user, + ]), + ).values(), + ]; + noticeTimers.current.forEach(clearTimeout); + setNotice((current) => ({ + newestId: arrivals[0].id, + users: authors.slice(0, 3), + total: authors.length, + count: (current?.count ?? 0) + arrivals.length, + leaving: false, + })); + }, + onFinish: () => { + loadingNew.current = false; + loadRequest.current = null; + }, + })(), + ); + }; + const { stop: stopPolling } = usePoll( 30_000, backgroundPollOptions(() => ({ - only: ['newCommitsCount', 'newCommitAuthors'], + only: ['newCommitsCount'], data: { since: newestId }, preserveUrl: true, + onSuccess: (page) => + loadNewest(page.props.newCommitsCount as number), })), { autoStart: newestId !== undefined }, ); @@ -281,114 +334,51 @@ export default function Show({ [stopPolling], ); - const reloadNewestFirst = () => - router.reload({ - only: ['commits', 'newCommitsCount', 'newCommitAuthors'], - reset: ['commits'], - data: { since: '' }, - preserveUrl: true, - onSuccess: () => window.scrollTo({ top: 0, behavior: 'smooth' }), - }); - - // The pill stays until the new commits have been seen, then lingers a moment and fades. - const dismissLoaded = (after: number) => { - loadedTimers.current.forEach(clearTimeout); - loadedTimers.current = [ - setTimeout(() => setLoaded('leaving'), after), - setTimeout(() => { - setLoaded(null); - setWaiting(0); - }, after + 450), - ]; - }; + const dismissNotice = useCallback(() => { + noticeTimers.current.forEach(clearTimeout); + setNotice((current) => current && { ...current, leaving: true }); + noticeTimers.current = [setTimeout(() => setNotice(null), 450)]; + }, []); + + const noticeNewestId = notice?.newestId; useEffect(() => { - seenFresh.current = () => dismissLoaded(1500); - }); + if (!noticeNewestId) return; + const row = Array.from( + feedRef.current?.querySelectorAll('[data-commit]') ?? + [], + ).find((row) => row.dataset.commit === noticeNewestId); + if (!row) return; + + const observer = new IntersectionObserver( + (entries) => { + if (entries.some((entry) => entry.isIntersecting)) + dismissNotice(); + }, + { threshold: 0.6 }, + ); + observer.observe(row); + + return () => observer.disconnect(); + }, [noticeNewestId, commits, dismissNotice]); useEffect( () => () => { - loadedTimers.current.forEach(clearTimeout); - clearTimeout(fetchTimer.current); + noticeTimers.current.forEach(clearTimeout); + loadRequest.current?.cancel(); }, - [], + [log], ); useEffect(() => { - const update = () => setIsAway(window.scrollY > 120); - update(); + const update = () => { + if (window.scrollY <= 0) dismissNotice(); + }; window.addEventListener('scroll', update, { passive: true }); return () => window.removeEventListener('scroll', update); - }, []); - - // Back at the top, the new commits are right there, so the pill has nothing left to ask for. - useEffect(() => { - if (!isAway && loaded === 'shown' && window.scrollY <= 0) { - seenFresh.current(); - } - }, [isAway, loaded]); - - // On Latest, new commits are fetched on their own, once the pill has had a moment to be read. - useEffect(() => { - if (!autoLoads || newCommitsCount === 0 || isFetching) { - return; - } + }, [dismissNotice]); - const timer = setTimeout(() => { - const count = newCommitsCount; - setAnnounced(count); - setAnnouncedAuthors(newCommitAuthors); - setIsFetching(true); - loadingNew.current = true; - - // The pill says it is fetching for a beat, so it reads as a step even when the answer is quick. - fetchTimer.current = setTimeout( - () => - router.reload({ - only: [ - 'commits', - 'newCommitsCount', - 'newCommitAuthors', - ], - reset: ['commits'], - data: { since: '' }, - // The new commits come with everything already loaded, so the reader keeps their row. - headers: { - 'X-Log-Through': oldestLoadedId ?? '', - }, - preserveUrl: true, - onFinish: () => { - loadingNew.current = false; - setIsFetching(false); - setWaiting((total) => total + count); - setLoaded('shown'); - loadedTimers.current.forEach(clearTimeout); - }, - }), - 700, - ); - }, 1000); - - return () => { - clearTimeout(timer); - }; - }, [ - autoLoads, - newCommitsCount, - newCommitAuthors, - isFetching, - oldestLoadedId, - ]); - - const preview = - isFetching || newCommitsCount === 0 - ? announcedAuthors - : newCommitAuthors; - const announcementLabel = isFetching - ? `Fetching ${announced} new ${announced === 1 ? 'commit' : 'commits'}` - : newCommitsCount > 0 - ? `Show ${newCommitsCount} new ${newCommitsCount === 1 ? 'commit' : 'commits'}` - : `${waiting} new ${waiting === 1 ? 'commit' : 'commits'} loaded. Scroll to top`; + const announcementLabel = `${notice?.count ?? 0} new ${notice?.count === 1 ? 'commit' : 'commits'} above. Scroll to top`; return ( <> @@ -428,44 +418,29 @@ export default function Show({ )} - {(newCommitsCount > 0 || isFetching || loaded !== null) && ( + {notice !== null && (
)} diff --git a/tests/Feature/Controllers/LogControllerTest.php b/tests/Feature/Controllers/LogControllerTest.php index 776b3df97..a0a1b46ee 100644 --- a/tests/Feature/Controllers/LogControllerTest.php +++ b/tests/Feature/Controllers/LogControllerTest.php @@ -782,7 +782,7 @@ ->assertUnprocessable()->assertJsonValidationErrors('through'); }); -it('returns distinct authors when polling for newer commits', function (): void { +it('polls only the count while loaded commits provide their authors', function (): void { $viewer = User::factory()->create(); $older = Commit::factory()->create(['created_at' => now()->subMinute()]); $author = User::factory()->create(); @@ -793,22 +793,19 @@ 'X-Inertia' => 'true', 'X-Inertia-Version' => (string) resolve(HandleInertiaRequests::class)->version(request()), 'X-Inertia-Partial-Component' => 'log/show', - 'X-Inertia-Partial-Data' => 'newCommitsCount,newCommitAuthors', + 'X-Inertia-Partial-Data' => 'newCommitsCount', ])->assertOk() ->assertJsonPath('props.newCommitsCount', 2) - ->assertJsonPath('props.newCommitAuthors.total', 1) - ->assertJsonCount(1, 'props.newCommitAuthors.users') - ->assertJsonPath('props.newCommitAuthors.users.0.id', $author->id); + ->assertJsonMissingPath('props.newCommitAuthors'); }); -it('clears author previews when reloading without a since cursor', function (): void { +it('resets the pending count when reloading without a since cursor', function (): void { $this->actingAs(User::factory()->create())->get(route('log.show', Log::Latest), [ 'X-Inertia' => 'true', 'X-Inertia-Version' => (string) resolve(HandleInertiaRequests::class)->version(request()), 'X-Inertia-Partial-Component' => 'log/show', - 'X-Inertia-Partial-Data' => 'newCommitsCount,newCommitAuthors', + 'X-Inertia-Partial-Data' => 'newCommitsCount', ])->assertOk() ->assertJsonPath('props.newCommitsCount', 0) - ->assertJsonPath('props.newCommitAuthors.total', 0) - ->assertJsonPath('props.newCommitAuthors.users', []); + ->assertJsonMissingPath('props.newCommitAuthors'); }); diff --git a/tests/Feature/LatestLogAuthorsTest.php b/tests/Feature/LatestLogCommitsTest.php similarity index 57% rename from tests/Feature/LatestLogAuthorsTest.php rename to tests/Feature/LatestLogCommitsTest.php index 163f98793..073ca0358 100644 --- a/tests/Feature/LatestLogAuthorsTest.php +++ b/tests/Feature/LatestLogCommitsTest.php @@ -7,7 +7,7 @@ use App\Models\User; use App\Queries\LatestLogCommitsQuery; -it('returns five distinct authors and counts additional authors rather than commits', function (): void { +it('counts every newer commit while excluding the viewer', function (): void { $viewer = User::factory()->create(); $seen = Commit::factory()->create(['created_at' => now()->subHour()]); $authors = User::factory()->count(7)->create(); @@ -16,13 +16,10 @@ }); Commit::factory()->for($viewer)->create(); - $result = resolve(LatestLogCommitsQuery::class)->authors(Log::Latest, $viewer, $seen); - - expect($result['total'])->toBe(7)->and($result['users'])->toHaveCount(5); - expect($result['users']->pluck('id')->diff($authors->modelKeys()))->toBeEmpty(); + expect(resolve(LatestLogCommitsQuery::class)->count(Log::Latest, $viewer, $seen))->toBe(14); }); -it('only shows followed authors in Following and none in Trending', function (): void { +it('only counts followed authors in Following and none in Trending', function (): void { $viewer = User::factory()->create(); $seen = Commit::factory()->create(['created_at' => now()->subHour()]); $followed = User::factory()->create(); @@ -30,10 +27,8 @@ Commit::factory()->for($followed)->create(); Commit::factory()->create(); $query = resolve(LatestLogCommitsQuery::class); - $result = $query->authors(Log::Following, $viewer, $seen); - - expect($result['total'])->toBe(1)->and($result['users']->pluck('id')->all())->toBe([$followed->id]); - expect($query->authors(Log::Trending, $viewer, $seen)['total'])->toBe(0); + expect($query->count(Log::Following, $viewer, $seen))->toBe(1); + expect($query->count(Log::Trending, $viewer, $seen))->toBe(0); }); it('excludes hidden authors replies and commits at or before the cursor', function (): void { @@ -54,25 +49,18 @@ Commit::factory()->create(['parent_id' => $seen->id]); Commit::factory()->create(['created_at' => now()->subHours(2)]); Commit::factory()->for($viewer)->create(); - $visible = Commit::factory()->create(); - - $result = resolve(LatestLogCommitsQuery::class)->authors(Log::Latest, $viewer, $seen); + Commit::factory()->create(); - expect($result['total'])->toBe(1) - ->and($result['users']->pluck('id')->all())->toBe([$visible->user_id]); + expect(resolve(LatestLogCommitsQuery::class)->count(Log::Latest, $viewer, $seen))->toBe(1); }); -it('previews public authors for guests and returns no authors when none are new', function (): void { +it('counts public commits for guests and returns zero when none are new', function (): void { $seen = Commit::factory()->create(['created_at' => now()->subHour()]); $query = resolve(LatestLogCommitsQuery::class); - expect($query->authors(Log::Latest, null, $seen)['total'])->toBe(0) - ->and($query->authors(Log::Latest, null, $seen)['users'])->toBeEmpty(); + expect($query->count(Log::Latest, null, $seen))->toBe(0); Commit::factory()->pending()->create(); - $visible = Commit::factory()->create(); - $result = $query->authors(Log::Latest, null, $seen); - - expect($result['total'])->toBe(1) - ->and($result['users']->pluck('id')->all())->toBe([$visible->user_id]); + Commit::factory()->create(); + expect($query->count(Log::Latest, null, $seen))->toBe(1); }); diff --git a/tests/Unit/LogInsertion.test.tsx b/tests/Unit/LogInsertion.test.tsx index e7038d457..f83926930 100644 --- a/tests/Unit/LogInsertion.test.tsx +++ b/tests/Unit/LogInsertion.test.tsx @@ -1,15 +1,23 @@ import assert from 'node:assert/strict'; +import type { ReloadOptions } from '@inertiajs/core'; import * as React from 'react'; import type { ReactElement, ReactNode } from 'react'; import { afterEach, test, vi } from 'vite-plus/test'; -import type { Commit, CursorPaginated, User } from '../../resources/js/types'; +import type { + Commit, + CursorPaginated, + Log, + User, +} from '../../resources/js/types'; let slots: unknown[]; let cursor: number; let effects: (() => unknown)[]; let layouts: (() => unknown)[]; const listeners = new Map void>(); +let viewer: User | null = null; const reload = vi.fn(); +let pollOptions: () => ReloadOptions; vi.doMock('react', () => ({ ...React, useRef: (initial: unknown) => (slots[cursor++] ??= { current: initial }), @@ -24,14 +32,55 @@ vi.doMock('react', () => ({ }, ]; }, - useEffect: (effect: () => unknown) => effects.push(effect), - useLayoutEffect: (effect: () => unknown) => layouts.push(effect), + useCallback: (callback: unknown, dependencies: unknown[]) => { + const index = cursor++; + const previous = slots[index] as + | { dependencies: unknown[]; callback: unknown } + | undefined; + if ( + !previous || + dependencies.some( + (value, i) => !Object.is(value, previous.dependencies[i]), + ) + ) + slots[index] = { callback, dependencies }; + return (slots[index] as { callback: unknown }).callback; + }, + useEffect: (effect: () => unknown, dependencies?: unknown[]) => + scheduleEffect(effects, effect, dependencies), + useLayoutEffect: (effect: () => unknown, dependencies?: unknown[]) => + scheduleEffect(layouts, effect, dependencies), })); +function scheduleEffect( + queue: (() => unknown)[], + effect: () => unknown, + dependencies?: unknown[], +) { + const index = cursor++; + const previous = slots[index] as + | { dependencies?: unknown[]; cleanup?: () => void } + | undefined; + if ( + !previous || + !dependencies || + dependencies.some( + (value, i) => !Object.is(value, previous.dependencies?.[i]), + ) + ) { + queue.push(() => { + previous?.cleanup?.(); + slots[index] = { dependencies, cleanup: effect() }; + }); + } +} vi.doMock('@inertiajs/react', () => ({ Head: () => null, Link: () => null, - usePage: () => ({ props: { auth: { user: null } } }), - usePoll: () => ({ stop: vi.fn() }), + usePage: () => ({ props: { auth: { user: viewer } } }), + usePoll: (_interval: number, options: () => ReloadOptions) => { + pollOptions = options; + return { stop: vi.fn() }; + }, router: { on: (name: string, listener: (event: unknown) => void) => { listeners.set(name, listener); @@ -68,9 +117,16 @@ function elements(node: ReactNode): Element[] { afterEach(() => { vi.useRealTimers(); vi.unstubAllGlobals(); + viewer = null; }); -function insertAt(offset: number, reduced = false, authors: User[] = []) { +function insertAt( + offset: number, + reduced = false, + authors: User[] = [], + log: Log = 'latest', + total = 3, +) { slots = []; listeners.clear(); reload.mockClear(); @@ -87,21 +143,36 @@ function insertAt(offset: number, reduced = false, authors: User[] = []) { height: 120, }), }; - const fresh = ['one', 'two', 'three'].map((id) => ({ + const fresh = Array.from( + { length: total }, + (_, index) => `new-${index}`, + ).map((id) => ({ dataset: { commit: id }, getBoundingClientRect: () => ({ height: 120 }), animate, + removeAttribute: vi.fn(), })); let rows: unknown[] = [existing]; + const scrollListeners: (() => void)[] = []; + const intersections: { + callback: (entries: unknown[]) => void; + disconnected: boolean; + observed: Set; + }[] = []; vi.stubGlobal('window', { get scrollY() { return scrollY; }, + scrollTo: ({ top }: { top: number }) => { + scrollY = top; + }, scrollBy: (_x: number, y: number) => { scrollY += y; }, matchMedia: () => ({ matches: reduced }), - addEventListener: vi.fn(), + addEventListener: (name: string, listener: () => void) => { + if (name === 'scroll') scrollListeners.push(listener); + }, removeEventListener: vi.fn(), }); vi.stubGlobal( @@ -114,26 +185,38 @@ function insertAt(offset: number, reduced = false, authors: User[] = []) { vi.stubGlobal( 'IntersectionObserver', class { - observe() {} - disconnect() {} + state: { + callback: (entries: unknown[]) => void; + disconnected: boolean; + observed: Set; + }; + constructor(callback: (entries: unknown[]) => void) { + this.state = { + callback, + disconnected: false, + observed: new Set(), + }; + intersections.push(this.state); + } + observe(row: unknown) { + this.state.observed.add(row); + } + disconnect() { + this.state.disconnected = true; + } }, ); const commits = (ids: string[]) => ({ data: ids.map((id) => ({ id })) }) as CursorPaginated; - const render = (commits: CursorPaginated, count = 0) => { + const render = (commits: CursorPaginated) => { cursor = 0; effects = []; layouts = []; const nodes = elements( Show({ - log: 'latest', + log, commits, isRanked: false, - newCommitsCount: count, - newCommitAuthors: { - users: count > 0 ? authors : [], - total: count > 0 ? authors.length : 0, - }, suggestions: [], }), ); @@ -142,7 +225,9 @@ function insertAt(offset: number, reduced = false, authors: User[] = []) { continue; const ref = node.props.ref as React.RefObject; if (node.props.className.includes('log-header')) - ref.current = { getBoundingClientRect: () => ({ bottom: 52 }) }; + ref.current = { + getBoundingClientRect: () => ({ bottom: 52, height: 52 }), + }; if (node.props.className.includes('log-feed')) ref.current = { querySelectorAll: () => rows }; } @@ -150,11 +235,25 @@ function insertAt(offset: number, reduced = false, authors: User[] = []) { for (const effect of effects) effect(); return nodes; }; - const pending = render(commits(['existing']), 3); - vi.advanceTimersByTime(1000); - const fetching = render(commits(['existing']), 3); - vi.advanceTimersByTime(700); - const incoming = commits(['one', 'two', 'three', 'existing']); + const previous = commits(['existing']); + const pending = render(previous); + const poll = () => + pollOptions().onSuccess!({ + props: { newCommitsCount: total }, + } as never); + poll(); + const fetching = render(previous); + const incoming = { + data: [ + ...fresh.map((row, index) => { + const user = authors.length + ? authors[index % authors.length] + : { id: 'author', name: 'Author', avatar_url: null }; + return { id: row.dataset.commit, user_id: user.id, user }; + }), + ...previous.data, + ], + } as CursorPaginated; listeners.get('beforeUpdate')!({ detail: { page: { @@ -164,14 +263,42 @@ function insertAt(offset: number, reduced = false, authors: User[] = []) { }, }, }); - rowPosition += 360; + rowPosition += total * 120; rows = [...fresh, existing]; render(incoming); - const finish = () => { - reload.mock.calls.at(-1)![0].onFinish(); + let current = incoming; + const finish = (success = true, delivered = incoming) => { + const request = reload.mock.calls.at(-1)![0]; + if (success) request.onSuccess?.({ props: { commits: delivered } }); + request.onFinish(); return render(incoming); }; return { + incoming, + render: () => render(current), + paginate: () => { + current = { + ...current, + data: [...current.data, { id: 'older' } as Commit], + }; + return render(current); + }, + scrollTo: (top: number) => { + scrollY = top; + scrollListeners.forEach((listener) => listener()); + }, + sight: (index: number) => { + for (const observer of intersections) { + if ( + !observer.disconnected && + observer.observed.has(fresh[index]) + ) + observer.callback([ + { target: fresh[index], isIntersecting: true }, + ]); + } + }, + poll, pending, fetching, animate, @@ -217,81 +344,177 @@ test('keeps the loaded control while new commits are above a shallowly scrolled assert.equal(vi.getTimerCount(), 0); }); -for (const [total, label] of [ - [1, undefined], - [5, undefined], - [6, '+1'], - [7, '+2'], - [104, '+99'], - [10000, '99+'], -] as const) { - test(`previews five authors and caps the overflow badge for ${total} authors`, () => { - slots = []; - cursor = 0; - effects = []; - layouts = []; - const authors = Array.from( - { length: Math.min(total, 5) }, - (_, index) => ({ - id: `author-${index}`, - name: `Author ${index}`, - avatar_url: null, - }), - ) as import('../../resources/js/types').User[]; - const nodes = elements( - Show({ - log: 'following', - commits: { data: [{ id: 'seen' }] } as CursorPaginated, - isRanked: false, - newCommitsCount: total * 2, - newCommitAuthors: { users: authors, total }, - suggestions: [], - }), +for (const log of ['following', 'latest'] as const) { + test(`silently loads ${log} and shows one prompt only after success while scrolled`, () => { + const result = insertAt(500, false, [], log); + assert.equal( + result.pending.some((node) => node.type === 'button'), + false, + ); + assert.equal( + result.fetching.some((node) => node.type === 'button'), + false, ); - const button = nodes.find((node) => node.type === 'button')!; + const button = result.finish().find((node) => node.type === 'button')!; assert.equal( button.props['aria-label'], - `Show ${total * 2} new commits`, + '3 new commits above. Scroll to top', ); + assert.equal(button.props.disabled, undefined); + const requests = reload.mock.calls.length; + (button.props.onClick as () => void)(); + assert.equal(reload.mock.calls.length, requests); + }); + test(`does not show a prompt at the top of ${log}`, () => { + const result = insertAt(0, false, [], log); + assert.equal( + result.finish().some((node) => node.type === 'button'), + false, + ); + }); + test(`does not announce an unsuccessful ${log} reload`, () => { + const result = insertAt(500, false, [], log); + assert.equal( + result.finish(false).some((node) => node.type === 'button'), + false, + ); + }); +} + +for (const [total, label] of [ + [1, undefined], + [3, undefined], + [4, '+1'], + [7, '+4'], + [102, '+99'], + [1000, '99+'], +] as const) { + test(`previews three loaded authors and caps overflow for ${total} authors`, () => { + const authors = Array.from({ length: total }, (_, index) => ({ + id: `author-${index}`, + name: `Author ${index}`, + avatar_url: null, + })) as User[]; + const result = insertAt(500, false, authors, 'latest', total); + const nodes = result.finish(); assert.equal( nodes.filter((node) => node.props.user !== undefined).length, - Math.min(total, 5), + Math.min(total, 3), ); const badge = nodes.find( - (node) => node.props.title === `${total - 5} more authors`, + (node) => node.props.title === `${total - 3} more authors`, ); assert.equal(Boolean(badge), label !== undefined); - if (badge) { - assert.equal(badge.props.children, label); - } - reload.mockClear(); - (button.props.onClick as () => void)(); - assert.deepEqual(reload.mock.calls[0][0].only, [ - 'commits', - 'newCommitsCount', - 'newCommitAuthors', - ]); + if (badge) assert.equal(badge.props.children, label); }); } -test('keeps the announced avatars while fetching and after the author prop clears', () => { - const authors = [ - { id: 'author', name: 'Demo author', avatar_url: null }, - ] as User[]; - const result = insertAt(500, false, authors); - const loaded = result.finish(); +test('previews distinct authors from the loaded commits', () => { + const author = { + id: 'author', + name: 'Demo author', + avatar_url: null, + } as User; + const nodes = insertAt(500, false, [author]).finish(); + const previews = nodes.filter((node) => node.props.user !== undefined); + assert.equal(previews.length, 1); + assert.equal((previews[0].props.user as User).id, author.id); + assert.equal( + nodes.find((node) => node.type === 'button')!.props['aria-label'], + '3 new commits above. Scroll to top', + ); +}); + +test('does not overlap loads and retries a failed load on the next count poll', () => { + const result = insertAt(500); + assert.equal(reload.mock.calls.length, 1); + result.poll(); + assert.equal(reload.mock.calls.length, 1); + result.finish(false); + result.poll(); + assert.equal(reload.mock.calls.length, 2); +}); - for (const nodes of [result.pending, result.fetching, loaded]) { - const previews = nodes.filter((node) => node.props.user !== undefined); - assert.equal(previews.length, 1); - assert.equal((previews[0].props.user as User).id, 'author'); - } +test('only announces commits actually delivered, excluding the viewer', () => { + viewer = { id: 'viewer', name: 'Viewer', avatar_url: null } as User; + const result = insertAt(500, false, [viewer]); + assert.equal( + result.finish().some((node) => node.type === 'button'), + false, + ); +}); + +test('does not announce a successful response with no new commits', () => { + const result = insertAt(500); + const nodes = result.finish(true, { + data: [{ id: 'existing' }], + } as CursorPaginated); + assert.equal( + nodes.some((node) => node.type === 'button'), + false, + ); +}); + +test('stays silent if the reader returns to the top before loading finishes', () => { + const result = insertAt(500); + result.scrollTo(0); + assert.equal( + result.finish().some((node) => node.type === 'button'), + false, + ); +}); + +test('dismisses when the reader reaches the top', () => { + const result = insertAt(500); + result.finish(); + result.scrollTo(0); + vi.advanceTimersByTime(450); + assert.equal( + result.render().some((node) => node.type === 'button'), + false, + ); +}); + +test('keeps the prompt until the newest arriving row is visible', () => { + const result = insertAt(500); + result.finish(); + result.sight(1); + vi.advanceTimersByTime(450); assert.equal( - result.fetching.find((node) => node.type === 'button')?.props.disabled, + result.render().some((node) => node.type === 'button'), true, ); + result.sight(0); + vi.advanceTimersByTime(450); + assert.equal( + result.render().some((node) => node.type === 'button'), + false, + ); +}); + +test('still dismisses on seeing the newest commit after older highlights expire', () => { + const result = insertAt(500); + result.finish(); + result.sight(1); + vi.advanceTimersByTime(7000); + result.sight(0); + vi.advanceTimersByTime(450); + assert.equal( + result.render().some((node) => node.type === 'button'), + false, + ); +}); + +test('keeps notice dismissal active after highlights settle and pagination appends rows', () => { + const result = insertAt(500); + result.finish(); + result.sight(1); + vi.advanceTimersByTime(7000); + result.paginate(); + result.sight(0); + vi.advanceTimersByTime(450); assert.equal( - loaded.find((node) => node.type === 'button')?.props['aria-label'], - '3 new commits loaded. Scroll to top', + result.render().some((node) => node.type === 'button'), + false, ); }); From 0ca6267fde5a2f12f20a9e8cdf532a13452c89ec Mon Sep 17 00:00:00 2001 From: Mark Date: Sat, 24 Oct 2026 03:24:31 +0200 Subject: [PATCH 4/5] test: cover immediate new commit prompt fade --- tests/Unit/LogInsertion.test.tsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tests/Unit/LogInsertion.test.tsx b/tests/Unit/LogInsertion.test.tsx index f83926930..d90b2df21 100644 --- a/tests/Unit/LogInsertion.test.tsx +++ b/tests/Unit/LogInsertion.test.tsx @@ -464,11 +464,18 @@ test('stays silent if the reader returns to the top before loading finishes', () ); }); -test('dismisses when the reader reaches the top', () => { +test('starts fading immediately at the top and removes the prompt after the fade', () => { const result = insertAt(500); result.finish(); result.scrollTo(0); - vi.advanceTimersByTime(450); + const fading = result.render().find((node) => node.type === 'button')!; + assert.match(fading.props.className as string, /\bopacity-0\b/); + vi.advanceTimersByTime(449); + assert.equal( + result.render().some((node) => node.type === 'button'), + true, + ); + vi.advanceTimersByTime(1); assert.equal( result.render().some((node) => node.type === 'button'), false, From 590a4de3477159264883a6f2c7c2119c16cca45d Mon Sep 17 00:00:00 2001 From: Mark Date: Sat, 24 Oct 2026 04:01:30 +0200 Subject: [PATCH 5/5] fix: reconcile deleted commits in feed announcements --- KNOWLEDGE_BASE.md | 2 +- app/Http/Controllers/LogController.php | 1 + app/Http/Requests/ShowLogRequest.php | 13 ++ app/Queries/LatestLogCommitsQuery.php | 15 ++ resources/js/pages/log/show.tsx | 157 +++++++++++------- .../Feature/Controllers/LogControllerTest.php | 27 +++ tests/Feature/LatestLogCommitsTest.php | 22 +++ tests/Unit/LogInsertion.test.tsx | 126 +++++++++++++- 8 files changed, 304 insertions(+), 59 deletions(-) diff --git a/KNOWLEDGE_BASE.md b/KNOWLEDGE_BASE.md index 7cc14a692..d7b5e774e 100644 --- a/KNOWLEDGE_BASE.md +++ b/KNOWLEDGE_BASE.md @@ -153,7 +153,7 @@ Sources: [follow import](app/Actions/CreateUserGitHubFollows.php), [suggestions] Logs are the app's timelines. **Following** contains your commits and those of developers you follow, newest first; it requires sign-in. **Latest** contains the newest public commits across the app. **Trending** favors recent public commits that attract engagement from distinct other developers across the thread, with replies and quotes weighing more than stars and diminishing returns from additional engagement. Pastéis don't affect Trending. On the web, Trending reshuffles each time you open it: higher-scoring commits are more likely to appear first, but the order varies between visits and the same draw is used while you scroll. The signed API does not redraw on each visit. -Logs list top-level commits, including quotes, rather than every reply, 20 at a time as you scroll, and show your own pending commits to you. Following and Latest check for new commits every 30 seconds and mark where you left off. Following and Latest load new commits automatically. At the top, they animate into view without a notification. If you have scrolled down, your reading position stays still and a **New commits** control appears only after new commits are loaded above you. It previews up to three distinct authors from the latest batch, with a +N circle for additional authors. Select it to scroll to the new commits; it disappears when you reach them. On touch screens, swipe sideways to move between logs. An empty Following log suggests three recent commits from people to follow. Muted and blocked people are filtered for signed-in viewers. The last log you opened is remembered. +Logs list top-level commits, including quotes, rather than every reply, 20 at a time as you scroll, and show your own pending commits to you. Following and Latest check for new commits every 30 seconds and mark where you left off. Following and Latest load new commits automatically. At the top, they animate into view without a notification. If you have scrolled down, your reading position stays still and a **New commits** control appears only after new commits are loaded above you. It previews up to three distinct authors from the new commits waiting above you, with a +N circle for additional authors. Each check removes deleted or no-longer-visible arrivals from the announcement and loaded feed; the control disappears if none remain. Select it to scroll to the new commits; it disappears when you reach them. On touch screens, swipe sideways to move between logs. An empty Following log suggests three recent commits from people to follow. Muted and blocked people are filtered for signed-in viewers. The last log you opened is remembered. Browser Back and Forward restore expanded replies and loaded thread continuations. Offscreen commits are measured before scroll restoration so returning to a log or discussion keeps the content in place. diff --git a/app/Http/Controllers/LogController.php b/app/Http/Controllers/LogController.php index 35b718e24..9662054ab 100644 --- a/app/Http/Controllers/LogController.php +++ b/app/Http/Controllers/LogController.php @@ -54,6 +54,7 @@ public function show(ShowLogRequest $request, #[CurrentUser] ?User $user, Log $l return $seen instanceof Commit ? $latest->count($log, $user, $seen) : 0; }), + 'removedCommitIds' => Inertia::optional(fn (): array => $latest->missing($log, $user, $request->pending())), 'suggestions' => fn (): Collection => $user instanceof User && $log === Log::Following && ! $user->following()->exists() ? $suggested->get($user) : new Collection(), diff --git a/app/Http/Requests/ShowLogRequest.php b/app/Http/Requests/ShowLogRequest.php index 42424109a..9e16b704c 100644 --- a/app/Http/Requests/ShowLogRequest.php +++ b/app/Http/Requests/ShowLogRequest.php @@ -22,6 +22,8 @@ public function rules(): array return [ 'since' => ['nullable', 'uuid'], 'through' => ['nullable', 'uuid'], + 'pending' => ['sometimes', 'array', 'list'], + 'pending.*' => ['required', 'uuid', 'distinct'], ]; } @@ -45,6 +47,17 @@ public function since(): ?Commit : null; } + /** + * @return list + */ + public function pending(): array + { + /** @var list $pending */ + $pending = $this->validated('pending', []); + + return $pending; + } + public function prepareForValidation(): void { $this->merge(['through' => $this->header('X-Log-Through')]); diff --git a/app/Queries/LatestLogCommitsQuery.php b/app/Queries/LatestLogCommitsQuery.php index 26893bfa9..fa97ee2fd 100644 --- a/app/Queries/LatestLogCommitsQuery.php +++ b/app/Queries/LatestLogCommitsQuery.php @@ -25,4 +25,19 @@ public function count(Log $log, ?User $user, Commit $since): int ->when($user, fn (CommitBuilder $query, User $user): CommitBuilder => $query->where($query->qualifyColumn('user_id'), '!=', $user->id)) ->count(); } + + /** + * @param list $pending + * @return list + */ + public function missing(Log $log, ?User $user, array $pending): array + { + if ($pending === [] || $log->isRanked()) { + return []; + } + + $visible = $log->query()->builder($user)->whereKey($pending)->toBase()->select('commits.id')->reorder()->pluck('commits.id'); + + return array_values(collect($pending)->reject(fn (string $id): bool => $visible->contains($id))->all()); + } } diff --git a/resources/js/pages/log/show.tsx b/resources/js/pages/log/show.tsx index 51dc1cd85..fed5ade38 100644 --- a/resources/js/pages/log/show.tsx +++ b/resources/js/pages/log/show.tsx @@ -1,4 +1,4 @@ -import type { CancelToken } from '@inertiajs/core'; +import type { CancelToken, PageProps } from '@inertiajs/core'; import { Head, Link, router, usePage, usePoll } from '@inertiajs/react'; import { ArrowUp } from 'lucide-react'; import { @@ -25,7 +25,7 @@ import AppLayout from '@/layouts/app-layout'; import { backgroundPollOptions } from '@/lib/background-poll'; import { emojify } from '@/lib/emoji'; import { cn } from '@/lib/utils'; -import type { Commit, CursorPaginated, Log, User } from '@/types'; +import type { Commit, CursorPaginated, Log } from '@/types'; export default function Show({ log, @@ -43,14 +43,10 @@ export default function Show({ } = usePage(); const user = auth.user; const [notice, setNotice] = useState<{ - newestId: string; - users: User[]; - total: number; - count: number; + commits: Commit[]; leaving: boolean; } | null>(null); const [stateLog, setStateLog] = useState(log); - const noticeTimers = useRef[]>([]); const loadRequest = useRef(null); const loadingNew = useRef(false); const freshIds = useRef>(new Set()); @@ -90,6 +86,22 @@ export default function Show({ return () => observer.disconnect(); }, []); + const preserveReadingPosition = useCallback((ids: Set) => { + const headerBottom = + tabsRef.current?.getBoundingClientRect().bottom ?? 0; + const rows = + feedRef.current?.querySelectorAll('[data-commit]') ?? + []; + const row = Array.from(rows).find( + (row) => + ids.has(row.dataset.commit ?? '') && + row.getBoundingClientRect().bottom > headerBottom, + ); + followAnchor.current = row + ? { row, top: row.getBoundingClientRect().top } + : null; + }, []); + useEffect( () => router.on('beforeUpdate', ({ detail: { page } }) => { @@ -123,23 +135,11 @@ export default function Show({ } } - const ids = new Set(incoming.data.map((commit) => commit.id)); - const headerBottom = - tabsRef.current?.getBoundingClientRect().bottom ?? 0; - const rows = - feedRef.current?.querySelectorAll( - '[data-commit]', - ) ?? []; - const row = Array.from(rows).find( - (row) => - ids.has(row.dataset.commit ?? '') && - row.getBoundingClientRect().bottom > headerBottom, + preserveReadingPosition( + new Set(incoming.data.map((commit) => commit.id)), ); - followAnchor.current = row - ? { row, top: row.getBoundingClientRect().top } - : null; }), - [log, commits], + [log, commits, preserveReadingPosition], ); useLayoutEffect(() => { @@ -282,20 +282,18 @@ export default function Show({ ); if (window.scrollY <= 0 || arrivals.length === 0) return; - const authors = [ - ...new Map( - arrivals.map((commit) => [ - commit.user.id, - commit.user, - ]), - ).values(), - ]; - noticeTimers.current.forEach(clearTimeout); setNotice((current) => ({ - newestId: arrivals[0].id, - users: authors.slice(0, 3), - total: authors.length, - count: (current?.count ?? 0) + arrivals.length, + commits: [ + ...arrivals, + ...(current?.leaving + ? [] + : (current?.commits ?? []) + ).filter((commit) => + incoming.data.some( + (row) => row.id === commit.id, + ), + ), + ], leaving: false, })); }, @@ -310,11 +308,45 @@ export default function Show({ const { stop: stopPolling } = usePoll( 30_000, backgroundPollOptions(() => ({ - only: ['newCommitsCount'], - data: { since: newestId }, + only: ['newCommitsCount', 'removedCommitIds'], + data: { + since: newestId, + pending: notice?.leaving + ? [] + : (notice?.commits.map((commit) => commit.id) ?? []), + }, preserveUrl: true, - onSuccess: (page) => - loadNewest(page.props.newCommitsCount as number), + onSuccess: (page) => { + const removed = new Set( + page.props.removedCommitIds as string[], + ); + if (removed.size > 0) { + setNotice((current) => { + if (!current || current.leaving) return current; + const remaining = current.commits.filter( + (commit) => !removed.has(commit.id), + ); + return remaining.length > 0 + ? { ...current, commits: remaining } + : { ...current, leaving: true }; + }); + router.replaceProp( + 'commits', + (value: unknown, props: PageProps) => { + if (props.log !== log) return value; + const current = value as CursorPaginated; + const retained = current.data.filter( + (commit) => !removed.has(commit.id), + ); + preserveReadingPosition( + new Set(retained.map((commit) => commit.id)), + ); + return { ...current, data: retained }; + }, + ); + } + loadNewest(page.props.newCommitsCount as number); + }, })), { autoStart: newestId !== undefined }, ); @@ -335,12 +367,17 @@ export default function Show({ ); const dismissNotice = useCallback(() => { - noticeTimers.current.forEach(clearTimeout); setNotice((current) => current && { ...current, leaving: true }); - noticeTimers.current = [setTimeout(() => setNotice(null), 450)]; }, []); - const noticeNewestId = notice?.newestId; + const noticeLeaving = notice?.leaving; + useEffect(() => { + if (!noticeLeaving) return; + const timer = setTimeout(() => setNotice(null), 450); + return () => clearTimeout(timer); + }, [noticeLeaving]); + + const noticeNewestId = notice?.commits[0]?.id; useEffect(() => { if (!noticeNewestId) return; const row = Array.from( @@ -363,7 +400,6 @@ export default function Show({ useEffect( () => () => { - noticeTimers.current.forEach(clearTimeout); loadRequest.current?.cancel(); }, [log], @@ -378,7 +414,12 @@ export default function Show({ return () => window.removeEventListener('scroll', update); }, [dismissNotice]); - const announcementLabel = `${notice?.count ?? 0} new ${notice?.count === 1 ? 'commit' : 'commits'} above. Scroll to top`; + const authors = [ + ...new Map( + notice?.commits.map((commit) => [commit.user.id, commit.user]), + ).values(), + ]; + const announcementLabel = `${notice?.commits.length ?? 0} new ${notice?.commits.length === 1 ? 'commit' : 'commits'} above. Scroll to top`; return ( <> @@ -438,24 +479,28 @@ export default function Show({ - {notice.users.map((author) => ( - + {[0, 1, 2].map((slot) => ( + + {authors[slot] && ( + + )} + ))} - {notice.total > 3 && ( + {authors.length > 3 && ( - {notice.total - 3 > 99 + {authors.length - 3 > 99 ? '99+' - : `+${notice.total - 3}`} + : `+${authors.length - 3}`} )} diff --git a/tests/Feature/Controllers/LogControllerTest.php b/tests/Feature/Controllers/LogControllerTest.php index a0a1b46ee..c14e3c9c9 100644 --- a/tests/Feature/Controllers/LogControllerTest.php +++ b/tests/Feature/Controllers/LogControllerTest.php @@ -809,3 +809,30 @@ ->assertJsonPath('props.newCommitsCount', 0) ->assertJsonMissingPath('props.newCommitAuthors'); }); + +it('reconciles pending arrivals even when no newer commits exist', function (Log $log): void { + $viewer = User::factory()->create(); + $author = User::factory()->create(); + $viewer->following()->attach($author); + $pending = Commit::factory()->for($author)->count(4)->create(); + $pending[0]->delete(); + $response = $this->actingAs($viewer)->get(route('log.show', [ + $log, 'since' => $pending[3]->id, 'pending' => $pending->modelKeys(), + ]), [ + 'X-Inertia' => 'true', + 'X-Inertia-Version' => (string) resolve(HandleInertiaRequests::class)->version(request()), + 'X-Inertia-Partial-Component' => 'log/show', + 'X-Inertia-Partial-Data' => 'newCommitsCount,removedCommitIds', + ]); + $response->assertOk()->assertJsonPath('props.newCommitsCount', 0) + ->assertJsonPath('props.removedCommitIds', [$pending[0]->id]) + ->assertJsonMissingPath('props.commits'); +})->with([Log::Following, Log::Latest]); + +it('rejects malformed pending commit identifiers', function (array $pending, string $field): void { + $this->actingAs(User::factory()->create())->getJson(route('log.show', [Log::Latest, ...$pending])) + ->assertUnprocessable()->assertJsonValidationErrors($field); +})->with([ + [['pending' => 'bad'], 'pending'], + [['pending' => ['bad']], 'pending.0'], +]); diff --git a/tests/Feature/LatestLogCommitsTest.php b/tests/Feature/LatestLogCommitsTest.php index 073ca0358..e3529d4a4 100644 --- a/tests/Feature/LatestLogCommitsTest.php +++ b/tests/Feature/LatestLogCommitsTest.php @@ -64,3 +64,25 @@ Commit::factory()->create(); expect($query->count(Log::Latest, null, $seen))->toBe(1); }); + +it('returns only requested identifiers no longer visible in the log', function (): void { + $viewer = User::factory()->create(); + $followed = User::factory()->create(); + $viewer->following()->attach($followed); + $visible = Commit::factory()->for($followed)->create(); + $deleted = Commit::factory()->for($followed)->create(); + $deleted->delete(); + + $stranger = Commit::factory()->create(); + $muted = User::factory()->create(); + $viewer->muting()->attach($muted); + $hidden = Commit::factory()->for($muted)->create(); + $reply = Commit::factory()->for($followed)->create(['parent_id' => $visible->id]); + $query = resolve(LatestLogCommitsQuery::class); + $ids = [$visible->id, $deleted->id, $stranger->id, $hidden->id, $reply->id]; + expect($query->missing(Log::Latest, $viewer, $ids))->toBe([$deleted->id, $hidden->id, $reply->id]); + expect($query->missing(Log::Following, $viewer, $ids))->toBe([$deleted->id, $stranger->id, $hidden->id, $reply->id]); + expect($query->missing(Log::Latest, null, [$visible->id, $deleted->id]))->toBe([$deleted->id]); + expect($query->missing(Log::Latest, $viewer, []))->toBe([]); + expect($query->missing(Log::Trending, $viewer, $ids))->toBe([]); +}); diff --git a/tests/Unit/LogInsertion.test.tsx b/tests/Unit/LogInsertion.test.tsx index d90b2df21..a26c1ada5 100644 --- a/tests/Unit/LogInsertion.test.tsx +++ b/tests/Unit/LogInsertion.test.tsx @@ -18,6 +18,7 @@ const listeners = new Map void>(); let viewer: User | null = null; const reload = vi.fn(); let pollOptions: () => ReloadOptions; +let replaceCommits: (value: (old: unknown, props: unknown) => unknown) => void; vi.doMock('react', () => ({ ...React, useRef: (initial: unknown) => (slots[cursor++] ??= { current: initial }), @@ -87,6 +88,10 @@ vi.doMock('@inertiajs/react', () => ({ return () => listeners.delete(name); }, reload, + replaceProp: ( + _name: string, + value: (old: unknown, props: unknown) => unknown, + ) => replaceCommits(value), flushAll: vi.fn(), }, })); @@ -237,9 +242,9 @@ function insertAt( }; const previous = commits(['existing']); const pending = render(previous); - const poll = () => + const poll = (count = total, removedCommitIds: string[] = []) => pollOptions().onSuccess!({ - props: { newCommitsCount: total }, + props: { newCommitsCount: count, removedCommitIds }, } as never); poll(); const fetching = render(previous); @@ -267,6 +272,18 @@ function insertAt( rows = [...fresh, existing]; render(incoming); let current = incoming; + replaceCommits = (value) => { + const next = value(current, { log }) as CursorPaginated; + const removed = current.data.length - next.data.length; + rows = rows.filter((row) => + next.data.some( + (c) => c.id === (row as typeof existing).dataset.commit, + ), + ); + rowPosition -= removed * 120; + current = next; + render(current); + }; const finish = (success = true, delivered = incoming) => { const request = reload.mock.calls.at(-1)![0]; if (success) request.onSuccess?.({ props: { commits: delivered } }); @@ -275,6 +292,9 @@ function insertAt( }; return { incoming, + data: () => current.data, + request: () => pollOptions(), + readingTop: () => existing.getBoundingClientRect().top, render: () => render(current), paginate: () => { current = { @@ -286,6 +306,7 @@ function insertAt( scrollTo: (top: number) => { scrollY = top; scrollListeners.forEach((listener) => listener()); + render(current); }, sight: (index: number) => { for (const observer of intersections) { @@ -297,6 +318,7 @@ function insertAt( { target: fresh[index], isIntersecting: true }, ]); } + render(current); }, poll, pending, @@ -525,3 +547,103 @@ test('keeps notice dismissal active after highlights settle and pagination appen false, ); }); + +for (const log of ['following', 'latest'] as const) { + test(`reconciles deleted announcements without new arrivals in ${log}`, () => { + const authors = Array.from({ length: 4 }, (_, i) => ({ + id: `author-${i}`, + name: `Author ${i}`, + avatar_url: null, + })) as User[]; + const result = insertAt(500, false, authors, log, 4); + result.finish(); + const requests = reload.mock.calls.length; + assert.deepEqual( + (result.request().data as { pending: string[] }).pending, + ['new-0', 'new-1', 'new-2', 'new-3'], + ); + const top = result.readingTop(); + result.poll(0, ['new-0']); + assert.equal(result.readingTop(), top); + const nodes = result.render(); + assert.equal( + nodes.find((n) => n.type === 'button')!.props['aria-label'], + '3 new commits above. Scroll to top', + ); + assert.deepEqual( + nodes + .filter((n) => n.props.user) + .map((n) => (n.props.user as User).id), + ['author-1', 'author-2', 'author-3'], + ); + assert.equal( + result.data().some((c) => c.id === 'new-0'), + false, + ); + assert.equal(reload.mock.calls.length, requests); + result.sight(1); + vi.advanceTimersByTime(450); + assert.equal( + result.render().some((n) => n.type === 'button'), + false, + ); + }); + test(`fades out after all pending commits are deleted in ${log}`, () => { + const result = insertAt(500, false, [], log); + const before = result.finish(); + result.poll(0, ['new-0', 'new-1', 'new-2']); + const fading = result.render(); + assert.match( + fading.find((n) => n.type === 'button')!.props.className as string, + /opacity-0/, + ); + assert.equal( + fading.filter((n) => n.props.user).length, + before.filter((n) => n.props.user).length, + ); + vi.advanceTimersByTime(450); + assert.equal( + result.render().some((n) => n.type === 'button'), + false, + ); + assert.deepEqual( + result.data().map((c) => c.id), + ['existing'], + ); + }); +} + +test('reconciles removals and new arrivals in the same polling cycle', () => { + const result = insertAt(500); + result.finish(); + result.poll(2, ['new-0']); + const request = reload.mock.calls.at(-1)![0]; + const delivered = { + data: [ + { + id: 'next-0', + user_id: 'next-author', + user: { id: 'next-author' }, + }, + { + id: 'next-1', + user_id: 'next-author', + user: { id: 'next-author' }, + }, + ...result.data(), + ], + } as CursorPaginated; + request.onSuccess({ props: { commits: delivered } }); + request.onFinish(); + const button = result.render().find((n) => n.type === 'button')!; + assert.equal( + button.props['aria-label'], + '4 new commits above. Scroll to top', + ); + assert.deepEqual((result.request().data as { pending: string[] }).pending, [ + 'next-0', + 'next-1', + 'new-1', + 'new-2', + ]); +});