From f711db93154c35d37845c07f1aeb1dedd92fef10 Mon Sep 17 00:00:00 2001 From: Yaojia Wang Date: Wed, 29 Jul 2026 18:22:50 +0200 Subject: [PATCH] fix(projects): build the git panel the way the design specifies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shipped panel was functionally right — every number matched git — but it was not the approved design. I implemented from the plan's semantics (which state is green, when to flag stale) and let the mock's layout drift, and nothing caught it: the tests asserted behaviour, and no assertion covered structure. Three deviations, now closed against docs/mockups/project-detail-git.html. 1. The sync band was compressed into one row of chips. Restored to the four labelled cells: Upstream / To push / To pull / Fetch, with display-size numerals and a footnote under each count. The footnotes are load-bearing, not decoration — "Last fetched 19d ago — this number cannot be trusted" is what tells a reader WHY the zero is suspect. Compressed to "fetched 19d ago", the causal link was left for the reader to infer, and not having to infer it is the entire point of the panel. The unverified state is a chip on the count again, so the doubt attaches to the digit. 2. The working-tree count sat inside the band. Moved to the title line beside the branch, where the design puts it: the band is about the remote, and the dirty count is not. Reads "● 3 uncommitted" instead of a bare dot; the dot remains as the fallback when a server sends no count. 3. Worktree rows were the old single line with the path right-aligned against the chips. Rebuilt as the two-line card the design calls for — identity and state on top, path underneath — plus the Open button, which was specified and simply missing, so a worktree row could be read but not entered. Open reuses the detail view pointed at the worktree path: a linked worktree is a project directory, so this needs no new concept and no new route. Also added the unpushed count beside the commit-list heading, so the number is readable before the eye walks the rows hunting for rails. Tests now assert structure, not just behaviour: cell captions exist, the stale footnote says why, counts render at display size, the path is outside the chip row, and Open fires with its worktree. That is the gap that let the drift through, so it is the gap that is now covered. Verified: tsc and build clean; npm test green (unit 78 files / 2159, e2e 27). --- public/git-log.ts | 9 ++ public/projects.ts | Bin 64206 -> 66775 bytes public/style.css | 177 ++++++++++++++++++++++++++++++++---- test/git-log.test.ts | 22 +++++ test/projects-panel.test.ts | 116 +++++++++++++++++++++-- 5 files changed, 295 insertions(+), 29 deletions(-) diff --git a/public/git-log.ts b/public/git-log.ts index 54bc11a..e3a4994 100644 --- a/public/git-log.ts +++ b/public/git-log.ts @@ -106,6 +106,15 @@ export function renderGitLog(container: HTMLElement, log: GitLogResult): void { container.append(el('div', 'proj-empty', 'No commits yet.')) return } + // Design: the section label carries the unpushed count, so the number is + // readable before the eye has walked the rows looking for rails. + const unpushedCount = log.commits.filter((c) => c.unpushed === true).length + if (unpushedCount > 0 && log.upstream !== undefined) { + const badge = el('div', 'proj-commitlog-count', `↑ ${unpushedCount} unpushed`) + badge.title = `${unpushedCount} commit(s) not on ${log.upstream}` + container.append(badge) + } + const list = el('div', 'proj-commitlog-list') // w6/G4: draw the upstream's position ONCE, right after the last unpushed row. // Only with a named upstream — with nothing to compare against there is no diff --git a/public/projects.ts b/public/projects.ts index 8dafee2e1098aa85e16c81a320bee7d70b7105df..c76b59100e76058a86da2bdfb74976126d93a1e8 100644 GIT binary patch delta 3832 zcmai1Pi!1l8JC-+kQNI>oyH|G{k%=RUUzrwNN`|-Z9+-a(56O>qY9LUeKYgco_coP zGV{i3%dFR~2o;Bj!m9*lqzZwMXuAgvNTovJz@>s-kT@We6DJNx95^8OeecceuBiY^ z*37&=-{0T&`@Vhu@2CFoozrjqZG?N?TR>;l&W5xL=xhkl-(6EwUyF|($b@)<%bB@lT1?p zS+q)vg^(s?I*pR8T9XbSMd>CzyLsdJYq6|!WTLM)`J+W%qdu*!(E%iysSh51INMKx z21~8h%-g>{XBvmoX65M2+)lM#T3V~&aA|-w*!=!@hEEN4t8R}~d!pNwNw{cFxMd$c zmCJmjI4pIZCWXQEA_RTt;D6_4={vi6!Y%dyk&}DyG}9N@_d59^#KE?RGl@X#stBKJ zM*QMDy!ps#tDCA<8rIJ&_b(mr>2<2oiRA`XtJI`wwcm2`t=E0?l{*WDh7VQ_N;{#6 z!?jp_R^p2D$OUEo@G-M;e9ruK@QC^4owb49hIOPBeCl*XDoJdKh?98#=nd*^N5M8l z8NINvNfqR|JlxH*?TSY?#Xe<=l<5hp|!0 zMxnUX1q`L=OoqcVmBU$pEHT0YFUS25uZ&sB1-b8ke&Y01yXt<(gnPHBo`LRusS-}q}HVaE1}Qm>qNTvv;r^F`fVVc;ht6dq!OfF zB?vsjK?Df4Y&kP(CnI9H$D!XwgjvJtbntL_x)zu8V59PsUmF0vEo)7S5-9;=t1?yr zXfBm)z#8d=auN5$KH!?>@(Q(_xE60%NUC;w4B!BEf>cx40Y$Qqx-A#Yr{$gqG<*Vv zRSYN(Tm{4eNN-CWYYdy?MY_U+JGZ{k^@JO%ZIL?)0*%`IS@1e?ll#$k~ z9%75tXe-jLP;Ba5GpM078MxP`V)N->VZEEju|q=1iDkehP*YxIg66;RC1d&JXl5=kFY1iJa%o9D*mFt)aco+|-{{;e*@I~2%wFVf%+ z&W+8#h4M>9MOY+{c3fy(cY)UqsDKALaE9`hAo4KUHP78UJ2z^D6Rn{*S*;Cs2h!SqF)u#(AMR~#Fj-nu&pcWtesRyKPI4I`0R zlcEsok>01L(2Y4U9DLQSquU@sIcrQLUf~!f8=i)6&nAu!z!`|+?kM#tkT=RID5#qO z$|;sY2x5A$4buWO=8m-mNCHJk0Ii{C{&|;XU~LBt)|%<^td%+bV5Wf$7-iStPMJJD z%ZcM>xrSrrt#7_~il6g`7tMUXQoKK5qW;C={c-c-{%1z(Q-^c>KAb=7O%;pf9Pr%u z`{?Tv?=$9CM`usn|MvIH`$x}=*N^8G?*HJsbm_pxa@`A~RPR5n@cxk?{7PzT3r`QO9@dPGexTP$shsm2v67ewWQ+ z)GN#buU>DkyfR*#D2^f57c*Z0{B_U4i{{Or@64F)YjgwFDgcgYi-BhiA`H7S@cfn;y7rwbOe-jycud28bE^;bXfz-hLA_@i%qeDU)W zbt(Q=QBo+ih~g7$vaoMy={VvT+A@-t7MB?oW;3w&cfc88S2HZ zy6Fd*v$GBpncN2<#N6lh5lcu!$ubi&^vkMEQH)Cwk8m*Yw;S-o$=cXMR3Xe@gp1;uI;1PwRG-0wB?^jJa{A3L@ ziUE`XQ0z=d=Lv?1fQQM@{Og_h>u_!(=IJw&DH>0mQ0MkS13fj^;m9!)uuq~0Do4UH z9XsXoEFIzp$MC{%>0xu_y=n8zyDNjTpRlzyf>)Vsc9>U6#=U+M5}Db+wX6~kmG0Zg zp~5(+aMa7bZ{2pntT^Wy?MAXkT@@wZ9Sj1N!nyheMY=HI^tcEE*~Kpva4>(TaK>9aQEH6Eq}^sC`C7IiGIme!0`58(~tcdOwjhQ delta 1519 zcmaJ>O>Ep$5LTP!hlOU5*z^~g$!c5gqI8oWiU`uOB^4F9ltXig)R2AaeO?=RpB+ED zA*8E?R7t2>abV~j4&?-h$|trbE~o+lJyhbvrBYS(1VZ9ZLMm@;*KtAOWWSj=Gv9pQ z%=_`up6`Df_~NfcyxiLt{<1hfaH3^-{@N)g5I)qvaeZ0MSn73{&gB4fM@SzmdDdq( zhguGY+eKtcFT7$h8O2H!EZ2lyWhQ4Wn5fkt5G`(4g16wph1l2RrX~L6#P^q~_|tN6 zPm_rjes$#_o@^LB2wdrVoOR&N_6HywUj??ocNCD0%RCOO%`63}3TIwl0N>;wSw~qV z6n8Ew+yGNk;0DqxPJ(pQMN2{_P^}1lo_k<0+g3);3EC!Qr$m-GE)S_rpy@H8o6zL8 zvs#UEWgr&t78}RmANFQ(58@}y=3~)S%YZT-*0s?JtVafbN1`9-2Wws6w&bawLSpt_ zhH2s~<#s2>LnQ(iue$|BRRM>$^7y$v(o3p$)L-_N?^O>`j`Ok${E-vg!4JR9AEN#A+Inzo{;Afl9W^MUsf3!(xoC}_tnVrF> zwoYgK#KFjra-Gl(yt*|$NCQyDH?|Mpsck+QJJjq&n&9p2`Q56IiT4U=id$QS@R>{b z-O3(LU(T1qYvHv)obChmzI_ru{bn3bfBEVdWu-lxQ#%PY^|p$@wsHQ(1U^_E!t?zx byt`5e@7?%4hxfmp2^VkvIu)+`X*}@{VfO { expect(normalizeGitLog({ commits: [], truncated: false, upstream: 42 })!.upstream).toBeUndefined() }) }) + +describe('renderGitLog — unpushed count beside the heading (w6 design)', () => { + function entry(hash: string, subject: string, unpushed?: boolean) { + return { hash, at: Date.now(), subject, ...(unpushed === true ? { unpushed: true } : {}) } + } + + it('states the count so it is readable before scanning the rows', () => { + const host = document.createElement('div') + renderGitLog(host, { + commits: [entry('a1', 'x', true), entry('b2', 'y', true), entry('c3', 'z')], + truncated: false, + upstream: 'origin/develop', + }) + expect(host.querySelector('.proj-commitlog-count')!.textContent).toContain('2') + }) + + it('omits the count when nothing is unpushed', () => { + const host = document.createElement('div') + renderGitLog(host, { commits: [entry('a1', 'x')], truncated: false, upstream: 'origin/m' }) + expect(host.querySelector('.proj-commitlog-count')).toBeNull() + }) +}) diff --git a/test/projects-panel.test.ts b/test/projects-panel.test.ts index 2484035..d39af2b 100644 --- a/test/projects-panel.test.ts +++ b/test/projects-panel.test.ts @@ -777,18 +777,42 @@ describe('makeSyncBand (w6 G3)', () => { expect(b.textContent).toContain('origin/develop') }) - it('counts dirty files instead of showing a bare dot', () => { - const b = band({ upstream: 'origin/main', ahead: 0, behind: 0, lastFetchMs: FRESH }, { - dirtyCount: 3, - })! - expect(b.querySelector('.proj-sync-dirty')!.textContent).toContain('3') + // The design puts the dirty count on the title line, not in the band — the + // band is about the remote, the working tree is not. Asserted at the header + // level in the renderProjectDetail block below. + it('keeps the working-tree count out of the band', () => { + const b = band({ upstream: 'origin/main', ahead: 0, behind: 0, lastFetchMs: FRESH })! + expect(b.querySelector('.proj-sync-dirty')).toBeNull() }) - it('omits the dirty chip entirely for a clean tree', () => { - const b = band({ upstream: 'origin/main', ahead: 0, behind: 0, lastFetchMs: FRESH }, { - dirtyCount: 0, - })! - expect(b.querySelector('.proj-sync-dirty')).toBeNull() + // ── design fidelity: the captions and footnotes are load-bearing ────────── + it('labels every cell, so a number is never shown without saying what it is', () => { + const b = band({ upstream: 'origin/develop', ahead: 3, behind: 0, lastFetchMs: STALE })! + const captions = Array.from(b.querySelectorAll('.proj-sync-k')).map((n) => + (n.textContent ?? '').toLowerCase(), + ) + expect(captions).toContain('upstream') + expect(captions).toContain('to push') + expect(captions).toContain('to pull') + }) + + it('spells out WHY a stale zero cannot be trusted, not just that it is stale', () => { + const b = band({ upstream: 'origin/develop', ahead: 3, behind: 0, lastFetchMs: STALE })! + const bad = b.querySelector('.proj-sync-sub-bad') + expect(bad).not.toBeNull() + expect(bad!.textContent).toMatch(/cannot be trusted/i) + expect(bad!.textContent).toMatch(/fetched/i) + }) + + it('explains a missing upstream in words as well as colour', () => { + const b = band({ lastFetchMs: FRESH })! + expect(b.querySelector('.proj-sync-sub')!.textContent).toMatch(/tracks nothing/i) + }) + + it('renders the counts at display size', () => { + const b = band({ upstream: 'origin/develop', ahead: 3, behind: 0, lastFetchMs: FRESH })! + expect(b.querySelector('.proj-sync-ahead')!.classList.contains('proj-sync-big')).toBe(true) + expect(b.querySelector('.proj-sync-behind')!.classList.contains('proj-sync-big')).toBe(true) }) }) @@ -894,3 +918,75 @@ describe('makeWorktreeRow — session chip (w6 G7)', () => { expect(row.querySelector('.proj-wt-sessions')).toBeNull() }) }) + +/* ── design fidelity: header + worktree card ───────────────────────────────── */ + +describe('renderProjectDetail — dirty count on the title line (w6)', () => { + const hooks = () => ({ onOpenProject: vi.fn(), onEnterSession: vi.fn(), onFanout: vi.fn() }) + const cbs = () => ({ onBack: vi.fn(), onKill: vi.fn() }) + + function detail(over: Partial = {}) { + return { + name: 'repo', + path: '/p/repo', + isGit: true, + branch: 'develop', + worktrees: [], + sessions: [], + hasClaudeMd: false, + ...over, + } as import('../src/types.js').ProjectDetail + } + + it('shows the count beside the branch, not a bare dot', () => { + const root = renderProjectDetail(detail({ dirty: true, dirtyCount: 3 }), hooks(), cbs()) + const chip = root.querySelector('.proj-detail-head .proj-sync-dirty') + expect(chip).not.toBeNull() + expect(chip!.textContent).toContain('3') + expect(root.querySelector('.proj-dirty')).toBeNull() + }) + + it('shows nothing for a clean tree', () => { + const root = renderProjectDetail(detail({ dirty: false, dirtyCount: 0 }), hooks(), cbs()) + expect(root.querySelector('.proj-sync-dirty')).toBeNull() + expect(root.querySelector('.proj-dirty')).toBeNull() + }) + + it('falls back to the bare dot when the server sent no count', () => { + const root = renderProjectDetail(detail({ dirty: true }), hooks(), cbs()) + expect(root.querySelector('.proj-dirty')).not.toBeNull() + }) +}) + +describe('makeWorktreeRow — two-line card (w6 design)', () => { + const wt = { path: '/p/repo', isMain: true, isCurrent: true, branch: 'develop' } + + it('puts the path on its own line so it never competes with the chips', () => { + const row = makeWorktreeRow(wt, undefined, { sync: { upstream: 'o/d', ahead: 9 } }) + const main = row.querySelector('.proj-wt-main')! + expect(main.querySelector('.proj-wt-top .proj-wt-branch')).not.toBeNull() + expect(main.querySelector('.proj-wt-path')!.textContent).toBe('/p/repo') + // The path must NOT sit inside the chip row. + expect(row.querySelector('.proj-wt-top .proj-wt-path')).toBeNull() + }) + + it('offers Open when a handler is wired, and calls it with the worktree', () => { + const onOpen = vi.fn() + const row = makeWorktreeRow(wt, { onRemove: vi.fn(), onOpen }) + const btn = row.querySelector('.proj-wt-open') as HTMLButtonElement + expect(btn).not.toBeNull() + btn.click() + expect(onOpen).toHaveBeenCalledWith(wt) + }) + + it('renders no Open button when no handler is wired', () => { + expect(makeWorktreeRow(wt, { onRemove: vi.fn() }).querySelector('.proj-wt-open')).toBeNull() + }) + + it('marks the current worktree so it reads as "you are here"', () => { + expect(makeWorktreeRow(wt).classList.contains('proj-wt-row-current')).toBe(true) + expect( + makeWorktreeRow({ ...wt, isCurrent: false }).classList.contains('proj-wt-row-current'), + ).toBe(false) + }) +})