From fcf414fa8e25a3c7d8900a98a5616c8b6f5ae584 Mon Sep 17 00:00:00 2001 From: Hosmel Quintana Date: Mon, 19 Oct 2026 21:37:59 -0600 Subject: [PATCH 1/2] fix(navigation): open log tabs instantly with safe prefetching Keep log GET requests read-only and persist the last displayed feed separately. Show the destination tab immediately with the existing feed skeletons while preserving cached responses. --- .ai/guidelines/resources.js.blade.php | 2 +- app/Http/Controllers/LogController.php | 23 +- app/Http/Controllers/UserLogController.php | 25 ++ app/Http/Middleware/HandleInertiaRequests.php | 5 +- app/Http/Requests/UpdateUserLogRequest.php | 28 ++ resources/js/components/page-tabs.tsx | 83 ++++-- resources/js/hooks/use-remember-log.ts | 131 ++++++++++ resources/js/layouts/app-layout.tsx | 2 + resources/js/lib/follow-requests.ts | 6 +- resources/js/lib/log-preference.ts | 64 +++++ resources/js/pages/log/show.tsx | 243 ++++++++++++------ routes/web.php | 3 + .../Feature/Controllers/LogControllerTest.php | 42 +-- .../Controllers/UserLogControllerTest.php | 42 +++ tests/Feature/LiveUpdatesTest.php | 24 ++ tests/Unit/FollowRequests.test.tsx | 12 + tests/Unit/LogInsertion.test.tsx | 10 + tests/Unit/LogPreference.test.ts | 107 ++++++++ tests/Unit/RememberLog.test.tsx | 212 +++++++++++++++ 19 files changed, 918 insertions(+), 146 deletions(-) create mode 100644 app/Http/Controllers/UserLogController.php create mode 100644 app/Http/Requests/UpdateUserLogRequest.php create mode 100644 resources/js/hooks/use-remember-log.ts create mode 100644 resources/js/lib/log-preference.ts create mode 100644 tests/Feature/Controllers/UserLogControllerTest.php create mode 100644 tests/Unit/LogPreference.test.ts create mode 100644 tests/Unit/RememberLog.test.tsx diff --git a/.ai/guidelines/resources.js.blade.php b/.ai/guidelines/resources.js.blade.php index 295ce20c95..3c57155662 100644 --- a/.ai/guidelines/resources.js.blade.php +++ b/.ai/guidelines/resources.js.blade.php @@ -16,7 +16,7 @@ ## Navigation and requests -- Every Inertia `` to a GET page prefetches on hover (`prefetch`). A GET page never writes; the only exceptions are `log/{log}` (remembers the log), `notifications` (marks read, remembers the filter), `remotes/@{partner}` and `remotes/orgs/{org}` (mark read), and every link to them keeps `prefetch={false}`. The navigation's Log is the exception: it opens the remembered log and a prefetch never changes the remembered log, so it prefetches everywhere except on a log page, where it scrolls to the top instead. `starred` also keeps it, because starring changes the list. A new GET page that must write joins this list. +- Every Inertia `` to a GET page prefetches on hover (`prefetch`). A GET page never writes; the only exceptions are `notifications` (marks read, remembers the filter), `remotes/@{partner}` and `remotes/orgs/{org}` (mark read), and every link to them keeps `prefetch={false}`. `starred` also keeps it, because starring changes the list. A new GET page that must write joins this list. The navigation's Log prefetches everywhere except on a log page, where it scrolls to the top instead. - Never prefetch a non-GET link, an external URL, a guest's sign-in link, a mention of a handle nobody here has (guests get a missing page, and a feed would ask for it on every hover), or a URL that is not an Inertia page (images, feeds, downloads, JSON). - A click target that is not a link (the commit card surface) prefetches on `mousedown`, never on hover: scrolling a feed hovers every card. - After an action that changes what a prefetched page shows, flush that page (`router.flush(url)`, `router.flushAll()`, or `invalidateCacheTags`), so the next click does not show the old state. diff --git a/app/Http/Controllers/LogController.php b/app/Http/Controllers/LogController.php index f5979a93f3..38e452a910 100644 --- a/app/Http/Controllers/LogController.php +++ b/app/Http/Controllers/LogController.php @@ -4,7 +4,6 @@ namespace App\Http\Controllers; -use App\Actions\UpdateUserLog; use App\Enums\Log; use App\Http\Requests\ShowLogRequest; use App\Http\Resources\ShownStreams; @@ -25,9 +24,9 @@ /** * Guests land straight on the Trending log, with the home page's link preview. */ - public function index(ShowLogRequest $request, SuggestedCommitsQuery $suggested, LatestLogCommitsQuery $latest, LogCommitsQuery $commits, UpdateUserLog $action, PageMetadata $metadata): Response|SymfonyResponse + public function index(ShowLogRequest $request, SuggestedCommitsQuery $suggested, LatestLogCommitsQuery $latest, LogCommitsQuery $commits, PageMetadata $metadata): Response|SymfonyResponse { - $response = $this->show($request, null, Log::Trending, $suggested, $latest, $commits, $action); + $response = $this->show($request, null, Log::Trending, $suggested, $latest, $commits); return $response instanceof Response ? $response->withViewData(['preview' => $metadata->home()]) : $response; } @@ -35,20 +34,16 @@ public function index(ShowLogRequest $request, SuggestedCommitsQuery $suggested, /** * Guests read Trending and Latest, while Following takes them to sign in with GitHub. */ - public function show(ShowLogRequest $request, #[CurrentUser] ?User $user, Log $log, SuggestedCommitsQuery $suggested, LatestLogCommitsQuery $latest, LogCommitsQuery $commits, UpdateUserLog $action): Response|SymfonyResponse + public function show(ShowLogRequest $request, #[CurrentUser] ?User $user, Log $log, SuggestedCommitsQuery $suggested, LatestLogCommitsQuery $latest, LogCommitsQuery $commits): Response|SymfonyResponse { - if (! $user instanceof User) { - if ($log === Log::Following) { - if (app()->isLocal()) { - Inertia::flash('localSignIn', true); + if (! $user instanceof User && $log === Log::Following) { + if (app()->isLocal()) { + Inertia::flash('localSignIn', true); - return to_route('home'); - } - - return Inertia::location(route('login')); + return to_route('home'); } - } elseif ($user->log !== $log && ! $request->prefetch()) { - $action->handle($user, $log); + + return Inertia::location(route('login')); } return Inertia::render('log/show', [ diff --git a/app/Http/Controllers/UserLogController.php b/app/Http/Controllers/UserLogController.php new file mode 100644 index 0000000000..1df10d0d62 --- /dev/null +++ b/app/Http/Controllers/UserLogController.php @@ -0,0 +1,25 @@ +log !== $request->log()) { + $action->handle($user, $request->log()); + } + + return response()->noContent(); + } +} diff --git a/app/Http/Middleware/HandleInertiaRequests.php b/app/Http/Middleware/HandleInertiaRequests.php index ffe4a8cc35..56ff566b86 100644 --- a/app/Http/Middleware/HandleInertiaRequests.php +++ b/app/Http/Middleware/HandleInertiaRequests.php @@ -108,6 +108,9 @@ public function share(Request $request): array ? Inertia::live($value, on: array_values(array_map(fn (UserUpdate $update): string => $update->value, $updates)), channel: new PrivateChannel($user)) : $value; + // Kept across pages, so a page prefetched before the reader moved to another log doesn't bring the old one back. + $log = Inertia::once(fn (): ?Log => $request->user()?->log)->as('auth.log:'.($user->id ?? 'guest')); + // Kept across pages until the first stream it lists runs out at most, so a stream extended meanwhile is read again. $liveFollowing = $user instanceof User && $hasAside ? Inertia::once(function () use (&$liveFollowing, $user): array { @@ -145,7 +148,7 @@ public function share(Request $request): array 'translationLanguage' => $live(fn (): string => resolve(CommitTranslations::class)->language(), UserUpdate::Account), 'mustReadCommitRules' => $live(fn (): bool => $user instanceof User && $user->commit_rules_accepted_at === null, UserUpdate::Account), 'termsToAccept' => $live(fn (): ?string => $user instanceof User && ! $user->hasAcceptedCurrentTerms() ? config()->string('legal.terms_version') : null, UserUpdate::Account), - 'log' => $live(fn (): ?Log => $request->user()?->log, UserUpdate::Account), + 'log' => $user instanceof User ? $log->live(UserUpdate::Account->value, new PrivateChannel($user)) : $log, // Each browser filters notifications its own way, and reloading the count goes through its cookie. 'unreadNotificationsCount' => $live(fn (): int => $user instanceof User ? resolve(NotificationGroupsQuery::class)->unreadCount($user, ListNotificationsRequest::rememberedType($request, $user)) : 0, UserUpdate::Notifications), 'unreadMessagesCount' => $live(fn (): int => $user?->remotes_enabled ? resolve(ConversationsQuery::class)->unreadCount($user) : 0, UserUpdate::Messages, UserUpdate::Inbox), diff --git a/app/Http/Requests/UpdateUserLogRequest.php b/app/Http/Requests/UpdateUserLogRequest.php new file mode 100644 index 0000000000..a19b4a2a91 --- /dev/null +++ b/app/Http/Requests/UpdateUserLogRequest.php @@ -0,0 +1,28 @@ +> + */ + public function rules(): array + { + return [ + 'log' => ['required', Rule::enum(Log::class)], + ]; + } + + public function log(): Log + { + return Log::from($this->string('log')->value()); + } +} diff --git a/resources/js/components/page-tabs.tsx b/resources/js/components/page-tabs.tsx index 0f033f894d..cbb8bdd70a 100644 --- a/resources/js/components/page-tabs.tsx +++ b/resources/js/components/page-tabs.tsx @@ -1,7 +1,24 @@ -import { Link } from '@inertiajs/react'; +import { Link, router } from '@inertiajs/react'; import type { InertiaLinkProps } from '@inertiajs/react'; +import { useState } from 'react'; import { cn } from '@/lib/utils'; +type Tab = { + title: string; + /** + * Shown instead of the title on phones, where a long title would wrap. + */ + shortTitle?: string; + href: NonNullable; + isActive: boolean; + onClick?: InertiaLinkProps['onClick']; + prefetch?: boolean; + component?: InertiaLinkProps['component']; + pageProps?: InertiaLinkProps['pageProps']; + onHttpException?: InertiaLinkProps['onHttpException']; + onNetworkError?: InertiaLinkProps['onNetworkError']; +}; + export function PageTabs({ label, tabs, @@ -17,41 +34,64 @@ export function PageTabs({ prefetch?: boolean; only?: InertiaLinkProps['only']; reset?: string[]; - tabs: { - title: string; - /** - * Shown instead of the title on phones, where a long title would wrap. - */ - shortTitle?: string; - href: NonNullable; - isActive: boolean; - onClick?: InertiaLinkProps['onClick']; - }[]; + tabs: Tab[]; }) { + const [pending, setPending] = useState(null); + const active = pending ?? tabs.find((tab) => tab.isActive)?.title; + const finish = (title: string) => { + setPending((current) => (current === title ? null : current)); + }; + const headers = reset?.length + ? { 'X-Inertia-Reset': reset.join(',') } + : undefined; const indicatorName = `tab-indicator-${label.toLowerCase().replaceAll(/[^a-z0-9]+/gu, '-')}`; return ( -