diff --git a/app/Devnull/CommitMenu.php b/app/Devnull/CommitMenu.php index d566caf..fec7e70 100644 --- a/app/Devnull/CommitMenu.php +++ b/app/Devnull/CommitMenu.php @@ -10,9 +10,8 @@ /** * What More holds, on the right of a commit's action row (see - * partials/commit-actions): Share link on every commit; Pin to profile or - * Unpin from profile on your own published commit, not a reply; then, after a - * divider, Report commit on someone else's published commit, or Delete commit + * partials/commit-actions): Pin to profile or Unpin from profile on your own + * published commit, not a reply; then, after a divider, Report commit on someone else's published commit, or Delete commit * on your own. Each item names the commit, so a tap never rebuilds anything * from an attribute (see ShowsCommits). */ @@ -26,17 +25,17 @@ public static function items(Commit $commit, ?User $viewer): array $isOwn = $viewer !== null && $commit->author->id === $viewer->id; $items = []; - $items[] = self::item($commit, 'share', 'Share link', Lucide::Link, 'shareCommit'); - if ($isOwn && $commit->isApproved() && $commit->parentId === null) { $items[] = self::item($commit, 'pin', $commit->isPinned ? 'Unpin from profile' : 'Pin to profile', Lucide::Pin, 'togglePin'); } if ($viewer !== null && ! $isOwn && $commit->isApproved()) { - $items[] = NavAction::divider(); $items[] = self::item($commit, 'report', 'Report commit', Lucide::Flag, 'reportCommit'); } elseif ($isOwn) { - $items[] = NavAction::divider(); + if ($items !== []) { + $items[] = NavAction::divider(); + } + $items[] = self::item($commit, 'delete', 'Delete commit', Lucide::Trash2, 'confirmDelete', destructive: true); } diff --git a/app/Icons/Lucide.php b/app/Icons/Lucide.php index e209092..18be07f 100644 --- a/app/Icons/Lucide.php +++ b/app/Icons/Lucide.php @@ -27,6 +27,7 @@ enum Lucide case Settings2; case MessageSquareReply; case Link; + case Share; case Star; case Bookmark; case Trash2; @@ -136,6 +137,7 @@ public function custom(bool $filled = false): ?string self::Settings2 => 'settings-2', self::MessageSquareReply => 'message-square-reply', self::Link => 'link', + self::Share => 'share', self::Star => $filled ? 'star-filled' : 'star', self::Bookmark => $filled ? 'bookmark-filled' : 'bookmark', self::Trash2 => 'trash-2', diff --git a/app/NativeComponents/Layouts/TabsLayout.php b/app/NativeComponents/Layouts/TabsLayout.php index 86dbb5e..558f213 100644 --- a/app/NativeComponents/Layouts/TabsLayout.php +++ b/app/NativeComponents/Layouts/TabsLayout.php @@ -62,7 +62,7 @@ public function navBar(NativeComponent $screen): ?NavBar ->displayMode('inline'); if ($screen instanceof Log) { - $navBar->titleView(Image::make(self::logo())->height(28)->alt('devnull')); + $navBar->titleView(Image::make(self::logo())->width(105)->height(28)->fit(1)->alt('devnull')); } return $navBar; diff --git a/resources/icons/share.svg b/resources/icons/share.svg new file mode 100644 index 0000000..85de1a7 --- /dev/null +++ b/resources/icons/share.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/resources/views/native/partials/commit-actions.blade.php b/resources/views/native/partials/commit-actions.blade.php index 4b48566..814a0d6 100644 --- a/resources/views/native/partials/commit-actions.blade.php +++ b/resources/views/native/partials/commit-actions.blade.php @@ -3,8 +3,9 @@ and its count, Quote, Reply, and in Commit Blue "› N replies" when the row has no thread under it already. On the right: More. - Share link, Report and Delete live in More, which opens the platform's own menu (a UIMenu on iOS, a dropdown - on Android, see CommitMenu for what's in it). Tapping More only opens the menu. + Share is its own button at the end of the row. Pin, Report and Delete live in More, which opens the platform's own + menu (a UIMenu on iOS, a dropdown on Android, see CommitMenu for what's in it), and is left off when there's + nothing 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 @@ -54,10 +55,11 @@ @php($canGivePastel = $pasteis !== null && $viewer !== null && ! $isOwn && $commit->isApproved()) @php($pastelLabel = \App\Devnull\Display::count($commit->pasteisCount, 'pastel', 'pastéis')) @php($pastelSize = $compact ? 18 : 20) +@php($menu = \App\Devnull\CommitMenu::items($commit, $viewer)) @php($starLabel = ($commit->starred ? 'Unstar commit' : 'Star commit').($commit->starsCount > 0 ? ', '.\App\Devnull\Display::count($commit->starsCount, 'star', 'stars') : '')) @if ($canReact && $viewer !== null && ! $commit->isDeleted()) - + @if ($commit->starsCount > 0) {{ \App\Devnull\Display::compactCount($commit->starsCount) }} @@ -66,19 +68,19 @@ @endif @if ($canGivePastel) - + {{ \App\Devnull\Display::compactCount($commit->pasteisCount) }} @elseif ($pasteis !== null) - + {{ \App\Devnull\Display::compactCount($commit->pasteisCount) }} @endif @if ($withQuote && $canReact) - + @if ($hasQuoteStash) @@ -87,11 +89,8 @@ @endif @if ($withReply) - + - @unless ($showsReplies) - Reply - @endunless @if ($hasReplyStash) @endif @@ -99,7 +98,7 @@ @endif @if ($showsReplies) - + @if ($isOpen) Hide replies @@ -110,11 +109,17 @@ @endif - + @unless ($commit->isDeleted()) + + + + @endunless - - - - - + @if ($menu !== []) + + + + + + @endif diff --git a/resources/views/native/partials/commit-row-content.blade.php b/resources/views/native/partials/commit-row-content.blade.php index 7b7a015..5c82910 100644 --- a/resources/views/native/partials/commit-row-content.blade.php +++ b/resources/views/native/partials/commit-row-content.blade.php @@ -12,7 +12,7 @@ smaller and in muted ink, with a smaller avatar and action row. They still open their own pages. A list builds and measures every node of a row as it scrolls in, so the content is one column: the rail's 64pt - gutter is its left margin, and the hairline under it is its bottom border. A border draws inside the box, so the + gutter is its left padding, so the hairline under it, its bottom border, runs the row's full width. A border draws inside the box, so the bottom padding takes the hairline's 1pt to keep the row as tall as it was. --}} @php($author = $commit->author) @@ -29,7 +29,7 @@ @php($inset = $focused ? 16 : 12) @php($padding = 'pt-['.$inset.']'.($threadBelow ? '' : ' pb-['.($hasHairline ? $inset + 1 : $inset).']')) @php($metaSize = $muted ? 'text-xs' : 'text-[13px]') - + {{ $author->name }} {{ $author->handle() }} diff --git a/resources/views/native/partials/thread-reply.blade.php b/resources/views/native/partials/thread-reply.blade.php index 06a90d8..16126fc 100644 --- a/resources/views/native/partials/thread-reply.blade.php +++ b/resources/views/native/partials/thread-reply.blade.php @@ -26,7 +26,7 @@ @php($avatarLine = 76 + $indent) @if ($reply->isDeleted()) @php($opened = $tombstones && array_key_exists($reply->id, $tombstoneReplies ?? [])) - + @@ -50,9 +50,6 @@ @if ($more !== null) @include('native.partials.thread-more', ['commit' => $thread ?? $reply]) @endif - @if ($last && ! $muted) - - @endif @include('native.partials.thread-links') @if ($opened && ($tombstoneReplies[$reply->id] ?? []) !== []) @@ -62,7 +59,7 @@ @else @php($commit = $reply) - + @@ -87,9 +84,6 @@ @if ($more !== null) @include('native.partials.thread-more', ['commit' => $thread ?? $reply]) @endif - @if ($last && ! $muted) - - @endif @include('native.partials.thread-links') diff --git a/tests/Feature/Screens/CommitCardTest.php b/tests/Feature/Screens/CommitCardTest.php index 8d1dfae..b7f36e8 100644 --- a/tests/Feature/Screens/CommitCardTest.php +++ b/tests/Feature/Screens/CommitCardTest.php @@ -101,10 +101,13 @@ function logOf(array $commits): TestableComponent ? array_values(array_filter($node['children'], fn (array $child): bool => $child['type'] !== 'top_bar_action'))[0]['layout'] : $node['layout']; - foreach (['star-c1', 'quote-c1', 'more-c1'] as $ref) { + // Star, Quote and Reply share one target size and centre what they hold, so the row reads evenly. + foreach (['star-c1', 'quote-c1', 'reply-to-c1'] as $ref) { expect($target($ref))->toMatchArray(['min_width' => 44.0, 'height' => 36.0]); } + expect($target('more-c1'))->toMatchArray(['min_width' => 44.0, 'height' => 36.0]); + expect($target('reply-to-c1')['height'])->toBe(36.0) ->and($target('replies-c1')['height'])->toBe(36.0) // 6pt more above and below than the 24pt the row used to be, so it's pulled in by that much. @@ -119,7 +122,7 @@ function logOf(array $commits): TestableComponent // One column beside the rail, its gutter a margin and the hairline under it a border, and buttons that are their // own rows. Wrapped the old way, this row took 47 views in eight levels. expect($row['layout']['flex_direction'])->toBe(1) - ->and($content['layout']['margin'])->toBe([0.0, 0.0, 0.0, 64.0]) + ->and($content['layout']['padding'][3])->toBe(64.0) ->and($content['props']['border_b'])->toBe(1.0) ->and(array_column(nodeByRef($screen, 'star-c1')['children'], 'type'))->toBe(['icon', 'text']) ->and(count(findNodes($row, fn (): bool => true)))->toBeLessThanOrEqual(35); diff --git a/tests/Feature/Screens/CommitMenuTest.php b/tests/Feature/Screens/CommitMenuTest.php index 9bb8750..343a637 100644 --- a/tests/Feature/Screens/CommitMenuTest.php +++ b/tests/Feature/Screens/CommitMenuTest.php @@ -27,26 +27,22 @@ expect($more['props']['has_menu'] ?? null)->toBeTrue() ->and($more['on_press'] ?? $more['props']['on_press'] ?? null)->toBeNull(); - // Someone else's published commit: share it or report it. + // Someone else's published commit: report it. Share is a button of its own in the row. expect(menuItems($screen, 'more-c4'))->toBe([ - ['share-c4', 'Share link', 'custom:link'], - 'divider', ['report-c4', 'Report commit', 'custom:flag'], ]) - // Theirs, waiting for approval: only its link, as the web leaves the rest off. - ->and(menuItems($screen, 'more-c3'))->toBe([['share-c3', 'Share link', 'custom:link']]); + // Theirs, waiting for approval: nothing to put in More, so it isn't drawn. + ->and(findNode($screen->tree(), fn (array $node): bool => ($node['ref'] ?? null) === 'more-c3'))->toBeNull() + ->and(nodeByRef($screen, 'share-c3')['type'])->toBe('pressable'); - // Your own published one: share it, pin it to your profile, or delete it. + // Your own published one: pin it to your profile, or delete it. expect(menuItems($screen, 'more-c2'))->toBe([ - ['share-c2', 'Share link', 'custom:link'], ['pin-c2', 'Pin to profile', $platform === 'ios' ? 'pin' : 'push_pin'], 'divider', ['delete-c2', 'Delete commit', 'custom:trash-2'], ]) // Yours, waiting for approval: the same, but it can't be pinned yet. ->and(menuItems($screen, 'more-c1'))->toBe([ - ['share-c1', 'Share link', 'custom:link'], - 'divider', ['delete-c1', 'Delete commit', 'custom:trash-2'], ]); diff --git a/tests/Feature/Screens/CommitSheetTest.php b/tests/Feature/Screens/CommitSheetTest.php index 5bd1372..42d0473 100644 --- a/tests/Feature/Screens/CommitSheetTest.php +++ b/tests/Feature/Screens/CommitSheetTest.php @@ -64,13 +64,12 @@ function reportFormOnLog(array $reportAnswer = [null, 204]): TestableComponent $screen = Native::visit('/log', platform: 'ios'); - // Someone else's published commit: after a divider, the web's Report. - expect(array_slice(menuItems($screen, 'more-c3'), -2))->toBe(['divider', ['report-c3', 'Report commit', 'custom:flag']]); + // Someone else's published commit: the web's Report. + expect(menuItems($screen, 'more-c3'))->toBe([['report-c3', 'Report commit', 'custom:flag']]); - // Your own commit, and someone else's that isn't published, can't be reported. - foreach (['c2', 'c1'] as $id) { - expect(array_column(array_filter(menuItems($screen, 'more-'.$id), is_array(...)), 0))->not->toContain('report-'.$id); - } + // Your own commit can't be reported, and someone else's that isn't published has no More at all. + expect(array_column(array_filter(menuItems($screen, 'more-c2'), is_array(...)), 0))->not->toContain('report-c2') + ->and(findNode($screen->tree(), fn (array $node): bool => ($node['ref'] ?? null) === 'more-c1'))->toBeNull(); // The sheet is there, waiting, but down. expect(sheetProps($screen)['visible'])->toBeFalse(); diff --git a/tests/Feature/Screens/DraftStashTest.php b/tests/Feature/Screens/DraftStashTest.php index b1fc756..abfbd1f 100644 --- a/tests/Feature/Screens/DraftStashTest.php +++ b/tests/Feature/Screens/DraftStashTest.php @@ -271,12 +271,12 @@ function stashLabel(TestableComponent $screen, string $ref): array $screen = Native::visit('/log', platform: 'ios'); // The dots sit inside the buttons, so the row has the same actions as without them. - expect(array_values(array_filter(childRefs($screen, 'actions-c2'))))->toBe(['star-c2', 'pastel-c2', 'quote-c2', 'reply-to-c2', 'replies-c2', 'more-c2']) - ->and(array_values(array_filter(childRefs($screen, 'actions-c1'))))->toBe(['star-c1', 'pastel-c1', 'quote-c1', 'reply-to-c1', 'more-c1']) + expect(array_values(array_filter(childRefs($screen, 'actions-c2'))))->toBe(['star-c2', 'pastel-c2', 'quote-c2', 'reply-to-c2', 'replies-c2', 'share-c2', 'more-c2']) + ->and(array_values(array_filter(childRefs($screen, 'actions-c1'))))->toBe(['star-c1', 'pastel-c1', 'quote-c1', 'reply-to-c1', 'share-c1', 'more-c1']) ->and(showsRef($screen, 'quote-stash-c2'))->toBeTrue() ->and(showsRef($screen, 'reply-stash-c2'))->toBeTrue() ->and(showsRef($screen, 'reply-stash-c1'))->toBeTrue() - ->and(findNode(nodeByRef($screen, 'reply-to-c1'), fn (array $node): bool => ($node['type'] ?? null) === 'text')['props']['text'] ?? null)->toBe('Reply'); + ->and(findNode(nodeByRef($screen, 'reply-to-c1'), fn (array $node): bool => ($node['type'] ?? null) === 'text'))->toBeNull(); }); // ── Signing out ──────────────────────────────── diff --git a/tests/Feature/Screens/LogTest.php b/tests/Feature/Screens/LogTest.php index b8f164a..7c6058b 100644 --- a/tests/Feature/Screens/LogTest.php +++ b/tests/Feature/Screens/LogTest.php @@ -373,10 +373,10 @@ // Someone else's pending commit can't be starred or quoted, so the web leaves them off. ->assertMissingElement('pressable', fn (array $node): bool => in_array($node['ref'] ?? null, ['star-c0', 'quote-c0'], true)); - // The web's order: star, Quote, Reply and replies on the left, then More for Share link and Report. - expect(array_values(array_filter(childRefs($screen, 'actions-c2'))))->toBe(['star-c2', 'quote-c2', 'reply-to-c2', 'replies-c2', 'more-c2']) - // Nor can it be reported, so all its menu has is the link. - ->and(menuItems($screen, 'more-c0'))->toBe([['share-c0', 'Share link', 'custom:link']]); + // The web's order: star, Quote, Reply and replies on the left, then Share, then More for Pin, Report and Delete. + expect(array_values(array_filter(childRefs($screen, 'actions-c2'))))->toBe(['star-c2', 'quote-c2', 'reply-to-c2', 'replies-c2', 'share-c2', 'more-c2']) + // Nor can it be reported, so it has no menu to open. + ->and(findNode($screen->tree(), fn (array $node): bool => ($node['ref'] ?? null) === 'more-c0'))->toBeNull(); $icon = fn (string $ref): ?array => findNode(nodeByRef($screen, $ref), fn (array $node): bool => ($node['type'] ?? null) === 'icon'); $text = fn (string $ref, string $text): ?array => findNode(nodeByRef($screen, $ref), fn (array $node): bool => ($node['props']['text'] ?? null) === $text); @@ -473,7 +473,6 @@ $screen = Native::visit('/log', platform: 'ios'); expect(menuItems($screen, 'more-c1'))->toBe([ - ['share-c1', 'Share link', 'custom:link'], ['pin-c1', 'Pin to profile', 'pin'], 'divider', ['delete-c1', 'Delete commit', 'custom:trash-2'], diff --git a/tests/Feature/Screens/PasteisTest.php b/tests/Feature/Screens/PasteisTest.php index 4217158..df2e283 100644 --- a/tests/Feature/Screens/PasteisTest.php +++ b/tests/Feature/Screens/PasteisTest.php @@ -85,7 +85,7 @@ function textInside(TestableComponent $screen, string $ref): ?string $screen = Native::visit('/log', platform: 'ios')->assertSee('3 replies'); - expect(array_values(array_filter(childRefs($screen, 'actions-c2'))))->toBe(['star-c2', 'quote-c2', 'reply-to-c2', 'replies-c2', 'more-c2']) + expect(array_values(array_filter(childRefs($screen, 'actions-c2'))))->toBe(['star-c2', 'quote-c2', 'reply-to-c2', 'replies-c2', 'share-c2', 'more-c2']) ->and(findNode($screen->tree(), fn (array $node): bool => str_starts_with((string) ($node['ref'] ?? ''), 'pastel')))->toBeNull() ->and(findNode($screen->tree(), fn (array $node): bool => ($node['props']['src'] ?? null) === Pasteis::Image))->toBeNull(); @@ -103,7 +103,7 @@ function textInside(TestableComponent $screen, string $ref): ?string $screen = Native::visit('/log', platform: 'ios'); - expect(array_values(array_filter(childRefs($screen, 'actions-c2'))))->toBe(['star-c2', 'pastel-c2', 'quote-c2', 'reply-to-c2', 'replies-c2', 'more-c2']) + expect(array_values(array_filter(childRefs($screen, 'actions-c2'))))->toBe(['star-c2', 'pastel-c2', 'quote-c2', 'reply-to-c2', 'replies-c2', 'share-c2', 'more-c2']) ->and(nodeByRef($screen, 'pastel-c2')['props']['a11y_label'])->toBe('Give a pastel, 3 pastéis, 7 left today') ->and(nodeByRef($screen, 'pastel-c1')['props']['a11y_label'])->toBe('Give a pastel, 0 pastéis, 7 left today') // The web shows the count even when it's 0. @@ -128,9 +128,9 @@ function textInside(TestableComponent $screen, string $ref): ?string $screen = Native::visit('/log', platform: 'ios'); $pastel = $pasteisMode ? ['pastel-c1'] : []; - // Star, the pastel in pastéis mode, Quote, Reply with its word, then More: no bookmark or link in the row. - expect(array_values(array_filter(childRefs($screen, 'actions-c1'))))->toBe(['star-c1', ...$pastel, 'quote-c1', 'reply-to-c1', 'more-c1']) - ->and(textInside($screen, 'reply-to-c1'))->toBe('Reply') + // Star, the pastel in pastéis mode, Quote, Reply (icons, no words), then More: no bookmark or link in the row. + expect(array_values(array_filter(childRefs($screen, 'actions-c1'))))->toBe(['star-c1', ...$pastel, 'quote-c1', 'reply-to-c1', 'share-c1', 'more-c1']) + ->and(nodeByRef($screen, 'reply-to-c1')['props']['a11y_label'])->toBe('Reply') ->and(textInside($screen, 'replies-c2'))->toBe('3 replies') ->and(nodeByRef($screen, 'replies-c2')['props']['a11y_label'])->toBe('3 replies'); diff --git a/tests/Feature/Screens/ProfileTest.php b/tests/Feature/Screens/ProfileTest.php index 94f319e..2c07d3d 100644 --- a/tests/Feature/Screens/ProfileTest.php +++ b/tests/Feature/Screens/ProfileTest.php @@ -71,7 +71,7 @@ function adaCommitJson(array $overrides = []): array expect(nodeByRef($screen, 'reply-to-c1')['props']['a11y_label'])->toBe('Reply') ->and($labels('reply-to-c1'))->toBe([]) - ->and($labels('reply-to-c2'))->toHaveCount(1); + ->and($labels('reply-to-c2'))->toBe([]); }); it('lists what GitHub says about them under the bio, as the web does', function () { diff --git a/tests/Feature/Screens/ReplyPreviewTest.php b/tests/Feature/Screens/ReplyPreviewTest.php index c6a7f3d..20b3d8a 100644 --- a/tests/Feature/Screens/ReplyPreviewTest.php +++ b/tests/Feature/Screens/ReplyPreviewTest.php @@ -202,5 +202,6 @@ function dashColours(array $node): array $screen = Native::visit('/log', platform: 'ios'); - expect(findNode($screen->tree(), fn (array $node): bool => ($node['ref'] ?? null) === 'thread-end-r9'))->not->toBeNull(); + // The hairline is the border of the reply's own row, so it runs under the rail's gutter too. + expect(nodeByRef($screen, 'thread-reply-r9')['props']['border_b'])->toBe(1.0); }); diff --git a/tests/Feature/Screens/StarredTest.php b/tests/Feature/Screens/StarredTest.php index e67d34e..58e830b 100644 --- a/tests/Feature/Screens/StarredTest.php +++ b/tests/Feature/Screens/StarredTest.php @@ -49,7 +49,7 @@ function starredIds(TestableComponent $screen): array expect(starredIds($screen))->toBe(['s2', 's1']) ->and(nodeByRef($screen, 'star-s1')['props']['a11y_label'])->toBe('Unstar commit, 1 star') - ->and(menuItems($screen, 'more-s1')[0][0])->toBe('share-s1'); + ->and(nodeByRef($screen, 'share-s1')['props']['a11y_label'])->toBe('Share link'); Http::assertNotSent(fn (Request $request): bool => str_contains($request->url(), '/saved') || str_contains($request->url(), '/bookmark')); })->with(['ios', 'android']);