From 982a56853845c8a6aa607418cf229d97390f39a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C5=ABdolfs=20O=C5=A1i=C5=86=C5=A1?= Date: Fri, 4 Sep 2026 10:47:36 +0200 Subject: [PATCH] Polish author and committer popover Show the email alongside each name. Co-authors are read from the commit message trailers and listed as their own section. The trigger avatars now stack and fan apart on hover, and cover everyone credited on the commit rather than just the author and committer. Timestamps now show both committed and authored times in the tooltip. --- src/components/CobCommitTeaser.svelte | 25 +- src/components/CommitAuthors.svelte | 279 ++++++++++++++++++ src/components/CompactCommitAuthorship.svelte | 74 +---- src/components/HoverPopover.svelte | 49 ++- src/lib/utils.ts | 52 ++++ src/views/repo/RepoCommit.svelte | 26 +- tests/unit/utils.test.ts | 55 +++- 7 files changed, 439 insertions(+), 121 deletions(-) create mode 100644 src/components/CommitAuthors.svelte diff --git a/src/components/CobCommitTeaser.svelte b/src/components/CobCommitTeaser.svelte index db084527d..927b52f39 100644 --- a/src/components/CobCommitTeaser.svelte +++ b/src/components/CobCommitTeaser.svelte @@ -2,14 +2,10 @@ import type { Commit } from "@bindings/repo/Commit"; import type { Snippet } from "svelte"; - import { - absoluteTimestamp, - formatTimestamp, - gravatarURL, - twemoji, - } from "@app/lib/utils"; + import { commitTimes, formatTimestamp, twemoji } from "@app/lib/utils"; import Button from "@app/components/Button.svelte"; + import CommitAuthors from "@app/components/CommitAuthors.svelte"; import CompactCommitAuthorship from "@app/components/CompactCommitAuthorship.svelte"; import Icon from "@app/components/Icon.svelte"; import Id from "@app/components/Id.svelte"; @@ -101,13 +97,6 @@ gap: 0.5rem; min-width: 0; } - .commit-avatar { - width: 1rem; - height: 1rem; - border-radius: var(--border-radius-sm); - flex-shrink: 0; - object-fit: cover; - } .stacked-header .summary-title { flex: 1; min-width: 0; @@ -149,11 +138,7 @@ aria-label="commit-teaser"> {#if stacked}
- +
- + {formatTimestamp(commit.committer.time * 1000)}
diff --git a/src/components/CommitAuthors.svelte b/src/components/CommitAuthors.svelte new file mode 100644 index 000000000..9147d3ba7 --- /dev/null +++ b/src/components/CommitAuthors.svelte @@ -0,0 +1,279 @@ + + + + +{#snippet role(label: string, members: GitIdentity[])} +
+
{label}
+ {#each members as who (utils.identityKey(who))} +
+ +
+ {#if who.name} + {who.name} + {/if} + {#if who.email} + + {/if} +
+
+ {/each} +
+{/snippet} + + + {#snippet toggle()} +
+ {#each stacked as who, index (utils.identityKey(who))} + + {/each} + {#if overflow > 0} + +{overflow} + {/if} +
+ {/snippet} + + {#snippet popover()} +
+ {#if authorIsCommitter} + {@render role("Author & committer", [commit.author])} + {:else} + {@render role("Author", [commit.author])} + {@render role("Committer", [commit.committer])} + {/if} + {#if coAuthors.length > 0} + {@render role( + coAuthors.length === 1 ? "Co-author" : "Co-authors", + coAuthors, + )} + {/if} +
+ {/snippet} + diff --git a/src/components/CompactCommitAuthorship.svelte b/src/components/CompactCommitAuthorship.svelte index 78b58e656..59be56759 100644 --- a/src/components/CompactCommitAuthorship.svelte +++ b/src/components/CompactCommitAuthorship.svelte @@ -4,7 +4,7 @@ import * as utils from "@app/lib/utils"; - import HoverPopover from "@app/components/HoverPopover.svelte"; + import CommitAuthors from "@app/components/CommitAuthors.svelte"; interface Props { children: Snippet; @@ -22,80 +22,12 @@ align-items: center; white-space: nowrap; } - .person { - display: flex; - align-items: center; - flex-wrap: nowrap; - white-space: nowrap; - gap: 0.5rem; - font: var(--txt-code-regular); - } - .label { - color: var(--color-text-secondary); - } - .avatar { - width: 1rem; - height: 1rem; - }
- - {#snippet toggle()} -
- {#if commit.author.email === commit.committer.email} - avatar - {:else} - avatar - avatar - {/if} -
- {/snippet} - - {#snippet popover()} -
- {#if commit.author.email === commit.committer.email} -
-
Author
- avatar - {commit.author.name} -
- {:else} -
-
Author
- avatar - {commit.author.name} -
-
-
Committer
- avatar - {commit.committer.name} -
- {/if} -
- {/snippet} - + {@render children()} -
+
{utils.formatTimestamp(commit.committer.time * 1000)}
diff --git a/src/components/HoverPopover.svelte b/src/components/HoverPopover.svelte index ce10b3af5..da079ce18 100644 --- a/src/components/HoverPopover.svelte +++ b/src/components/HoverPopover.svelte @@ -35,6 +35,9 @@ // Whether the pointer is currently over the anchor or popover. let hovering = false; + // Whether focus is inside the anchor or popover, so that a keyboard user can + // reach any controls the popover holds. + let focused = false; // Whether a text-selection drag started inside the popover is ongoing. let selecting = false; @@ -46,31 +49,49 @@ visible = false; }, 200); - function enter() { - hovering = true; + function open() { hide.cancel(); show(); } - function leave() { - hovering = false; - show.cancel(); - // A selection drag may move the pointer out of the popover; keep it open - // until the drag ends (see endSelection). - if (!selecting) { + // Any one of pointer, focus or an ongoing selection drag is enough to keep + // the popover open, so closing waits until none of them holds it. + function close() { + if (!hovering && !focused && !selecting) { + show.cancel(); hide(); } } + function enter() { + hovering = true; + open(); + } + + function leave() { + hovering = false; + close(); + } + + // Focus crossing between the anchor and the portalled popover leaves and + // re-enters within one task, which the debounced hide absorbs. + function focus() { + focused = true; + open(); + } + + function blur() { + focused = false; + close(); + } + // A text-selection drag can move the pointer outside the popover and release // anywhere, so we follow it for the duration of the drag with a listener that // removes itself on release, rather than closing on pointer-leave. function endSelection() { selecting = false; document.removeEventListener("pointerup", endSelection); - if (!hovering) { - hide(); - } + close(); } function startSelection() { @@ -121,7 +142,9 @@ class="container" bind:this={anchorEl} onmouseenter={enter} - onmouseleave={leave}> + onmouseleave={leave} + onfocusin={focus} + onfocusout={blur}>
e.stopPropagation()}> {@render popover()} diff --git a/src/lib/utils.ts b/src/lib/utils.ts index 0aee91a47..f7736e642 100644 --- a/src/lib/utils.ts +++ b/src/lib/utils.ts @@ -3,6 +3,7 @@ import type { Issue } from "@bindings/cob/issue/Issue"; import type { Patch } from "@bindings/cob/patch/Patch"; import type { Review } from "@bindings/cob/patch/Review"; import type { Config } from "@bindings/config/Config"; +import type { Commit } from "@bindings/repo/Commit"; import type { RepoInfo } from "@bindings/repo/RepoInfo"; import type { ComponentProps } from "svelte"; @@ -374,3 +375,54 @@ export function explorerUrl(path: string, config: Config): string { .replace("$host", seed) .replace("$rid$path", path); } + +export interface GitIdentity { + name: string; + email: string; +} + +// Two git identities are the same person when they share an address, compared +// case-insensitively the way `gravatarURL` normalises it. An identity may carry +// no address at all, so those fall back to the name rather than collapsing into +// each other. +export function identityKey(who: GitIdentity): string { + const email = who.email.trim().toLowerCase(); + return email ? `email:${email}` : `name:${who.name.trim().toLowerCase()}`; +} + +// A commit carries an author date and a commit date, which come apart on a +// rebase or an amend. Views show the commit date, so the tooltip only names the +// two apart when there is something to tell apart. +export function commitTimes(commit: Commit): string { + const committed = absoluteTimestamp(commit.committer.time * 1000); + if (commit.author.time === commit.committer.time) { + return committed; + } + + return [ + `Authored — ${absoluteTimestamp(commit.author.time * 1000)}`, + `Committed — ${committed}`, + ].join("\n"); +} + +// Extract `Co-authored-by:` trailers from a commit message. +// +// Git only treats the final paragraph of a message as trailers, so a line +// appearing anywhere earlier — quoted in prose, or in a nested patch — is not +// one and is ignored here too. Matching is case-insensitive because git's own +// trailer handling is. The name may be empty, since a git identity only +// requires an address. +export function coAuthors(message: string): GitIdentity[] { + const paragraphs = message.trimEnd().split(/(?:\r?\n){2,}/); + const trailers = paragraphs[paragraphs.length - 1] ?? ""; + const authors: GitIdentity[] = []; + + for (const line of trailers.split("\n")) { + const match = /^co-authored-by:\s*(.*?)\s*<([^<>\s]+)>$/i.exec(line.trim()); + if (match) { + authors.push({ name: match[1], email: match[2] }); + } + } + + return authors; +} diff --git a/src/views/repo/RepoCommit.svelte b/src/views/repo/RepoCommit.svelte index 52b9aee94..94713f699 100644 --- a/src/views/repo/RepoCommit.svelte +++ b/src/views/repo/RepoCommit.svelte @@ -13,14 +13,15 @@ import * as router from "@app/lib/router"; import type { SidebarData } from "@app/lib/router/definitions"; import { - absoluteTimestamp, + commitTimes, formatOid, formatTimestamp, - gravatarURL, + identityKey, pluralize, } from "@app/lib/utils"; import Button from "@app/components/Button.svelte"; + import CommitAuthors from "@app/components/CommitAuthors.svelte"; import DiffActions from "@app/components/DiffActions.svelte"; import DiffStatBadge from "@app/components/DiffStatBadge.svelte"; import Icon from "@app/components/Icon.svelte"; @@ -51,6 +52,10 @@ }>(); let allCollapsed = $state(false); + const authorIsCommitter = $derived( + identityKey(commit.author) === identityKey(commit.committer), + ); + const changedFiles = $derived(gitStatusEntries(diff.files)); const treePaths = $derived(changedFiles.map(file => file.path)); @@ -143,12 +148,6 @@ align-items: center; gap: 0.5rem; } - .summary-avatar { - width: 1rem; - height: 1rem; - border-radius: 999px; - flex: none; - } .summary-timestamp { color: var(--color-text-quaternary); } @@ -211,17 +210,12 @@
- + {commit.author.name} - committed + {authorIsCommitter ? "committed" : "authored"} - + {formatTimestamp(commit.committer.time * 1000)} diff --git a/tests/unit/utils.test.ts b/tests/unit/utils.test.ts index 3be1c5070..cd43a130f 100644 --- a/tests/unit/utils.test.ts +++ b/tests/unit/utils.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "vitest"; -import { safeHttpUrl } from "@app/lib/utils"; +import { coAuthors, safeHttpUrl } from "@app/lib/utils"; describe("safeHttpUrl", () => { test.each([ @@ -27,3 +27,56 @@ describe("safeHttpUrl", () => { expect(safeHttpUrl(input)).toBeUndefined(); }); }); + +describe("coAuthors", () => { + test("reads trailers from the final paragraph", () => { + expect( + coAuthors( + "Fix the thing\n\nSome body text.\n\nSigned-off-by: A \nCo-authored-by: Alice Lidell \nCo-authored-by: Bob ", + ), + ).toEqual([ + { name: "Alice Lidell", email: "alice@lidell.com" }, + { name: "Bob", email: "bob@example.com" }, + ]); + }); + + test("is case-insensitive", () => { + expect( + coAuthors("Subject\n\nCO-AUTHORED-BY: Bob "), + ).toEqual([{ name: "Bob", email: "bob@example.com" }]); + }); + + test("ignores a trailer that is not in the last paragraph", () => { + expect( + coAuthors( + "Subject\n\nCo-authored-by: Bob \n\nThat line was quoted, not a trailer.", + ), + ).toEqual([]); + }); + + test("keeps the last paragraph rule for CRLF messages", () => { + expect( + coAuthors( + "Subject\r\n\r\nCo-authored-by: Bob \r\n\r\nThat line was quoted, not a trailer.\r\n", + ), + ).toEqual([]); + }); + + test("reads a trailer that carries no name", () => { + expect(coAuthors("Subject\n\nCo-authored-by: ")).toEqual([ + { name: "", email: "bob@example.com" }, + ]); + }); + + test("ignores malformed lines", () => { + expect( + coAuthors( + "Subject\n\nCo-authored-by: Bob\nCo-authored-by:\nNot-a-trailer: x ", + ), + ).toEqual([]); + }); + + test("returns nothing when there are no trailers", () => { + expect(coAuthors("Subject\n\nJust a body.")).toEqual([]); + }); +});