From dba83559d37fb9888004244f974d069fc575c3fc Mon Sep 17 00:00:00 2001 From: Yaojia Wang Date: Thu, 30 Jul 2026 07:57:00 +0200 Subject: [PATCH] fix(projects): close the last four git-panel design gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The low-severity leftovers from the panel audit, plus the tooling trap that made reviewing this work harder than it needed to be. .gitattributes (new) — `public/projects.ts diff` That file embeds literal NUL bytes on purpose: the namespace sentinels '\0active' / '\0other' are prefixed so they cannot collide with a real `First.Second` key. git therefore classified it as binary and printed "Binary files differ" for every diff, and `git diff --stat` reported "Bin 66815 -> 68701 bytes" instead of line counts — the file was effectively unreviewable. The `diff` attribute makes git diff it as text without setting `text`, so no line endings are normalised and the bytes on disk are untouched. (Same trap in the shell: grep prints only "Binary file ... matches", which reads as a no-match. The comment in the file says to use grep -a.) Detached HEAD now shows its short sha The Head cell held the "detached HEAD" chip and nothing else: it said what was wrong and nothing about where you were. SyncState gains an optional `head`, populated from the .git/HEAD text readSyncState ALREADY reads for the detached test — so this costs no extra I/O and no extra git call. Guarded by a hex regex and rendered as textContent. One geometry for the ghost button role The design defines two button roles. The outline one had drifted to three different sizes (.proj-wt-open at 11.5px/4px 12px/6px, .proj-diff-toggle at 12.5px/7px 15px/7px) and both used full-strength --text instead of the muted label. All four outline buttons are now declared once, together, at the design's geometry; the duplicate .proj-diff-toggle block further down the file is gone, since a second block at equal specificity is exactly how the role drifted in the first place. Header branch chip joins the chip language .proj-branch was the only chip with no border and no mono face, and 1px tighter all round, so the detail header's branch did not match the branch chips in the worktree list right below it. "Changes" heading over the View Diff toggle Every other block on the page is introduced by a section heading. Without one the toggle hung 4px off the bottom of the sync band and read as part of it. Tests: 2167 unit (4 new — detached sha present/absent on both the server and the panel) and 27 integration, twice. --- .gitattributes | 12 ++++++ public/projects.ts | Bin 68701 -> 69248 bytes public/style.css | 71 +++++++++++++++++------------------- src/http/projects.ts | 9 ++++- src/types.ts | 1 + test/projects-panel.test.ts | 12 ++++++ test/projects.test.ts | 15 ++++++++ 7 files changed, 82 insertions(+), 38 deletions(-) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..1cacf13 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,12 @@ +# public/projects.ts embeds literal NUL bytes on purpose: the namespace-group +# sentinels ACTIVE_GROUP_KEY = '\0active' and OTHER_GROUP_KEY = '\0other' are +# prefixed with NUL so they can never collide with a real `First.Second` key. +# +# git classifies any file containing NUL as binary, so `git diff`/`git show` +# print "Binary files differ" and review of this file is impossible. The `diff` +# attribute tells git to diff it as text anyway. It does NOT set `text`, so +# nothing is line-ending-normalised and the bytes on disk are untouched. +# +# Same trap in the shell: `grep` prints only "Binary file ... matches" and looks +# like a no-match. Use `grep -a` on this file. +public/projects.ts diff diff --git a/public/projects.ts b/public/projects.ts index 8bc9ad5cc72eef63e82ed74a68d427eba0e091dc..490e7efb9ec3d05a71a5d3c942bf424525b529ab 100644 GIT binary patch delta 493 zcmY*WJxc^J6hsgND-Wz?Ji+ro?gX(CZRCWjEv(-r*-daSIkH)oMMQfGi?y(^@COJ7 z*9um4mRj2QOMDx-!azQFnKzl4d_OMVJ}u7<=w>*?xd|H9A@O7dpJ<{Gv_hes54;k_ zXR$!GsFX)==vG>J1)5;o#n0_SycR0YSHXih%RtaTVKu=1uH(+NS;=(RHkIb*q`(t+|RL*VA9^+@;(A delta 31 ncmZpe%W`)H%LdcL$^GSQlm8})PnJxv*nB>TJ7x2Z<0sqz-G&Zh diff --git a/public/style.css b/public/style.css index 08e2327..7db0aa1 100644 --- a/public/style.css +++ b/public/style.css @@ -1541,13 +1541,17 @@ body { white-space: nowrap; } -/* Branch chip */ +/* Branch chip — same geometry and border as every other chip in the panel. It was + the odd one out: no border, no mono face, and 1px tighter all round, so the + detail header's branch did not match the branch chips in the worktree list. */ .proj-branch { - font-size: 11px; + font-family: var(--mono-font); + font-size: 11.5px; color: var(--accent); background: var(--accent-soft); - border-radius: 5px; - padding: 2px 7px; + border: 1px solid rgba(227, 166, 74, 0.36); + border-radius: 6px; + padding: 3px 9px; white-space: nowrap; flex: none; } @@ -1806,18 +1810,22 @@ body { flex: none; } -/* The design has exactly two button roles: filled accent for the action that - commits a form (Fetch, Create Worktree), and this ghost for anything that only - opens or reveals. Everything in a worktree's action rail is a ghost — including - the ✕, which previously had no rule at all and rendered as a raw grey OS button. */ +/* Role 2 of 2 — the ghost. The design has exactly two button roles: filled accent + for the action that commits a form (Fetch, Create Worktree), and this outline for + anything that only opens, reveals or toggles. ONE geometry for every one of them: + the role had drifted to three different sizes across the panel, all at + full-strength --text instead of the muted label the design asks for. + Every outline button in the panel is listed here, and nowhere else, so a new one + cannot quietly pick its own size again. */ .proj-wt-open, .proj-wt-remove, -.proj-wt-prune { +.proj-wt-prune, +.proj-diff-toggle { font: inherit; - font-size: 11.5px; + font-size: 12.5px; cursor: pointer; - padding: 4px 12px; - border-radius: 6px; + padding: 7px 15px; + border-radius: 7px; border: 1px solid var(--border-strong); background: transparent; color: var(--text-dim); @@ -1825,24 +1833,27 @@ body { .proj-wt-open:hover, .proj-wt-remove:hover, -.proj-wt-prune:hover { +.proj-wt-prune:hover, +.proj-diff-toggle:hover { border-color: var(--accent); color: var(--accent); } .proj-wt-open:focus-visible, .proj-wt-remove:focus-visible, -.proj-wt-prune:focus-visible { +.proj-wt-prune:focus-visible, +.proj-diff-toggle:focus-visible { outline: 2px solid var(--text); outline-offset: 2px; } -/* Square icon variant — same role, just not a word. Height matches Open so the - two sit flush in the rail. */ +/* Square icon variant — same role, just not a word. Keeps the shared height + (12.5px line + 7px padding twice) so it sits flush beside Open in the rail + rather than stretching the row. */ .proj-wt-remove { - width: 26px; - padding: 0; - line-height: 24px; + width: 33px; + padding: 7px 0; + line-height: 1; text-align: center; flex: none; } @@ -2513,26 +2524,12 @@ body { cursor: not-allowed; } -/* Role 2 — ghost, for anything that only reveals or toggles. */ -.proj-diff-toggle { - font: inherit; - font-size: 12.5px; - cursor: pointer; - padding: 7px 15px; - border-radius: 7px; - border: 1px solid var(--border-strong); - background: transparent; - color: var(--text-dim); -} - -.proj-diff-toggle:hover { - border-color: var(--accent); - color: var(--accent); -} +/* Role 2 (the ghost, incl. .proj-diff-toggle) is defined once, with the worktree + action rail — see "Role 2 of 2" above. Nothing for it here on purpose: a second + block at equal specificity is how the role drifted to three sizes before. */ .proj-wt-submit:focus-visible, -.proj-fanout-submit:focus-visible, -.proj-diff-toggle:focus-visible { +.proj-fanout-submit:focus-visible { outline: 2px solid var(--text); outline-offset: 2px; } diff --git a/src/http/projects.ts b/src/http/projects.ts index 875948b..dadaff5 100644 --- a/src/http/projects.ts +++ b/src/http/projects.ts @@ -209,7 +209,14 @@ async function readSyncState(repoPath: string): Promise { if (counts.ahead !== undefined) state.ahead = counts.ahead if (counts.behind !== undefined) state.behind = counts.behind if (lastFetchMs !== undefined) state.lastFetchMs = lastFetchMs - if (headText !== null && parseGitHead(headText) === null) state.detached = true + if (headText !== null && parseGitHead(headText) === null) { + state.detached = true + // A detached HEAD file holds the raw object id and nothing else. It is the + // only identity the cell can show — there is no branch name — and it costs + // no extra I/O, since the file is already read above for the detached test. + const sha = headText.trim() + if (/^[0-9a-f]{7,40}$/i.test(sha)) state.head = sha.slice(0, 7) + } return state } diff --git a/src/types.ts b/src/types.ts index 7ee8b12..c7e5f96 100644 --- a/src/types.ts +++ b/src/types.ts @@ -395,6 +395,7 @@ export interface SyncState { behind?: number; // commits on @{u} not on HEAD — trust only with a fresh lastFetchMs lastFetchMs?: number; // FETCH_HEAD mtime; undefined ⇒ never fetched detached?: boolean; // HEAD is not on a branch ⇒ no branch, no ahead/behind + head?: string; // short sha, ONLY when detached — the cell's sole identity } /** w6/G7: the git state of ONE worktree, fetched lazily per row. diff --git a/test/projects-panel.test.ts b/test/projects-panel.test.ts index 499eccf..a575a55 100644 --- a/test/projects-panel.test.ts +++ b/test/projects-panel.test.ts @@ -799,6 +799,18 @@ describe('makeSyncBand (w6 G3)', () => { expect(btn.disabled).toBe(true) }) + it('shows the short sha on a detached HEAD — it is the only identity left', () => { + const b = band({ detached: true, head: 'deadbee' })! + expect(b.querySelector('.proj-sync-upstream')!.textContent).toBe('deadbee') + expect(b.querySelector('.proj-sync-detached')!.textContent).toBe('detached HEAD') + }) + + it('still renders the detached chip when the sha could not be read', () => { + const b = band({ detached: true })! + expect(b.querySelector('.proj-sync-upstream')).toBeNull() + expect(b.querySelector('.proj-sync-detached')).not.toBeNull() + }) + it('names the upstream so the arrows are not floating numbers', () => { const b = band({ upstream: 'origin/develop', ahead: 9, behind: 0, lastFetchMs: FRESH })! expect(b.textContent).toContain('origin/develop') diff --git a/test/projects.test.ts b/test/projects.test.ts index 75f5687..28c7fd8 100644 --- a/test/projects.test.ts +++ b/test/projects.test.ts @@ -662,6 +662,21 @@ describe('buildProjectDetail — sync state (w6 G1)', { timeout: 30_000 }, () => expect(d!.branch).toBeUndefined() expect(d!.sync!.ahead).toBeUndefined() expect(d!.sync!.behind).toBeUndefined() + // With no branch name the short sha is the only identity the panel can show. + // Read out of the .git/HEAD text already loaded for the detached test — no + // extra git call. + expect(d!.sync!.head).toBe(stdout.trim().slice(0, 7)) + }) + + it('does not report a head sha when HEAD is on a branch', async () => { + if (!(await gitAvailable())) return + const repo = path.join(tmp, 'g1-attached-head') + await initRepo(repo) + await commit(repo, 'a.txt', 'only') + + const d = await buildProjectDetail(makeCfg({}), repo, []) + expect(d!.sync!.detached).not.toBe(true) + expect(d!.sync!.head).toBeUndefined() }) it('counts dirty files instead of only flagging them', async () => {