From f9dabc6c39785b50ce9738968c76e03d3ab5347f Mon Sep 17 00:00:00 2001 From: Simon Hamp Date: Mon, 26 Oct 2026 12:41:29 +0000 Subject: [PATCH] Open replies from the replies button, five at a time The "N replies" button, where a thread can't open in place (a reply on a commit page, notifications and the like), now opens the commit with its replies already showing. "Show more" under a thread opened in a feed does the same. The app now asks devnull for five replies at a time, everywhere: the commit page, threads opened in a feed, and a deleted reply's replies. "Show more" loads the next five. A deleted reply's replies get their own "Show more", since a deleted commit has no page to send you to. A refresh used to drop the pages you'd loaded once there were none left to load. With five a page that would happen all the time, so pages loaded after the first now stay. A reply's page now shows the commit it replies to above it. Only the ones before that fold behind "N earlier commits". A part of a series shows the part before it the same way. Co-Authored-By: Claude Opus 5.5 --- app/Devnull/Client.php | 23 ++- app/NativeComponents/CommitDetail.php | 91 +++++++++--- .../Concerns/ExpandsThreads.php | 36 ++++- .../Concerns/ShowsCommits.php | 14 ++ .../views/native/commit-detail.blade.php | 29 ++-- .../native/partials/commit-actions.blade.php | 6 +- .../views/native/partials/feed.blade.php | 11 +- .../Feature/Screens/CommitDetailCacheTest.php | 3 +- tests/Feature/Screens/CommitDetailTest.php | 134 +++++++++++++++--- tests/Feature/Screens/LogTest.php | 20 +++ tests/Feature/Screens/ReplyPreviewTest.php | 13 +- tests/Feature/Screens/SeriesTest.php | 10 +- 12 files changed, 309 insertions(+), 81 deletions(-) diff --git a/app/Devnull/Client.php b/app/Devnull/Client.php index b7edc13..9d53ee9 100644 --- a/app/Devnull/Client.php +++ b/app/Devnull/Client.php @@ -91,6 +91,12 @@ */ public const string IssuesPath = 'issues'; + /** + * How many replies the app asks for at a time. devnull sends 50 a page + * unless it's asked for fewer, and the app never asks for more than this. + */ + public const int RepliesPerPage = 5; + public function __construct(private DeviceKey $deviceKey, private string $baseUrl, private ResponseCache $cache) {} /** @@ -530,7 +536,7 @@ public static function notificationsQuery(string $type, ?string $cursor = null): */ public function signedGet(string $path, array $query = []): SignedRequest { - $target = $this->target($path, $query); + $target = $this->target($path, self::paged($path, $query)); return new SignedRequest($this->origin().$target, $this->sign('GET', $target, '')); } @@ -575,7 +581,7 @@ public static function sendSigned(SignedRequest $request): Response */ private function get(string $path, array $query = []): array { - $json = $this->json($this->send('GET', $path, $query)); + $json = $this->json($this->send('GET', $path, self::paged($path, $query))); $this->cache->put($path, $query, $json); ImageCache::prefetch($json); @@ -690,6 +696,19 @@ private function sign(string $method, string $target, string $body, string $name ->headers($method, $target, $body, CarbonImmutable::now()->getTimestamp(), RequestSigner::nonce(), $namespace); } + /** + * A GET's query with the page size the app asks for: a page of replies + * holds RepliesPerPage. The size isn't part of what the answer's kept + * under, as every page of replies has it. + * + * @param array $query + * @return array + */ + private static function paged(string $path, array $query): array + { + return preg_match('#^commits/[^/]+/replies$#', $path) === 1 ? [...$query, 'limit' => (string) self::RepliesPerPage] : $query; + } + /** * The request target: the path as the server sees it, plus the query string. * diff --git a/app/NativeComponents/CommitDetail.php b/app/NativeComponents/CommitDetail.php index 9f1b9c7..399a786 100644 --- a/app/NativeComponents/CommitDetail.php +++ b/app/NativeComponents/CommitDetail.php @@ -3,6 +3,7 @@ namespace App\NativeComponents; use App\Devnull\Account; +use App\Devnull\Client; use App\Devnull\Data\Commit; use App\Devnull\Data\CommitThread; use App\Devnull\Data\Page; @@ -29,8 +30,12 @@ * parts below, with its replies between, as on the web. Deleted ancestors, * parts and replies show as tombstones. * - * The commit opens at the top, with what it replies to behind "↑ N earlier - * commits" and its replies behind "› N replies", each opened in place. + * The commit opens near the top, under the commit it replies to, with the + * ones before that behind "↑ N earlier commits" and its replies behind + * "› N replies", each opened in place. Opened from a "› N replies" button + * ($data['replies']), its replies already show. They come five at a time + * (see Client::RepliesPerPage), and "Show more" under the last asks for the + * next five, as it does under a deleted reply's replies opened in place. * * It opens on the commit as it was last time, or opened from a row, on what * the row knew of it, then asks devnull for the rest behind it and fills it in. @@ -91,6 +96,13 @@ class CommitDetail extends NativeComponent */ public array $tombstoneReplies = []; + /** + * Where the next page of each deleted reply's replies starts, by the deleted reply's id, or null when they're all shown. + * + * @var array + */ + public array $tombstoneCursors = []; + public string $reply = ''; /** @@ -99,7 +111,7 @@ class CommitDetail extends NativeComponent public bool $replyRestored = false; /** - * Whether the ancestors show above the commit, rather than "↑ N earlier commits". + * Whether the ancestors before the commit's parent show above it, rather than "↑ N earlier commits". */ public bool $earlierShown = false; @@ -136,6 +148,7 @@ class CommitDetail extends NativeComponent public function mount(): void { $this->focusReply = $this->data('reply', false) === true; + $this->repliesShown = $this->data('replies', false) === true; $this->reply = app(Stash::class)->get($this->me()?->id, $this->replyStashName()); $this->replyRestored = $this->reply !== ''; $this->showKept(); @@ -193,7 +206,11 @@ protected function revalidate(): void $this->refresh('commits/'.rawurlencode($id).'/replies', [], function (array $json) use ($id): void { if (array_key_exists($id, $this->tombstoneReplies)) { - $this->tombstoneReplies[$id] = Page::fromArray($json, Commit::fromArray(...))->items; + [$this->tombstoneReplies[$id], $this->tombstoneCursors[$id]] = self::withFirstPage( + $this->tombstoneReplies[$id], + $this->tombstoneCursors[$id] ?? null, + Page::fromArray($json, Commit::fromArray(...)), + ); } }); } @@ -213,8 +230,9 @@ public function showEarlier(): void } /** - * Tapping into the reply bar brings the commit being answered to the top: - * the earlier commits fold away again and the page starts over from the top. + * Tapping into the reply bar brings the commit being answered back up, under + * its parent: the earlier commits fold away again and the page starts over + * from the top. */ public function replyTapped(): void { @@ -236,7 +254,7 @@ public function toggleReplies(): void public function toggleTombstone(string $id): void { if (array_key_exists($id, $this->tombstoneReplies)) { - unset($this->tombstoneReplies[$id]); + unset($this->tombstoneReplies[$id], $this->tombstoneCursors[$id]); return; } @@ -244,6 +262,25 @@ public function toggleTombstone(string $id): void $this->loadTombstoneReplies($id); } + /** + * "Show more" under a deleted reply's replies: its next five, in place. + */ + public function loadMoreTombstoneReplies(string $id): void + { + $cursor = $this->tombstoneCursors[$id] ?? null; + + if ($cursor === null) { + return; + } + + $page = $this->attempt(fn ($devnull) => $devnull->replies($id, $cursor)); + + if ($page !== null && array_key_exists($id, $this->tombstoneReplies)) { + $this->tombstoneReplies[$id] = [...$this->tombstoneReplies[$id], ...$page->items]; + $this->tombstoneCursors[$id] = $page->nextCursor; + } + } + public function loadMoreReplies(): void { if ($this->repliesCursor === null) { @@ -503,19 +540,35 @@ private function takeThread(array $json): void */ private function takeReplies(array $json): void { - $page = Page::fromArray($json, Commit::fromArray(...)); - $fresh = array_flip(array_map(fn (Commit $reply): string => $reply->id, $page->items)); - - $later = $this->repliesCursor !== null - ? array_values(array_filter(array_slice($this->replies, count($page->items)), fn (Commit $reply): bool => ! isset($fresh[$reply->id]))) - : []; - - $this->replies = [...$page->items, ...$later]; - $this->repliesCursor = $later === [] ? $page->nextCursor : $this->repliesCursor; + [$this->replies, $this->repliesCursor] = self::withFirstPage($this->replies, $this->repliesCursor, Page::fromArray($json, Commit::fromArray(...))); $this->repliesLoaded = true; $this->keepStagedReplies(); } + /** + * A fresh first page over the replies showing, and where the next page + * starts. Pages loaded after the first stay, past what it holds: there's + * a page after it still to load, or more show than a page holds. Staged + * replies are left for keepStagedReplies() to put back. + * + * @param list $shown + * @param Page $page + * @return array{0: list, 1: string|null} + */ + private static function withFirstPage(array $shown, ?string $cursor, Page $page): array + { + $shown = array_values(array_filter($shown, fn (Commit $reply): bool => ! $reply->isStaged())); + + if ($cursor === null && count($shown) <= Client::RepliesPerPage) { + return [$page->items, $page->nextCursor]; + } + + $fresh = array_flip(array_map(fn (Commit $reply): string => $reply->id, $page->items)); + $later = array_values(array_filter(array_slice($shown, count($page->items)), fn (Commit $reply): bool => ! isset($fresh[$reply->id]))); + + return [[...$page->items, ...$later], $later === [] ? $page->nextCursor : $cursor]; + } + private function replyStashName(): string { return 'reply-'.$this->id; @@ -532,6 +585,7 @@ private function loadTombstoneReplies(string $id): void if ($page !== null) { $this->tombstoneReplies[$id] = $page->items; + $this->tombstoneCursors[$id] = $page->nextCursor; } } @@ -542,7 +596,8 @@ public function isOwn(): bool public function render(): View { - $earlier = $this->earlierShown ? [] : $this->ancestors; + $earlier = $this->earlierShown ? [] : array_slice($this->ancestors, 0, -1); + $above = $this->earlierShown ? $this->ancestors : array_slice($this->ancestors, -1); $viewer = $this->me(); $this->replyRows ??= new RowCache; $this->replyRows->startFrame(); @@ -554,7 +609,7 @@ public function render(): View 'stashed' => app(Stash::class)->names($viewer?->id), 'pasteis' => app(Account::class)->pasteis(), 'rows' => $this->commit === null ? [] : Timeline::thread( - [...($this->earlierShown ? $this->ancestors : []), $this->commit, ...$this->series], + [...$above, $this->commit, ...$this->series], laneAbove: $earlier === [] ? null : end($earlier)->lane(), ), 'earlierCount' => $this->commit === null ? 0 : count($earlier), diff --git a/app/NativeComponents/Concerns/ExpandsThreads.php b/app/NativeComponents/Concerns/ExpandsThreads.php index 2e4bc79..2a5763c 100644 --- a/app/NativeComponents/Concerns/ExpandsThreads.php +++ b/app/NativeComponents/Concerns/ExpandsThreads.php @@ -10,10 +10,14 @@ * the web's thread toggle does, and "Hide replies" closes them again. One * level deep: a reply's own replies open its page. * + * The app asks for replies oldest first, five at a time (see + * Client::RepliesPerPage). A thread opened in place shows its first five, + * and "Show more" under them opens the rest on the commit's page, with its + * replies showing. + * * Under a commit devnull sends with its newest reply, "Show more" opens the - * rest in place, newest first as on the web. The API only pages replies - * oldest first, 50 at a time, so they're turned round here, and a thread - * longer than a page opens the commit instead. + * rest in place, newest first as on the web, so they're turned round here, + * and a thread longer than a page opens the commit's page instead. * * A thread opened before opens at once from the cache, and refreshes behind. */ @@ -33,10 +37,17 @@ trait ExpandsThreads */ public array $newestFirst = []; + /** + * Threads opened in place with more replies than they show. + * + * @var list + */ + public array $threadsWithMore = []; + public function toggleReplies(string $id): void { if (array_key_exists($id, $this->threads)) { - unset($this->threads[$id]); + $this->closeThread($id); return; } @@ -113,22 +124,27 @@ private function openThread(string $id, Page $page): void { // A thread whose last reply went closes, so its commit gets its hairline back. if ($page->items === []) { - unset($this->threads[$id]); + $this->closeThread($id); return; } if (! in_array($id, $this->newestFirst, true)) { $this->threads[$id] = $page->items; + $this->threadsWithMore = array_values(array_diff($this->threadsWithMore, [$id])); + + if ($page->hasMore()) { + $this->threadsWithMore[] = $id; + } return; } // More than a page: the newest replies aren't on it, so the commit's page shows them. if ($page->hasMore()) { - unset($this->threads[$id]); + $this->closeThread($id); $this->newestFirst = array_values(array_diff($this->newestFirst, [$id])); - $this->openCommit($id); + $this->openReplies($id); return; } @@ -136,6 +152,12 @@ private function openThread(string $id, Page $page): void $this->threads[$id] = array_reverse($page->items); } + private function closeThread(string $id): void + { + unset($this->threads[$id]); + $this->threadsWithMore = array_values(array_diff($this->threadsWithMore, [$id])); + } + /** * @return list */ diff --git a/app/NativeComponents/Concerns/ShowsCommits.php b/app/NativeComponents/Concerns/ShowsCommits.php index 0689240..bd0727a 100644 --- a/app/NativeComponents/Concerns/ShowsCommits.php +++ b/app/NativeComponents/Concerns/ShowsCommits.php @@ -119,6 +119,20 @@ public function openCommit(string $id): void $this->openInTab('/commits/'.$id, $commit === null ? [] : ['commit' => $commit]); } + /** + * "› N replies" where they can't open in place: the commit's page, with its replies already showing. + */ + public function openReplies(string $id): void + { + $commit = $this->shownCommit($id); + + if ($commit?->isStaged() === true) { + return; + } + + $this->openInTab('/commits/'.$id, ['replies' => true, ...($commit === null ? [] : ['commit' => $commit])]); + } + /** * Opens a profile with what a row already knows of its author. */ diff --git a/resources/views/native/commit-detail.blade.php b/resources/views/native/commit-detail.blade.php index 422f2dc..abcf20a 100644 --- a/resources/views/native/commit-detail.blade.php +++ b/resources/views/native/commit-detail.blade.php @@ -12,14 +12,15 @@ @endif {{-- - The commit is the main thing, at the top: larger, in full ink, with its git date and its replies behind - a toggle in its action row. What it replies to waits behind "↑ N earlier commits" above it, since EDGE - can't open the page scrolled down to it. Opened, the ancestors, the commit and the rest of its series - share one rail, so someone replying to themselves shows their avatar once. The replies hang under it - on the thread's dashed lines, with the rail ending in a dot above them. A part of a series with more - parts after it has its replies right under it, beside the rail running on, and the later parts follow. - Everything around the commit is muted, and still opens its own page. A deleted reply's own replies - open in place, one level in, since a deleted commit has no page. + The commit is the main thing, near the top: larger, in full ink, with its git date and its replies behind + a toggle in its action row. The commit it replies to shows above it, and what that replies to waits + behind "↑ N earlier commits", since EDGE can't open the page scrolled down to the commit. Opened, the + ancestors, the commit and the rest of its series share one rail, so someone replying to themselves shows + their avatar once. The replies hang under it on the thread's dashed lines, with the rail ending in a dot + above them, five at a time with "Show more" under the last. A part of a series with more parts after it + has its replies right under it, beside the rail running on, and the later parts follow. Everything + around the commit is muted, and still opens its own page. A deleted reply's own replies open in place, + one level in, five at a time too, since a deleted commit has no page. --}} @if ($earlierCount > 0) @include('native.partials.earlier-commits', ['count' => $earlierCount, 'lane' => $earlierLane]) @@ -48,6 +49,7 @@ @php($railLane = $loop->last ? null : $commit->lane()) @foreach ($replies as $index => $threadReply) @php($nested = $tombstoneReplies[$threadReply->id] ?? []) + @php($nestedMore = ($tombstoneCursors[$threadReply->id] ?? null) !== null) @php($hasNext = ! $loop->last || $repliesCursor !== null) @php($replyRow = [ 'reply' => $threadReply, @@ -68,13 +70,18 @@ 'reply' => $nestedReply, 'lane' => \App\Devnull\Lane::Rail, 'previousLane' => $nestedIndex === 0 ? null : $nested[$nestedIndex - 1]->lane(), - 'hasNext' => ! $loop->last, + 'hasNext' => ! $loop->last || $nestedMore, 'railLane' => $railLane, 'indent' => 32, 'spine' => $hasNext ? $threadReply->lane() : null, 'muted' => true, - 'thread' => $commit, - 'more' => $loop->last && $loop->parent->last && $repliesCursor !== null ? 'loadMoreReplies' : null, + 'thread' => $nestedMore ? $threadReply : $commit, + 'more' => match (true) { + ! $loop->last => null, + $nestedMore => "loadMoreTombstoneReplies('{$threadReply->id}')", + $loop->parent->last && $repliesCursor !== null => 'loadMoreReplies', + default => null, + }, 'last' => $loop->last && $loop->parent->last, ]) @php($rowScope = array_diff_key(get_defined_vars(), ['__data' => 1, '__path' => 1])) diff --git a/resources/views/native/partials/commit-actions.blade.php b/resources/views/native/partials/commit-actions.blade.php index 4b48566..82213a6 100644 --- a/resources/views/native/partials/commit-actions.blade.php +++ b/resources/views/native/partials/commit-actions.blade.php @@ -7,8 +7,8 @@ on Android, see CommitMenu for what's in it). Tapping More only opens the menu. Reply opens the commit with its reply composer focused. In a feed ($expandable) the replies link opens the thread in - place, as on the web; elsewhere it opens the commit. Like the web, someone else's commit that isn't approved gets no - star or Quote. Your own commits can be starred too, including ones waiting for approval. + place, as on the web; elsewhere it opens the commit with its replies showing. Like the web, someone else's commit + that isn't approved gets no star or Quote. Your own commits can be starred too, including ones waiting for approval. Quote opens a sheet to write the quote in, over the commit being quoted (see commit-sheet). @@ -38,7 +38,7 @@ @php($compact ??= false) @php($expandable ??= isset($threads)) @php($isOpen = $repliesOpen ?? ($expandable && array_key_exists($commit->id, $threads))) -@php($repliesPress = $repliesToggle ?? ($expandable ? "toggleReplies('{$commit->id}')" : "openCommit('{$commit->id}')")) +@php($repliesPress = $repliesToggle ?? ($expandable ? "toggleReplies('{$commit->id}')" : "openReplies('{$commit->id}')")) @php($repliesCount ??= $commit->repliesCount) @php($iconSize = $compact ? 14 : 16) @php($labelSize = $compact ? 'text-xs' : 'text-[13px]') diff --git a/resources/views/native/partials/feed.blade.php b/resources/views/native/partials/feed.blade.php index f9923cc..27a7bd4 100644 --- a/resources/views/native/partials/feed.blade.php +++ b/resources/views/native/partials/feed.blade.php @@ -1,8 +1,8 @@ {{-- A feed's rows from App\Devnull\Timeline: day markers, the last-visit marker and commits on the rail. Under a commit hangs its thread, inside its row's hairline, as on the web: the newest reply devnull previews, with "Show more" - when there are others, or the replies opened in place. The commit's rail runs on past them when its author's run - goes on below. + when there are others, or the replies opened in place, with "Show more" to the commit's page when there are more + than they show. The commit's rail runs on past them when its author's run goes on below. Given a $rowCache (see App\Devnull\RowCache), commits and replies that haven't changed since the last frame are drawn from it rather than built again. Given a $rowWindow (see App\Devnull\RowWindow), rows far from the screen @@ -35,7 +35,10 @@ $opened = $threads[$feedCommit->id] ?? null; $threadReplies = $opened ?? ($feedCommit->isDeleted() ? [] : $feedCommit->replyPreview?->replies ?? []); $feedDraws = $rowWindow?->place($feedIndex, 1 + count($threadReplies)) ?? true; - $hasMoreReplies = $opened === null && ! $feedCommit->isDeleted() && ($feedCommit->replyPreview?->hasMore ?? false); + $hasMoreReplies = $opened === null + ? ! $feedCommit->isDeleted() && ($feedCommit->replyPreview?->hasMore ?? false) + : in_array($feedCommit->id, $threadsWithMore ?? [], true); + $morePress = $opened === null ? "showMoreReplies('{$feedCommit->id}')" : "openReplies('{$feedCommit->id}')"; $railLane = $row->continuesNext ? $feedCommit->lane() : null; $commitRow = [ 'commit' => $feedCommit, @@ -66,7 +69,7 @@ 'hasNext' => ! $loop->last || $hasMoreReplies, 'railLane' => $railLane, 'thread' => $feedCommit, - 'more' => $loop->last && $hasMoreReplies ? "showMoreReplies('{$feedCommit->id}')" : null, + 'more' => $loop->last && $hasMoreReplies ? $morePress : null, 'last' => $loop->last, ]; @endphp diff --git a/tests/Feature/Screens/CommitDetailCacheTest.php b/tests/Feature/Screens/CommitDetailCacheTest.php index b19704d..b5b8e60 100644 --- a/tests/Feature/Screens/CommitDetailCacheTest.php +++ b/tests/Feature/Screens/CommitDetailCacheTest.php @@ -74,7 +74,6 @@ function serveThread(): void finishRefreshes($commit) ->assertSee('A compiler') ->assertMissingElement('row', fn (array $node): bool => ($node['ref'] ?? null) === 'loading-replies') - ->tap('earlier-commits') ->assertSee('The commit it replies to'); }); @@ -144,6 +143,6 @@ function serveThread(): void $screen->call('load'); - Http::assertSent(fn (Request $request): bool => $request->url() === 'http://devnull.test/api/v1/commits/d1/replies'); + Http::assertSent(fn (Request $request): bool => $request->url() === 'http://devnull.test/api/v1/commits/d1/replies?limit=5'); $screen->assertSee('Under a deleted reply'); }); diff --git a/tests/Feature/Screens/CommitDetailTest.php b/tests/Feature/Screens/CommitDetailTest.php index b341b6e..94c4fda 100644 --- a/tests/Feature/Screens/CommitDetailTest.php +++ b/tests/Feature/Screens/CommitDetailTest.php @@ -23,59 +23,64 @@ function fakeThread(array $commit = [], array $replies = [], array $extra = []): fakeApi([ 'GET /api/v1/commits/c2' => [commitPageJson( commitJson(['id' => 'c2', 'parent_id' => 'c1', 'body' => 'The focused commit', ...$commit]), - [commitJson(['id' => 'c1', 'body' => 'The parent commit', 'author' => userJson(['id' => 'u2', 'username' => 'ada', 'name' => 'Ada'])])], + [ + commitJson(['id' => 'c0', 'body' => 'The root commit', 'author' => userJson(['id' => 'u3', 'username' => 'grace', 'name' => 'Grace'])]), + commitJson(['id' => 'c1', 'parent_id' => 'c0', 'body' => 'The parent commit', 'author' => userJson(['id' => 'u2', 'username' => 'ada', 'name' => 'Ada'])]), + ], )], 'GET /api/v1/commits/c2/replies' => [pageJson($replies)], ...$extra, ]); } -it('opens on the commit, with its ancestors and replies folded away above and below it', function () { +it('opens on the commit under its parent, with the older ancestors and the replies folded away', function () { fakeThread(['replies_count' => 1], replies: [replyJson(['id' => 'r1', 'parent_id' => 'c2', 'body' => 'First reply', 'author' => userJson(['username' => 'grace', 'name' => 'Grace'])])]); $screen = Native::visit('/log/commits/c2') ->assertNavTitle('Commit') ->assertSee('The focused commit') + ->assertSee('The parent commit') ->assertSee('1 earlier commit') ->assertSee('1 reply') - ->assertDontSee('The parent commit') + ->assertDontSee('The root commit') ->assertDontSee('First reply') ->assertSee('Reply to @local_dev'); - // The commit comes first, after only the row standing in for what it replies to. - expect(childRefs($screen, 'thread-page'))->toBe(['earlier-commits', 'focused-commit', 'reply-bar-clearance']); + // The commit comes after the one it replies to, and the row standing in for what that replies to. + expect(childRefs($screen, 'thread-page'))->toBe(['earlier-commits', 'commit-c1', 'focused-commit', 'reply-bar-clearance']); $screen->tap('earlier-commits') - ->assertSee('The parent commit') + ->assertSee('The root commit') ->assertDontSee('1 earlier commit'); - expect(childRefs($screen, 'thread-page'))->toBe(['commit-c1', 'focused-commit', 'reply-bar-clearance']); + expect(childRefs($screen, 'thread-page'))->toBe(['commit-c0', 'commit-c1', 'focused-commit', 'reply-bar-clearance']); $screen->tap('replies-c2') ->assertSee('First reply') ->assertSee('Hide replies'); - expect(childRefs($screen, 'thread-page'))->toBe(['commit-c1', 'focused-commit', 'thread-reply-r1', 'reply-bar-clearance']); + expect(childRefs($screen, 'thread-page'))->toBe(['commit-c0', 'commit-c1', 'focused-commit', 'thread-reply-r1', 'reply-bar-clearance']); $screen->tap('replies-c2') ->assertDontSee('First reply') ->assertSee('1 reply'); }); -it('brings the commit back to the top when you tap into the reply bar', function () { +it('brings the commit back up under its parent when you tap into the reply bar', function () { fakeThread(['replies_count' => 1], replies: [replyJson(['id' => 'r1', 'parent_id' => 'c2', 'body' => 'First reply', 'author' => userJson(['username' => 'grace', 'name' => 'Grace'])])]); - $screen = Native::visit('/log/commits/c2')->tap('earlier-commits')->assertSee('The parent commit'); + $screen = Native::visit('/log/commits/c2')->tap('earlier-commits')->assertSee('The root commit'); $before = nodeByRef($screen, 'thread-scroll')['id']; - // EDGE can't scroll from PHP, so the ancestors fold away and the scroll view mounts again at the top. + // EDGE can't scroll from PHP, so the older ancestors fold away and the scroll view mounts again at the top. $screen->pressUp('reply-field') ->assertSet('earlierShown', false) ->assertSet('scrollKey', 1) ->assertSee('1 earlier commit') - ->assertDontSee('The parent commit'); + ->assertSee('The parent commit') + ->assertDontSee('The root commit'); - expect(childRefs($screen, 'thread-page'))->toBe(['earlier-commits', 'focused-commit', 'reply-bar-clearance']) + expect(childRefs($screen, 'thread-page'))->toBe(['earlier-commits', 'commit-c1', 'focused-commit', 'reply-bar-clearance']) ->and(nodeByRef($screen, 'thread-scroll')['id'])->not->toBe($before); }); @@ -86,7 +91,7 @@ function fakeThread(array $commit = [], array $replies = [], array $extra = []): $field = idsDownTo($screen->tree(), 'reply-input'); // Tapping into the bar, which folds the earlier commits away above it. - $screen->pressUp('reply-field')->assertDontSee('The parent commit'); + $screen->pressUp('reply-field')->assertDontSee('The root commit'); expect(idsDownTo($screen->tree(), 'reply-input'))->toBe($field); // More of the thread drawn above it. @@ -99,12 +104,15 @@ function fakeThread(array $commit = [], array $replies = [], array $extra = []): ->and(idsDownTo($screen->tree(), 'reply-input'))->toBe($field); }); -it('runs the rail from the folded ancestors into the commit, which shows its avatar', function () { +it('runs the rail from the folded ancestors into the parent, which shows its avatar', function () { $ada = userJson(['id' => 'u2', 'username' => 'ada', 'name' => 'Ada', 'lane' => 1]); fakeApi([ 'GET /api/v1/commits/c2' => [commitPageJson( commitJson(['id' => 'c2', 'parent_id' => 'c1', 'body' => 'Ada again', 'author' => $ada]), - [commitJson(['id' => 'c1', 'body' => 'Ada first', 'author' => $ada])], + [ + commitJson(['id' => 'c0', 'body' => 'Ada first', 'author' => $ada]), + commitJson(['id' => 'c1', 'parent_id' => 'c0', 'body' => 'Ada second', 'author' => $ada]), + ], )], 'GET /api/v1/commits/c2/replies' => [pageJson([])], ]); @@ -113,9 +121,10 @@ function fakeThread(array $commit = [], array $replies = [], array $extra = []): $lane = config('native-ui.theme.light.lane-1'); $segment = fn (array $node): bool => ($node['layout']['width'] ?? null) === 2.0 && ($node['style']['bg_color'] ?? null) === $lane; - expect(railNode($screen, 'focused-commit'))->toBe('avatar') + expect(railNode($screen, 'commit-c1'))->toBe('avatar') + ->and(railNode($screen, 'focused-commit'))->toBe('dot') ->and(findNode(nodeByRef($screen, 'earlier-commits'), $segment))->not->toBeNull() - ->and(findNode(nodeByRef($screen, 'focused-commit'), $segment))->not->toBeNull(); + ->and(findNode(nodeByRef($screen, 'commit-c1'), $segment))->not->toBeNull(); }); it('draws the commit as the main thing, and what is around it muted', function () { @@ -375,6 +384,82 @@ function fakeThread(array $commit = [], array $replies = [], array $extra = []): Native::visit('/log/commits/c2')->call('openProfile', 'ada')->assertNavigatedTo('/log/users/ada'); }); +it('opens with its replies showing from a replies button', function () { + fakeThread(['replies_count' => 1], replies: [replyJson(['id' => 'r1', 'parent_id' => 'c2', 'body' => 'First reply', 'replies_count' => 1])], extra: [ + 'GET /api/v1/commits/r1' => [commitPageJson(replyJson(['id' => 'r1', 'parent_id' => 'c2', 'body' => 'First reply', 'replies_count' => 1]))], + 'GET /api/v1/commits/r1/replies' => [pageJson([replyJson(['id' => 'r2', 'parent_id' => 'r1', 'body' => 'Answering the reply'])])], + ]); + + Native::visit('/log/commits/c2', ['replies' => true]) + ->assertSee('First reply') + ->assertSee('Hide replies'); + + // A reply's replies don't open in place on a commit page, so its button opens its page with them showing. + Native::visit('/log/commits/c2') + ->tap('replies-c2') + ->tap('replies-r1') + ->assertNavigatedTo('/log/commits/r1') + ->follow() + ->assertSee('Answering the reply') + ->assertSee('Hide replies'); +}); + +it('asks for the replies five at a time, and the next five from Show more', function () { + $replies = fn (int ...$numbers): array => array_map(fn (int $n): array => replyJson(['id' => "r{$n}", 'parent_id' => 'c2', 'body' => "Reply {$n}"]), $numbers); + + Http::fake(fn (Request $request) => match (parse_url($request->url(), PHP_URL_PATH)) { + '/api/v1/commits/c2/replies' => Http::response(str_contains($request->url(), 'cursor=page2') + ? pageJson($replies(6)) + : pageJson($replies(1, 2, 3, 4, 5), 'page2', 'http://devnull.test/api/v1/commits/c2/replies')), + default => Http::response(commitPageJson(commitJson(['id' => 'c2', 'body' => 'The focused commit', 'replies_count' => 6]))), + }); + + $screen = Native::visit('/log/commits/c2', ['replies' => true]) + ->assertSee('Reply 5') + ->assertDontSee('Reply 6') + ->tap('more-replies-c2') + ->assertSee('Reply 6') + ->assertDontSee('Show more'); + + $asked = collect(Http::recorded())->map(fn (array $sent): string => $sent[0]->url())->filter(fn (string $url): bool => str_contains($url, '/replies')); + + expect($asked)->toHaveCount(2) + ->and($asked->every(fn (string $url): bool => str_contains($url, 'limit=5')))->toBeTrue(); + + // Asking again for the first five keeps the one loaded after them. + $screen->call('load') + ->assertSee('Reply 6') + ->assertDontSee('Show more'); +}); + +it('opens the replies of a deleted reply five at a time too', function () { + $nested = fn (int ...$numbers): array => array_map(fn (int $n): array => replyJson(['id' => "n{$n}", 'parent_id' => 'r1', 'body' => "Nested {$n}"]), $numbers); + + Http::fake(fn (Request $request) => match (parse_url($request->url(), PHP_URL_PATH)) { + '/api/v1/commits/c2/replies' => Http::response(pageJson([ + tombstoneJson(['id' => 'r1', 'parent_id' => 'c2', 'replies_count' => 6]), + replyJson(['id' => 'r2', 'parent_id' => 'c2', 'body' => 'Still here']), + ])), + '/api/v1/commits/r1/replies' => Http::response(str_contains($request->url(), 'cursor=page2') + ? pageJson($nested(6)) + : pageJson($nested(1, 2, 3, 4, 5), 'page2', 'http://devnull.test/api/v1/commits/r1/replies')), + default => Http::response(commitPageJson(commitJson(['id' => 'c2', 'body' => 'The focused commit', 'replies_count' => 2]))), + }); + + $more = fn (array $node): bool => ($node['ref'] ?? null) === 'more-replies-r1'; + + Native::visit('/log/commits/c2', ['replies' => true]) + ->tap('tombstone-replies-r1') + ->assertSee('Nested 5') + ->assertDontSee('Nested 6') + ->assertSee('Still here') + ->tap('more-replies-r1') + ->assertSee('Nested 6') + ->assertMissingElement('pressable', $more); + + Http::assertSent(fn (Request $request): bool => $request->url() === 'http://devnull.test/api/v1/commits/r1/replies?cursor=page2&limit=5'); +}); + it('keeps deleted ancestors on the rail as tombstones', function () { fakeApi([ 'GET /api/v1/commits/c2' => [commitPageJson( @@ -390,7 +475,10 @@ function fakeThread(array $commit = [], array $replies = [], array $extra = []): $rail = config('native-ui.theme.light.rail'); $screen = Native::visit('/log/commits/c2') - ->assertSee('2 earlier commits') + // The deleted parent shows above the commit, and only the root waits behind the fold. + ->assertSee('1 earlier commit') + ->assertSee('This commit was deleted.') + ->assertDontSee('The root commit') ->tap('earlier-commits') ->assertSee('The root commit') ->assertSee('[deleted]') @@ -573,10 +661,12 @@ function fakeThread(array $commit = [], array $replies = [], array $extra = []): ]); $screen = Native::visit('/log/commits/c3') - ->assertSee('And one more thing'); + ->assertSee('And one more thing') + ->assertSee('Replying to Ada'); - // Folded away, the run above has no avatar to share, so the commit shows its own. - expect(railNode($screen, 'focused-commit'))->toBe('avatar'); + // The parent shows, and it's yours, so the two share its avatar. + expect(railNode($screen, 'commit-c2'))->toBe('avatar') + ->and(railNode($screen, 'focused-commit'))->toBe('dot'); $screen->tap('earlier-commits'); diff --git a/tests/Feature/Screens/LogTest.php b/tests/Feature/Screens/LogTest.php index 490dec8..5aeef7f 100644 --- a/tests/Feature/Screens/LogTest.php +++ b/tests/Feature/Screens/LogTest.php @@ -536,6 +536,26 @@ Native::visit('/log')->tap('replies-c2')->tap('thread-reply-r1')->assertNavigatedTo('/log/commits/r1'); }); +it('opens five replies of a thread in place, and the rest on its commit page from Show more', function () { + fakeApi([ + 'GET /api/v1/log/following' => [pageJson([commitJson(['id' => 'c2', 'replies_count' => 7])])], + 'GET /api/v1/commits/c2/replies' => [pageJson(array_map(fn (int $n): array => replyJson(['id' => "r{$n}", 'parent_id' => 'c2', 'body' => "Reply {$n}"]), range(1, 5)), 'page2', 'http://devnull.test/api/v1/commits/c2/replies')], + 'GET /api/v1/commits/c2' => [commitPageJson(commitJson(['id' => 'c2', 'replies_count' => 7]))], + ]); + + Native::visit('/log', platform: 'ios') + ->tap('replies-c2') + ->assertNoNavigation() + ->assertSee('Reply 5') + ->tap('more-replies-c2') + ->assertNavigatedTo('/log/commits/c2') + ->follow() + ->assertSet('repliesShown', true) + ->assertSee('Reply 5'); + + Http::assertSent(fn (Request $request): bool => $request->url() === 'http://devnull.test/api/v1/commits/c2/replies?limit=5'); +}); + it('closes a thread whose replies have all gone', function () { $repliesGone = false; diff --git a/tests/Feature/Screens/ReplyPreviewTest.php b/tests/Feature/Screens/ReplyPreviewTest.php index c6a7f3d..57e1def 100644 --- a/tests/Feature/Screens/ReplyPreviewTest.php +++ b/tests/Feature/Screens/ReplyPreviewTest.php @@ -120,7 +120,7 @@ function dashColours(array $node): array expect(dashColours(nodeByRef($screen, 'thread-reply-r2')))->toContain(config('native-ui.theme.light.lane-4')) ->and(findNode(nodeByRef($screen, 'thread-reply-r2'), fn (array $node): bool => ($node['ref'] ?? null) === 'thread-elbow'))->toBeNull(); - Http::assertSent(fn (Request $request): bool => $request->url() === 'http://devnull.test/api/v1/commits/c1/replies'); + Http::assertSent(fn (Request $request): bool => $request->url() === 'http://devnull.test/api/v1/commits/c1/replies?limit=5'); }); it('opens Show more at once from the thread kept last time, and refreshes it behind', function () { @@ -138,13 +138,18 @@ function dashColours(array $node): array ->and(refreshesSent($screen))->toBeGreaterThan(1); }); -it('opens the commit instead when its thread is longer than a page', function () { +it('opens the commit with its replies showing instead when its thread is longer than a page', function () { fakeApi([ 'GET /api/v1/log/following' => [pageJson([previewedJson([replyJson(['id' => 'r60', 'parent_id' => 'c1'])], more: true)])], - 'GET /api/v1/commits/c1/replies' => [pageJson(array_map(fn (int $n): array => replyJson(['id' => "r{$n}", 'parent_id' => 'c1']), range(1, 50)), 'page2')], + 'GET /api/v1/commits/c1/replies' => [pageJson(array_map(fn (int $n): array => replyJson(['id' => "r{$n}", 'parent_id' => 'c1']), range(1, 5)), 'page2')], + 'GET /api/v1/commits/c1' => [commitPageJson(previewedJson([], more: true))], ]); - Native::visit('/log', platform: 'ios')->tap('more-replies-c1')->assertNavigatedTo('/log/commits/c1'); + Native::visit('/log', platform: 'ios') + ->tap('more-replies-c1') + ->assertNavigatedTo('/log/commits/c1') + ->follow() + ->assertSet('repliesShown', true); }); it('stars a reply in the thread in place', function () { diff --git a/tests/Feature/Screens/SeriesTest.php b/tests/Feature/Screens/SeriesTest.php index 8d54a3a..04fa0ce 100644 --- a/tests/Feature/Screens/SeriesTest.php +++ b/tests/Feature/Screens/SeriesTest.php @@ -111,14 +111,8 @@ function railRunsOn(TestableComponent $screen, string $ref): bool ]); $screen = Native::visit('/log/commits/p2') - // The earlier part waits above, the later one follows. - ->assertSee('1 earlier commit') - ->assertDontSee('Part 1 of the thread') - ->assertSee('Part 3 of the thread'); - - expect(threadRows($screen))->toBe(['focused-commit', 'commit-p3']); - - $screen->tap('earlier-commits') + // The part before it shows above, as the commit it replies to, and the later one follows. + ->assertDontSee('earlier commit') ->assertSee('Part 1 of the thread') ->assertSee('Part 2 of the thread') ->assertSee('Part 3 of the thread')