From 5aa5527279d7125275555ccc2c72391e4e13835f Mon Sep 17 00:00:00 2001 From: Jarred Stelfox Date: Mon, 12 Oct 2026 12:43:49 -0700 Subject: [PATCH 1/6] Make PR jail opt-in and add a hold setting The team found the popup pushy: it was on for everyone unless they went looking for the switch. The default is now the header count alone, and the popup is something a person turns on. The settings store writes every field on any change, so a saved 'auto' can't tell a choice from the old default. A one-time migration (settingsVersion 1) moves a saved 'auto' to the new default once; 'off' stays off, and anyone who turns the popup back on keeps it. jailHoldToClose (off by default) is for the people who liked having to hold the button down; the modal starts using it in a later commit. Co-Authored-By: Claude Opus 5.5 --- frontend-v2/src/components/JailMode.test.tsx | 14 ++++- frontend-v2/src/settings.test.ts | 50 ++++++++++++++++ frontend-v2/src/settings.ts | 63 ++++++++++++++++---- 3 files changed, 112 insertions(+), 15 deletions(-) create mode 100644 frontend-v2/src/settings.test.ts diff --git a/frontend-v2/src/components/JailMode.test.tsx b/frontend-v2/src/components/JailMode.test.tsx index 94bac16d..8cea53eb 100644 --- a/frontend-v2/src/components/JailMode.test.tsx +++ b/frontend-v2/src/components/JailMode.test.tsx @@ -1,10 +1,10 @@ // @vitest-environment jsdom import { act } from 'react'; import { createRoot, type Root } from 'react-dom/client'; -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { derive, type DerivedPull } from '../../../shared/model/status'; import type { PullData } from '../../../shared/types'; -import { setSettings } from '../settings'; +import { DEFAULT_SETTINGS, setSettings } from '../settings'; import { JailMode } from './JailMode'; // React's act() warns unless the environment says it's a test @@ -62,11 +62,12 @@ const many = (count: number) => Array.from({ length: count }, () => pull()); let root: Root | null = null; const host = document.createElement('div'); document.body.append(host); +// most of these watch it pop up, and close it the old way, by holding +beforeEach(() => setSettings({ jailMode: 'auto', jailHoldToClose: true })); afterEach(() => { act(() => root?.unmount()); root = null; localStorage.clear(); - setSettings({ jailMode: 'auto' }); vi.useRealTimers(); }); @@ -116,6 +117,13 @@ describe('JailMode while the cell is down', () => { expect(why()).toBe('8 open PRs. Parole at 7. Merge your ready one and you’re out.'); }); + it('only shows the badge until you turn the popup on', () => { + setSettings({ jailMode: DEFAULT_SETTINGS.jailMode }); + render(many(9)); + expect(dialog()).toBeNull(); + expect(document.querySelector('[aria-label^="PR jail"]')).not.toBeNull(); + }); + it('only shows the badge when PR jail is manual', () => { setSettings({ jailMode: 'manual' }); render(many(9)); diff --git a/frontend-v2/src/settings.test.ts b/frontend-v2/src/settings.test.ts new file mode 100644 index 00000000..f66587c4 --- /dev/null +++ b/frontend-v2/src/settings.test.ts @@ -0,0 +1,50 @@ +// @vitest-environment jsdom +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { DEFAULT_SETTINGS, migrateSettings } from './settings'; + +describe('Finish line is opt-in', () => { + it('shows only the header count by default, and closes without a hold', () => { + expect(DEFAULT_SETTINGS.jailMode).toBe('manual'); + expect(DEFAULT_SETTINGS.jailHoldToClose).toBe(false); + }); + + it('moves a saved popup from before to the new default, once', () => { + const old = { theme: 'dark' as const, jailMode: 'auto' as const }; + const once = migrateSettings(old); + expect(once).toEqual({ theme: 'dark', jailMode: 'manual', settingsVersion: 1 }); + // turned back on after the move, it stays on + const chosen = { ...once, jailMode: 'auto' as const }; + expect(migrateSettings(chosen)).toBe(chosen); + }); + + it('keeps Off, and carries over the day-old on/off switch', () => { + expect(migrateSettings({ jailMode: 'off' }).jailMode).toBe('off'); + expect(migrateSettings({ jailOn: false, jailMode: 'auto' })).toEqual({ + jailMode: 'off', + settingsVersion: 1, + }); + expect(migrateSettings({ jailOn: true, jailMode: 'auto' })).toEqual({ + jailMode: 'manual', + settingsVersion: 1, + }); + }); +}); + +describe('the saved blob on load', () => { + afterEach(() => { + localStorage.clear(); + vi.resetModules(); + }); + + it('is migrated before the store reads it', async () => { + localStorage.setItem('pd2.settings', JSON.stringify({ jailMode: 'auto', theme: 'dark' })); + vi.resetModules(); + const { getSettings } = await import('./settings'); + expect(getSettings().jailMode).toBe('manual'); + expect(getSettings().theme).toBe('dark'); + expect(JSON.parse(localStorage.getItem('pd2.settings') ?? '{}')).toMatchObject({ + jailMode: 'manual', + settingsVersion: 1, + }); + }); +}); diff --git a/frontend-v2/src/settings.ts b/frontend-v2/src/settings.ts index 0c6feae6..5dce2d35 100644 --- a/frontend-v2/src/settings.ts +++ b/frontend-v2/src/settings.ts @@ -1,6 +1,6 @@ import { STARVE_DAYS } from '../../shared/model/status'; import { JAIL_MAX_DAYS, JAIL_MAX_OPEN } from './model/jail'; -import { createPersistentStore } from './storage'; +import { createPersistentStore, readStorage, writeStorage } from './storage'; /** one named review circle; DEFAULT_TEAM_NAME is what quick add-to-team * gestures (the row kebab, the People filter) create when no roster exists */ @@ -111,15 +111,21 @@ export interface Settings { * or age tooltip) before it opens, so brushing the pointer across the board * doesn't flash panels open. 0 = open instantly; a click always bypasses it. */ hoverDelayMs: number; - /** PR jail: 'auto' pops it up when you're over the limits, 'manual' only - * shows the header lock (click to open), 'off' shows neither */ + /** Finish line (once "PR jail", hence the stored keys): 'auto' pops it up + * when you're over the limits, 'manual' only shows the header count + * (click to open), 'off' shows neither. Opt-in: 'manual' by default. */ jailMode: 'auto' | 'manual' | 'off'; - /** PR jail pops up when you have more than this many open PRs of your own */ + /** Finish line counts you over when you have more than this many open PRs */ jailMaxOpen: number; /** ...or when any of them has been open longer than this many days */ jailMaxDays: number; - /** whether your draft PRs count toward PR jail */ + /** whether your draft PRs count toward Finish line */ jailCountDrafts: boolean; + /** Finish line closes only by holding a button down; Esc and the close + * button do nothing. Off by default, for people who want the friction. */ + jailHoldToClose: boolean; + /** which one-time migrations (migrateSettings) this saved blob has had */ + settingsVersion: number; } /** The heaviest age text tier (red), derived from ageWarnDays rather than @@ -172,21 +178,54 @@ export const DEFAULT_SETTINGS: Settings = { repoQueueCap: 15, openPrsNewTab: true, hoverDelayMs: 250, - jailMode: 'auto', + jailMode: 'manual', jailMaxOpen: JAIL_MAX_OPEN, jailMaxDays: JAIL_MAX_DAYS, jailCountDrafts: false, + jailHoldToClose: false, + settingsVersion: 1, }; -const store = createPersistentStore('pd2.settings', DEFAULT_SETTINGS); -// jailOn (an on/off switch, live for a day) became jailMode; an Off carries over +type SavedSettings = Partial & { jailOn?: boolean }; + +/** + * Brings a saved blob up to date; the same object when there's nothing to do. + * - jailOn (an on/off switch, live for a day) became jailMode; an Off carries over. + * - Version 1 made Finish line opt-in. The store saves every field on any + * change, so a saved 'auto' can't tell a choice from the old default: a + * blob from before moves 'auto' to the new default once. Someone who wants + * the popup turns it back on, and it stays. + */ +export function migrateSettings(saved: SavedSettings): SavedSettings { + let next = saved; + if (next.jailOn !== undefined) { + const { jailOn, ...rest } = next; + next = jailOn ? rest : { ...rest, jailMode: 'off' }; + } + if ((next.settingsVersion ?? 0) < 1) { + next = { + ...next, + jailMode: next.jailMode === 'off' ? 'off' : DEFAULT_SETTINGS.jailMode, + settingsVersion: 1, + }; + } + return next; +} + +const SETTINGS_KEY = 'pd2.settings'; { - const saved = store.get() as Settings & { jailOn?: boolean }; - if (saved.jailOn !== undefined) { - const { jailOn, ...rest } = saved; - store.set({ ...rest, jailMode: jailOn ? rest.jailMode : 'off' }); + let saved: SavedSettings | null = null; + try { + saved = JSON.parse(readStorage(SETTINGS_KEY) ?? 'null') as SavedSettings | null; + } catch { + // unreadable: the store falls back to the defaults + } + if (saved) { + const next = migrateSettings(saved); + if (next !== saved) writeStorage(SETTINGS_KEY, JSON.stringify(next)); } } +const store = createPersistentStore(SETTINGS_KEY, DEFAULT_SETTINGS); /** Union of every personal roster, sorted — "your people": the set that * leads the review queues and wears the teammate corner heart. */ From 75c51f02c57c4ebe83b92b8823ff00f9b317bf33 Mon Sep 17 00:00:00 2001 From: Jarred Stelfox Date: Mon, 12 Oct 2026 12:54:13 -0700 Subject: [PATCH 2/6] Rename PR jail to Finish line and explain it The team read "jail" as punitive, didn't know what it meant or who saw it, couldn't find its settings, and some wanted a normal way to close it. Finish line keeps the same rule and asks for the same work, framed as finishing what's open. - The modal says why you're seeing it in numbers, that only you see it, and exactly what to do ("Merge or close 2 of these to finish. Start with the ones ready to merge."). The title counts down "2 to the finish line"; the sad robot behind bars is now one cheering at a flag. - Esc and a close button put it away, with the same 4-hour, only-if-worse return as before. "Make me hold to close it" (off by default) brings back the hold button and blocks both. - A cog in the modal opens Settings scrolled to and focused on the new Finish line section. - The help panel gets a Finish line section. - "jail" is gone from the code too: files, names, settings keys, the stored record key and the preview hash (#finish=1). Old saved values under the old keys are ignored on purpose, so everyone starts from the new defaults. Co-Authored-By: Claude Opus 5.5 --- bin/repair-stale-open-pulls | 2 +- frontend-v2/src/app.tsx | 4 +- ...{JailMode.test.tsx => FinishLine.test.tsx} | 162 +++++-- .../{JailMode.tsx => FinishLine.tsx} | 433 +++++++++--------- frontend-v2/src/components/Legend.tsx | 20 +- frontend-v2/src/components/Settings.tsx | 172 ++++--- frontend-v2/src/components/SettingsBits.tsx | 17 +- frontend-v2/src/hooks.ts | 2 +- frontend-v2/src/model/finishLine.test.ts | 211 +++++++++ frontend-v2/src/model/finishLine.ts | 186 ++++++++ frontend-v2/src/model/jail.test.ts | 200 -------- frontend-v2/src/model/jail.ts | 194 -------- frontend-v2/src/settings.test.ts | 49 +- frontend-v2/src/settings.ts | 70 +-- frontend-v2/src/styles.css | 42 +- 15 files changed, 909 insertions(+), 855 deletions(-) rename frontend-v2/src/components/{JailMode.test.tsx => FinishLine.test.tsx} (63%) rename frontend-v2/src/components/{JailMode.tsx => FinishLine.tsx} (66%) create mode 100644 frontend-v2/src/model/finishLine.test.ts create mode 100644 frontend-v2/src/model/finishLine.ts delete mode 100644 frontend-v2/src/model/jail.test.ts delete mode 100644 frontend-v2/src/model/jail.ts diff --git a/bin/repair-stale-open-pulls b/bin/repair-stale-open-pulls index 6d64d253..c64e97e9 100755 --- a/bin/repair-stale-open-pulls +++ b/bin/repair-stale-open-pulls @@ -3,7 +3,7 @@ * Pulls whose `closed` webhook never landed stay open in the DB. Until the * stale-open check listed repos outside config.repos, nothing repaired them * there: 21 PRs closed 2026-09-26 to 09-30, while prod wrote into dev's DB, - * stayed open on prod (iFixit/ops#1055 in PR jail). The hourly repair now + * stayed open on prod (iFixit/ops#1055, counted against their owners' open-PR limits). The hourly repair now * catches these; this fixes them without waiting for a deploy. * * It lists the open pulls of every repo the DB holds open pulls in and, with diff --git a/frontend-v2/src/app.tsx b/frontend-v2/src/app.tsx index 37e4fd0c..5d6d9b36 100644 --- a/frontend-v2/src/app.tsx +++ b/frontend-v2/src/app.tsx @@ -57,7 +57,7 @@ import { Stats } from './views/Stats'; import type { ProjectsNav } from './views/Projects'; import { Search } from './views/Search'; import { Settings } from './components/Settings'; -import { JailMode } from './components/JailMode'; +import { FinishLine } from './components/FinishLine'; export type { Lens }; @@ -1142,7 +1142,7 @@ export function App() { > v1 board - + setSettings({ jailMode: 'auto', jailHoldToClose: true })); +beforeEach(() => setSettings({ finishLineMode: 'auto', finishLineHoldToClose: true })); afterEach(() => { act(() => root?.unmount()); root = null; @@ -73,34 +73,42 @@ afterEach(() => { function render(pulls: DerivedPull[]) { const r = (root ??= createRoot(host)); - act(() => r.render()); + act(() => r.render()); } -const why = () => document.getElementById('jail-why')?.textContent; +const why = () => document.getElementById('finish-why')?.textContent; +const ask = () => document.getElementById('finish-ask')?.textContent; const listed = () => document.querySelectorAll('[role="dialog"] li').length; const dialog = () => document.querySelector('[role="dialog"]'); -const countdown = () => document.querySelector('#jail-title span')?.textContent; +const countdown = () => document.getElementById('finish-title')?.textContent; +const badge = () => document.querySelector('[aria-label*="to the finish line"]'); +const esc = () => + act(() => { + document.body.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Escape', bubbles: true, cancelable: true }) + ); + }); const headers = () => [...document.querySelectorAll('[role="dialog"] h3')].map(h => h.textContent); -describe('JailMode while the cell is down', () => { +describe('FinishLine while it is up', () => { it('follows new PRs and merges live', () => { const first = many(8); render(first); - expect(why()).toMatch(/^8 open PRs/); + expect(why()).toMatch(/^You have 8 open PRs/); expect(listed()).toBe(8); // a new PR arrives over the socket render([...first, pull()]); - expect(why()).toMatch(/^9 open PRs/); + expect(why()).toMatch(/^You have 9 open PRs/); expect(listed()).toBe(9); - expect(countdown()).toBe('2 to go'); + expect(countdown()).toBe('2 to the finish line'); // merges bring you under the limits: it says so, then lets you go vi.useFakeTimers(); render(first.slice(0, 3)); - expect(why()).toBe('You’re out. Nice work.'); - expect(countdown()).toBe('Free'); + expect(why()).toBe('Your open PRs are all under your limits. Nice work.'); + expect(countdown()).toBe('You’re at the finish line'); expect(listed()).toBe(0); act(() => vi.advanceTimersByTime(2500)); expect(dialog()).toBeNull(); @@ -114,48 +122,50 @@ describe('JailMode while the cell is down', () => { expect(document.querySelector('[role="dialog"] li a')?.textContent).toContain( ready.data.title ); - expect(why()).toBe('8 open PRs. Parole at 7. Merge your ready one and you’re out.'); + expect(why()).toBe( + 'You have 8 open PRs, over the limit of 7. Only you see this: it’s about your own PRs, and nobody else is told.' + ); + expect(ask()).toBe('Merge or close 1 of these to finish. Start with the one ready to merge.'); }); it('only shows the badge until you turn the popup on', () => { - setSettings({ jailMode: DEFAULT_SETTINGS.jailMode }); + setSettings({ finishLineMode: DEFAULT_SETTINGS.finishLineMode }); render(many(9)); expect(dialog()).toBeNull(); - expect(document.querySelector('[aria-label^="PR jail"]')).not.toBeNull(); + expect(badge()).not.toBeNull(); }); - it('only shows the badge when PR jail is manual', () => { - setSettings({ jailMode: 'manual' }); + it('only shows the badge when the popup is off', () => { + setSettings({ finishLineMode: 'manual' }); render(many(9)); expect(document.querySelector('[role="dialog"]')).toBeNull(); - const badge = document.querySelector('[aria-label^="PR jail"]'); - expect(badge).not.toBeNull(); - act(() => badge?.click()); - expect(why()).toMatch(/^9 open PRs/); + expect(badge()?.getAttribute('aria-label')).toMatch(/^2 to the finish line: You have 9/); + act(() => badge()?.click()); + expect(why()).toMatch(/^You have 9 open PRs/); }); - it('stays away, badge and all, when PR jail is off', () => { - setSettings({ jailMode: 'off' }); + it('stays away, badge and all, when it is off', () => { + setSettings({ finishLineMode: 'off' }); render(many(9)); expect(document.querySelector('[role="dialog"]')).toBeNull(); - expect(document.querySelector('[aria-label^="PR jail"]')).toBeNull(); + expect(badge()).toBeNull(); }); - it('lets you out only when the button is held down, by key or pointer', () => { + it('with the hold setting, closes only when the button is held down', () => { vi.useFakeTimers(); render(many(8)); - const snooze = [ + const hold = [ ...document.querySelectorAll('[role="dialog"] button'), - ].find(b => b.textContent?.includes('Hold to get back to work')); + ].find(b => b.textContent?.includes('Hold to close')); // it has focus from the start, so the keyboard can hold it right away - expect(document.activeElement).toBe(snooze); + expect(document.activeElement).toBe(hold); const key = (type: string, k = 'Enter') => - act(() => snooze?.dispatchEvent(new KeyboardEvent(type, { key: k, bubbles: true }))); + act(() => hold?.dispatchEvent(new KeyboardEvent(type, { key: k, bubbles: true }))); // a tap of Enter does nothing but say to hold it; neither does Esc key('keydown'); act(() => vi.advanceTimersByTime(200)); key('keyup'); - expect(snooze?.textContent).toBe('Hold it down'); + expect(hold?.textContent).toBe('Hold it down'); act(() => vi.advanceTimersByTime(3000)); act(() => document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' }))); expect(dialog()).not.toBeNull(); @@ -170,11 +180,11 @@ describe('JailMode while the cell is down', () => { vi.useFakeTimers(); const nine = many(9); render(nine); - expect(countdown()).toBe('2 to go'); + expect(countdown()).toBe('2 to the finish line'); render(nine.slice(1)); - expect(countdown()).toBe('1 to go'); + expect(countdown()).toBe('1 to the finish line'); // still there for the slide, then gone - expect(document.querySelectorAll('.jail-row-leave')).toHaveLength(1); + expect(document.querySelectorAll('.finish-row-leave')).toHaveLength(1); expect(listed()).toBe(9); act(() => vi.advanceTimersByTime(450)); expect(listed()).toBe(8); @@ -215,10 +225,10 @@ describe('JailMode while the cell is down', () => { ); }); - it('lets a screen reader or voice control out with a lone click', () => { + it('lets a screen reader or voice control close the hold with a lone click', () => { render(many(8)); const hold = [...document.querySelectorAll('[role="dialog"] button')].find( - b => b.textContent?.includes('Hold to get back to work') + b => b.textContent?.includes('Hold to close') ); // a mouse click carries detail 1 and does nothing act(() => hold?.dispatchEvent(new MouseEvent('click', { bubbles: true, detail: 1 }))); @@ -228,7 +238,7 @@ describe('JailMode while the cell is down', () => { expect(dialog()).toBeNull(); }); - it('keeps Esc from reaching anything behind it', () => { + it('with the hold setting, keeps Esc from closing it or reaching anything behind it', () => { render(many(8)); let reached = false; const behind = (e: KeyboardEvent) => { @@ -249,9 +259,9 @@ describe('JailMode while the cell is down', () => { vi.useFakeTimers(); const nine = many(9); render(nine); - const shownAt = JSON.parse(localStorage.getItem('pd2.jail') ?? 'null').at; + const shownAt = JSON.parse(localStorage.getItem('pd2.finishLine') ?? 'null').at; render(nine.slice(0, 3)); - expect(JSON.parse(localStorage.getItem('pd2.jail') ?? 'null')).toEqual({ + expect(JSON.parse(localStorage.getItem('pd2.finishLine') ?? 'null')).toEqual({ at: shownAt, count: 0, over: [], @@ -278,17 +288,27 @@ describe('JailMode while the cell is down', () => { expect(modalOpen()).toBe(true); }); - it('opens the preview every time Settings asks', () => { + it('opens the preview every time Settings asks, or the URL says #finish=1', () => { render(many(2)); expect(dialog()).toBeNull(); - act(() => window.dispatchEvent(new Event('pd2:preview-jail'))); + act(() => window.dispatchEvent(new Event('pd2:preview-finish-line'))); + expect(dialog()).not.toBeNull(); + act(() => document.querySelector('[data-hold]')?.dispatchEvent( + new MouseEvent('click', { bubbles: true, detail: 0 }) + )); + expect(dialog()).toBeNull(); + act(() => { + location.hash = 'finish=1'; + window.dispatchEvent(new HashChangeEvent('hashchange')); + }); expect(dialog()).not.toBeNull(); + location.hash = ''; }); it('a quick Space tap is not a lone click, even where the browser clicks on keyup', () => { render(many(8)); const hold = [...document.querySelectorAll('[role="dialog"] button')].find( - b => b.textContent?.includes('Hold to get back to work') + b => b.textContent?.includes('Hold to close') ); act(() => { hold?.dispatchEvent(new KeyboardEvent('keydown', { key: ' ', bubbles: true })); @@ -300,10 +320,10 @@ describe('JailMode while the cell is down', () => { }); it('keeps the last showing while it doesn’t know who you are', () => { - localStorage.setItem('pd2.jail', JSON.stringify({ at: Date.now(), count: 9, over: [] })); + localStorage.setItem('pd2.finishLine', JSON.stringify({ at: Date.now(), count: 9, over: [] })); const r = (root ??= createRoot(host)); - act(() => r.render()); - expect(localStorage.getItem('pd2.jail')).not.toBeNull(); + act(() => r.render()); + expect(localStorage.getItem('pd2.finishLine')).not.toBeNull(); expect(dialog()).toBeNull(); }); @@ -311,10 +331,10 @@ describe('JailMode while the cell is down', () => { // another window wrote the record after this one loaded const r = (root ??= createRoot(host)); act(() => - r.render() + r.render() ); - localStorage.setItem('pd2.jail', JSON.stringify({ at: Date.now(), count: 8, over: [] })); - act(() => r.render()); + localStorage.setItem('pd2.finishLine', JSON.stringify({ at: Date.now(), count: 8, over: [] })); + act(() => r.render()); expect(dialog()).toBeNull(); }); @@ -336,4 +356,54 @@ describe('JailMode while the cell is down', () => { render(many(9)); expect(dialog()).not.toBeNull(); }); + + it('closes on Esc or the close button, and stays away until things get worse', () => { + setSettings({ finishLineHoldToClose: false }); + const nine = many(9); + render(nine); + // focus starts on the close button; there's no hold button + expect(document.activeElement?.getAttribute('aria-label')).toBe('close'); + expect(document.querySelector('[data-hold]')).toBeNull(); + esc(); + expect(dialog()).toBeNull(); + render([...nine, pull()]); // worse, minutes later: within the 4 hours + expect(dialog()).toBeNull(); + act(() => badge()?.click()); + expect(dialog()).not.toBeNull(); + act(() => document.querySelector('[data-close]')?.click()); + expect(dialog()).toBeNull(); + }); + + it('the cog closes it and opens Settings at its own settings', () => { + setSettings({ finishLineHoldToClose: false }); + const asked: unknown[] = []; + const listen = (e: Event) => asked.push((e as CustomEvent).detail); + window.addEventListener('pd2:open-settings', listen); + render(many(9)); + act(() => + document + .querySelector('[aria-label="Finish line settings"]') + ?.click() + ); + window.removeEventListener('pd2:open-settings', listen); + expect(asked).toEqual(['finish-line']); + expect(dialog()).toBeNull(); + }); +}); + +describe('Settings, opened from the cog', () => { + it('scrolls to and focuses the Finish line settings', async () => { + const { Settings } = await import('./Settings'); + const scrolled = vi.fn(); + Element.prototype.scrollIntoView = scrolled; + const r = (root ??= createRoot(host)); + act(() => r.render( undefined} />)); + act(() => { + window.dispatchEvent(new CustomEvent('pd2:open-settings', { detail: 'finish-line' })); + }); + const section = document.getElementById('finish-line'); + expect(scrolled.mock.contexts[0]).toBe(section); + expect(section?.contains(document.activeElement)).toBe(true); + expect(document.activeElement?.textContent).toBe('Pop up'); + }); }); diff --git a/frontend-v2/src/components/JailMode.tsx b/frontend-v2/src/components/FinishLine.tsx similarity index 66% rename from frontend-v2/src/components/JailMode.tsx rename to frontend-v2/src/components/FinishLine.tsx index 45926b14..0a189bdb 100644 --- a/frontend-v2/src/components/JailMode.tsx +++ b/frontend-v2/src/components/FinishLine.tsx @@ -1,38 +1,40 @@ import { useEffect, useMemo, useRef, useState } from 'react'; import { createPortal } from 'react-dom'; -import { Lock } from 'lucide-react'; +import { Flag, Settings as SettingsIcon, X } from 'lucide-react'; import type { DerivedPull } from '../../../shared/model/status'; import { githubUrl } from '../../../shared/format'; import { askedOf, askForInputHint, rowNote, rowWord } from '../model/actions'; import { STAGE_WORDS } from '../model/stage'; import { - jailCase, - jailDue, - jailGroups, - jailRecord, - parseJailRecord, - JAIL_KEY, - type JailCase, -} from '../model/jail'; + finishLineCase, + finishDue, + finishGroups, + finishRecord, + parseFinishRecord, + FINISH_LINE_KEY, + type FinishLineCase, +} from '../model/finishLine'; import { useSettings } from '../settings'; import { readStorage, writeStorage } from '../storage'; import { HeaderIconButton, QuietButton, RepoRef } from './bits'; +import { Icon } from './Icon'; import { modalOpen } from '../hooks'; import { GroupHeader } from './Lane'; import { CiGlyph } from './pips'; /** - * PR jail: when your own open PRs pass a limit (model/jail.ts), a modal pops - * up listing them, and comes back only when that gets worse (jailDue). It - * has no close button and Esc does nothing: you hold Snooze down for a - * couple of seconds (a little work on purpose), or get under the limits and - * it lets you go. While you're over, a header badge counts down what's left - * and reopens it. `#jail=1` in the URL forces it, any time, for a preview. - * The jailMode setting picks: pop up (auto), the badge alone (manual), or off. + * Finish line: when your own open PRs pass a limit (model/finishLine.ts), a + * header badge counts down how many to merge or close, and opens a list of + * them. Only you see it. Turned on in Settings, the list also pops up by + * itself, and comes back only when that gets worse (finishDue). Esc or the + * close button puts it away; finishLineHoldToClose makes you hold a button + * down instead. Getting under the limits closes it too. `#finish=1` in the + * URL forces it, any time, for a preview. The finishLineMode setting picks: + * pop up (auto), the badge alone (manual), or off. */ // whether a mouse button or finger is down, or a native drag is under way: -// the jail never pops up mid-drag (a Roadmap bar or row, say) +// it never pops up mid-drag (a Roadmap bar or row, say) let pressing = false; let dragging = false; const pointerBusy = () => pressing || dragging; @@ -60,10 +62,10 @@ if (typeof document !== 'undefined') { window.addEventListener('blur', () => (pressing = dragging = false)); } -const hasPreviewFlag = () => new URLSearchParams(location.hash.slice(1)).get('jail') === '1'; +const hasPreviewFlag = () => new URLSearchParams(location.hash.slice(1)).get('finish') === '1'; /** Renders the header badge in place, and the cell over the page. */ -export function JailMode({ +export function FinishLine({ pulls, me, extraBots, @@ -78,22 +80,22 @@ export function JailMode({ // read on the first render: the app rewrites the hash right after and // drops the unknown param const [preview, setPreview] = useState(hasPreviewFlag); - // which jail is open, not a copy of it: the list below is read live, so a + // which list is open, not a copy of it: the list below is read live, so a // merge or a new PR shows up while it's open const [shown, setShown] = useState<'real' | 'preview' | null>(null); // the last showing, kept here too so blocked storage still holds it for // this page load; another tab's showing arrives by the storage event - const record = useRef(parseJailRecord(readStorage(JAIL_KEY))); + const record = useRef(parseFinishRecord(readStorage(FINISH_LINE_KEY))); const found = useMemo( () => // no case until we know who you are: an empty `me` (a failed whoami) // would read as "under the limits" and wipe the last showing - initialized && me && settings.jailMode !== 'off' - ? jailCase(pulls, me, extraBots, { - maxOpen: settings.jailMaxOpen, - maxDays: settings.jailMaxDays, - countDrafts: settings.jailCountDrafts, + initialized && me && settings.finishLineMode !== 'off' + ? finishLineCase(pulls, me, extraBots, { + maxOpen: settings.finishLineMaxOpen, + maxDays: settings.finishLineMaxDays, + countDrafts: settings.finishLineCountDrafts, }) : null, [initialized, pulls, me, extraBots, settings] @@ -104,30 +106,30 @@ export function JailMode({ // so it still comes back at most every 4 hours useEffect(() => { const last = record.current; - if (!initialized || !me || found || settings.jailMode === 'off' || !last) return; + if (!initialized || !me || found || settings.finishLineMode === 'off' || !last) return; if (!last.count && !last.over.length) return; record.current = { at: last.at, count: 0, over: [] }; - writeStorage(JAIL_KEY, JSON.stringify(record.current)); - }, [initialized, me, found, settings.jailMode]); + writeStorage(FINISH_LINE_KEY, JSON.stringify(record.current)); + }, [initialized, me, found, settings.finishLineMode]); // the latest case, for closing: the button's hold started seconds ago, and // what you saw last is what the next showing compares against const latest = useRef(found); latest.current = found; - // open PRs on the whole board, so a row leaving the jail's list only slides + // open PRs on the whole board, so a row leaving the list only slides // out as merged when it really closed (not when the rule switched) const openIds = useMemo( () => new Set(pulls.filter(p => p.data.state === 'open').map(pullId)), [pulls] ); - const remember = (jail: JailCase) => { - record.current = jailRecord(jail, Date.now()); - writeStorage(JAIL_KEY, JSON.stringify(record.current)); + const remember = (c: FinishLineCase) => { + record.current = finishRecord(c, Date.now()); + writeStorage(FINISH_LINE_KEY, JSON.stringify(record.current)); }; - const show = (jail: JailCase) => { - remember(jail); + const show = (c: FinishLineCase) => { + remember(c); setShown('real'); }; @@ -135,18 +137,18 @@ export function JailMode({ const onHash = () => { if (hasPreviewFlag()) setPreview(true); }; - // Settings' "Preview PR jail": an event, since setting the same hash + // Settings' "Preview it": an event, since setting the same hash // twice fires nothing const onPreview = () => setPreview(true); - window.addEventListener('pd2:preview-jail', onPreview); + window.addEventListener('pd2:preview-finish-line', onPreview); const onStorage = (e: StorageEvent) => { - if (e.key === JAIL_KEY) record.current = parseJailRecord(e.newValue); + if (e.key === FINISH_LINE_KEY) record.current = parseFinishRecord(e.newValue); }; window.addEventListener('hashchange', onHash); window.addEventListener('storage', onStorage); return () => { window.removeEventListener('hashchange', onHash); - window.removeEventListener('pd2:preview-jail', onPreview); + window.removeEventListener('pd2:preview-finish-line', onPreview); window.removeEventListener('storage', onStorage); }; }, []); @@ -162,17 +164,17 @@ export function JailMode({ // hidden tab or a focused field waits for the next one, so it never lands // mid-typing or out of sight useEffect(() => { - if (!found || shown || settings.jailMode !== 'auto') return; + if (!found || shown || settings.finishLineMode !== 'auto') return; const check = () => { if (document.visibilityState === 'hidden') return; - // another dialog is up (Settings), or a drag is under way: the jail + // another dialog is up (Settings), or a drag is under way: it // would land on top of it, and swallow the Esc that cancels a drag if (modalOpen() || pointerBusy()) return; if (document.activeElement?.matches('input, textarea, select')) return; // read the record fresh: a second visible window may have shown it // a moment ago, before its storage event reached this one - record.current = parseJailRecord(readStorage(JAIL_KEY)) ?? record.current; - if (!jailDue(found, record.current, Date.now())) return; + record.current = parseFinishRecord(readStorage(FINISH_LINE_KEY)) ?? record.current; + if (!finishDue(found, record.current, Date.now())) return; show(found); }; check(); @@ -182,7 +184,7 @@ export function JailMode({ document.removeEventListener('visibilitychange', check); window.removeEventListener('focus', check); }; - }, [found, shown, settings.jailMode]); + }, [found, shown, settings.finishLineMode]); // you got under the limits while it was up: it says so, then lets you go const freed = shown === 'real' && !found; @@ -196,25 +198,26 @@ export function JailMode({ <> {found && ( show(found)} className="relative" > - + {found.toGo > 9 ? '9+' : found.toGo} )} {shown && createPortal( - { // what you saw last is what the next showing compares @@ -229,93 +232,111 @@ export function JailMode({ ); } -/** How long "You're out" shows before the jail closes on its own. */ +/** How long "You're at the finish line" shows before it closes on its own. */ const FREED_MS = 2500; +/** "3 to the finish line", the countdown's words everywhere it shows. */ +const toFinish = (n: number) => `${n} to the finish line`; + /** Merged or closed your way under the limits while it was up. */ -const FREE: JailCase = { +const FREE: FinishLineCase = { pulls: [], - why: 'You’re out. Nice work.', + why: 'Your open PRs are all under your limits. Nice work.', + ask: '', + ready: 0, count: 0, over: [], toGo: 0, }; -/** The preview when you're not in jail: your PRs as jail counts them (any +/** The preview when you're under the limits: your PRs as it counts them (any * open one trips a limit of 0), or the board's oldest when you have none, * so there's something to look at. */ function sampleCase( pulls: DerivedPull[], me: string, extraBots: ReadonlySet, - settings: { jailMaxDays: number; jailCountDrafts: boolean } -): JailCase { - const mine = jailCase(pulls, me, extraBots, { + settings: { finishLineMaxDays: number; finishLineCountDrafts: boolean } +): FinishLineCase { + const mine = finishLineCase(pulls, me, extraBots, { maxOpen: 0, - maxDays: settings.jailMaxDays, - countDrafts: settings.jailCountDrafts, + maxDays: settings.finishLineMaxDays, + countDrafts: settings.finishLineCountDrafts, }); - if (mine) return { ...mine, why: 'Preview: your open PRs, as jail would list them' }; + if (mine) return { ...mine, why: 'Preview: your open PRs, as it would list them.', ask: '' }; return { pulls: pulls .filter(p => p.data.state === 'open') .sort((a, b) => b.ageDays - a.ageDays) .slice(0, 6), - why: 'Preview: these are sample PRs from the board', + why: 'Preview: these are sample PRs from the board.', + ask: '', + ready: 0, count: 0, over: [], toGo: 0, }; } -/** How long "Hold to get back to work" has to be held down. */ +/** How long "Hold to close" has to be held down. */ const HOLD_MS = 2000; -/** How long the open lock shows before the jail closes. */ +/** How long the full ring shows before it closes. */ const OPEN_MS = 700; /** How long a merged or closed row takes to leave the list. */ const LEAVE_MS = 450; +const iconButton = + 'pressable inline-flex h-8 w-8 items-center justify-center rounded-lg text-ink-3 hover:bg-muted hover:text-ink'; + /** The modal over the board: a centered card on a desktop, a sheet up from - * the bottom on a phone. Its header and its one button stay put while the - * list scrolls between them. No close button and no Esc: the way out besides - * getting under the limits is holding the button down for a couple of - * seconds, by pointer, finger, Enter or Space. */ -function JailModal({ - jail, + * the bottom on a phone. Its header stays put while the list scrolls under + * it. Esc or the close button puts it away; with `hold` on, neither does, and + * the way out besides getting under the limits is holding the button at the + * bottom down for a couple of seconds, by pointer, finger, Enter or Space. + * The cog opens Settings at this feature's own settings. */ +function FinishModal({ + found, me, maxDays, + hold, openIds, onDone, }: { - jail: JailCase; + found: FinishLineCase; me: string; maxDays: number; + hold: boolean; openIds: ReadonlySet; onDone: () => void; }) { const rootRef = useRef(null); - const free = jail === FREE; + const free = found === FREE; + // the key handler is set up once; it reads these fresh + const latest = useRef({ hold, onDone }); + latest.current = { hold, onDone }; - // "You're out" unmounts the focused button: keep focus in the dialog + // "You're at the finish line" unmounts the focused button: keep focus in + // the dialog useEffect(() => { if (free) rootRef.current?.focus({ preventScroll: true }); }, [free]); useEffect(() => { const root = rootRef.current; - // focus goes back where it was (the header lock, say) when it closes + // focus goes back where it was (the header flag, say) when it closes const before = document.activeElement as HTMLElement | null; - // focus starts on the button, so holding Enter or Space works right - // away; the dialog itself when it's showing "You're out" - const hold = root?.querySelector('[data-hold]'); - (hold ?? root)?.focus({ preventScroll: true }); - // Esc doesn't close it (holding the button does); Tab stays inside + // focus starts on the hold button, so holding Enter or Space works + // right away; on the close button when there's no hold + const start = root?.querySelector('[data-hold], [data-close]'); + (start ?? root)?.focus({ preventScroll: true }); const onKey = (e: KeyboardEvent) => { - // Esc does nothing here, and must not reach a dialog behind this one - // (Settings) either: listening in the capture phase runs first + // Esc closes it (unless you asked to hold), and must not reach a + // dialog behind this one (Settings) either: listening in the + // capture phase runs first if (e.key === 'Escape') { e.preventDefault(); e.stopPropagation(); + if (!latest.current.hold) latest.current.onDone(); } if (e.key !== 'Tab' || !root) return; const focusables = root.querySelectorAll('a[href], button'); @@ -349,47 +370,76 @@ function JailModal({ tabIndex={-1} role="dialog" aria-modal="true" - aria-labelledby="jail-title" - aria-describedby="jail-why" + aria-labelledby="finish-title" + aria-describedby="finish-why finish-ask" className="fixed inset-0 z-[200] flex flex-col items-center justify-end bg-ink/40 pt-8 outline-none sm:justify-start sm:px-4 sm:pt-[8vh] sm:pb-8" >
-
- +
+
-

- PR jail - {/* the countdown to freedom: ticks down live as you merge - or close */} - - {free ? 'Free' : `${jail.toGo} to go`} - + {/* the countdown: ticks down live as you merge or close */} +

+ {free + ? 'You’re at the finish line' + : found.toGo + ? toFinish(found.toGo) + : 'Finish line'}

- {jail.why} + {found.why} + {found.count > 0 && + ' Only you see this: it’s about your own PRs, and nobody else is told.'}

+
+ + {!hold && ( + + )} +
+ {found.ask && ( +

+ {found.ask} +

+ )} {!free && ( <> {/* the board's own sticky group headers, pinned to the top of this list on the card's color instead of under the app header on the canvas */}
- +
- + {hold && } )}
@@ -397,12 +447,10 @@ function JailModal({ ); } -/** The one button: hold it down for HOLD_MS, by pointer, finger, Enter or - * Space. The key in the lock turns a quarter turn as you hold and the ring - * around it fills, both off the same clock; letting go early springs them - * back, and a quick click says to hold it. At the end the lock opens and - * the jail closes. */ -function HoldToLeave({ onDone }: { onDone: () => void }) { +/** The hold button: hold it down for HOLD_MS, by pointer, finger, Enter or + * Space. The ring fills as you hold; letting go early springs it back, and a + * quick click says to hold it. At the end it closes. */ +function HoldToClose({ onDone }: { onDone: () => void }) { const [holding, setHolding] = useState(false); const [opened, setOpened] = useState(false); // a click too short to be a hold: the label teaches the gesture @@ -442,8 +490,7 @@ function HoldToLeave({ onDone }: { onDone: () => void }) { [] ); const isPress = (key: string) => key === 'Enter' || key === ' '; - // the ring and the key share one timing: fill and turn while held, snap - // back when let go + // the ring fills while held, and snaps back when let go const sweep = holding ? `${HOLD_MS}ms linear` : '150ms ease-out'; return ( ); @@ -621,7 +635,7 @@ function CopyNudge({ rows }: { rows: { pull: DerivedPull }[] }) { * quickest way out first, each group under the board's own header with a * word on what to do, oldest first. Someone else's PRs (the preview's * fallback) list flat, since their groups would be a reviewer's. */ -function JailList({ +function FinishList({ pulls, me, maxDays, @@ -637,7 +651,7 @@ function JailList({ const all = [...pulls, ...leaving]; const mine = all.every(p => p.data.user.login === me); const groups = mine - ? jailGroups(all, me) + ? finishGroups(all, me) : { ready: [], move: [], waiting: all.map(pull => ({ pull })) }; const live = (rows: { pull: DerivedPull }[]) => rows.filter(r => !gone.has(pullId(r.pull))); // the PRs waiting on people you asked, wherever they sit (a request gone @@ -670,7 +684,7 @@ function JailList({ )}
    {g.rows.map(({ pull }) => ( - maxDays; return (
  • @@ -759,9 +773,9 @@ function JailRow({ ); } -/** The inmate: a sad robot behind bars, drawn in the board's own line - * weights and color tokens so it themes with everything else. */ -function SadRobot() { +/** A robot cheering at a checkered finish flag, drawn in the board's own + * line weights and color tokens so it themes with everything else. */ +function FinishMark() { return ( - {/* a drooping antenna */} - - - {/* head, and the face on its screen */} + {/* the finish line on the ground */} + + {/* the flag: a pole and a checkered cloth */} + + + + {/* an upright antenna with a lit tip */} + + + {/* head, and a happy face on its screen */} - - - - - {/* striped prison body */} + + + {/* body, one arm up at the flag, one down */} - - {/* the bars, in front of him */} - + + ); } diff --git a/frontend-v2/src/components/Legend.tsx b/frontend-v2/src/components/Legend.tsx index 2a3b0695..351cbba6 100644 --- a/frontend-v2/src/components/Legend.tsx +++ b/frontend-v2/src/components/Legend.tsx @@ -1,5 +1,5 @@ import type { ReactNode } from 'react'; -import { CircleHelp, Heart, Star } from 'lucide-react'; +import { CircleHelp, Flag, Heart, Star } from 'lucide-react'; import { HeaderIconButton } from './bits'; import { Icon } from './Icon'; import { Popover } from './Popover'; @@ -160,6 +160,24 @@ export function Legend() { /> + + + + + 2 + + + } + def="shows in the header when your own open PRs pass your limits (by default more than 7 open, or one open over 14 days): the number is how many to merge or close. Click it for the list, ready ones first. Only you see it; nobody else is told" + /> + Settings} + def="under Finish line: Pop up opens the list by itself when you go over (off by default), Header count only waits for a click, Off hides it. Esc or ✕ closes the list" + /> + + weight:xs,s} diff --git a/frontend-v2/src/components/Settings.tsx b/frontend-v2/src/components/Settings.tsx index 992a513a..e5a5026e 100644 --- a/frontend-v2/src/components/Settings.tsx +++ b/frontend-v2/src/components/Settings.tsx @@ -123,10 +123,17 @@ export function Settings({ const triggerRef = useRef(null); const { armed: armReset, run: runReset } = useArmedConfirm(); + // which section to scroll to and focus once open: a Group's id + const [section, setSection] = useState(null); + // open on demand — the code-regions tip's "Set them up" action dispatches - // this so a reader can jump straight here from the board + // this so a reader can jump straight here from the board; a CustomEvent + // whose detail names a Group's id opens at that group (Finish line's cog) useEffect(() => { - const openIt = () => setOpen(true); + const openIt = (e: Event) => { + setSection(typeof (e as CustomEvent).detail === 'string' ? (e as CustomEvent).detail : null); + setOpen(true); + }; window.addEventListener('pd2:open-settings', openIt); return () => window.removeEventListener('pd2:open-settings', openIt); }, []); @@ -134,7 +141,7 @@ export function Settings({ useEffect(() => { if (!open) return; const onKey = (e: KeyboardEvent) => { - // a dialog above this one (PR jail) handled it + // a dialog above this one (Finish line) handled it if (e.defaultPrevented) return; if (e.key === 'Escape') setOpen(false); // aria-modal promises a focus trap; without this, Tab walks out of @@ -161,12 +168,20 @@ export function Settings({ // this blocks scroll const prevOverflow = document.body.style.overflow; document.body.style.overflow = 'hidden'; - // focus the panel so Escape and tabbing land inside it + // focus the panel so Escape and tabbing land inside it, or the first + // control of the section it was opened at panelRef.current?.focus(); + if (section) { + const target = panelRef.current?.querySelector(`#${section}`); + target?.scrollIntoView({ block: 'start' }); + target?.querySelector('button, input')?.focus({ preventScroll: true }); + setSection(null); + } return () => { document.removeEventListener('keydown', onKey); document.body.style.overflow = prevOverflow; }; + // section is read once, when it opens, so it isn't a dependency }, [open]); // return focus to the cog when the panel closes — but only after a real @@ -330,73 +345,6 @@ export function Settings({ onChange={ageDisplay => set({ ageDisplay })} /> - - set({ jailMode })} - /> - - - set({ jailMaxOpen })} - /> - - - set({ jailMaxDays })} - /> - - - set({ jailCountDrafts: v === 'on' })} - /> - - - { - // close Settings first so the jail isn't stacked on it - setOpen(false); - window.dispatchEvent(new Event('pd2:preview-jail')); - }} - > - Preview PR jail - - + + + A count in the header of your own open PRs past your limits, and a list of + them to merge or close. Only you see it. + + + set({ finishLineMode })} + /> + + + set({ finishLineMaxOpen })} + /> + + + set({ finishLineMaxDays })} + /> + + + set({ finishLineCountDrafts: v === 'on' })} + /> + + + set({ finishLineHoldToClose: v === 'on' })} + /> + + + { + // close Settings first so the list isn't stacked on it + setOpen(false); + window.dispatchEvent(new Event('pd2:preview-finish-line')); + }} + > + Preview + + + + diff --git a/frontend-v2/src/components/SettingsBits.tsx b/frontend-v2/src/components/SettingsBits.tsx index 2b84c556..cfcbda42 100644 --- a/frontend-v2/src/components/SettingsBits.tsx +++ b/frontend-v2/src/components/SettingsBits.tsx @@ -6,9 +6,22 @@ import { Icon } from './Icon'; /** A labelled group of controls inside a settings surface (the Settings panel * or the bell's nudge-settings view) — one definition so both render the * section chrome identically. */ -export function Group({ title, children }: { title: string; children: ReactNode }) { +export function Group({ + title, + id, + children, +}: { + title: string; + /** a target to open Settings at (see Settings' pd2:open-settings) */ + id?: string; + children: ReactNode; +}) { return ( -
    +

    {title}

    diff --git a/frontend-v2/src/hooks.ts b/frontend-v2/src/hooks.ts index b9cec02b..34aaeb15 100644 --- a/frontend-v2/src/hooks.ts +++ b/frontend-v2/src/hooks.ts @@ -1,6 +1,6 @@ import { type RefObject, useEffect, useLayoutEffect } from 'react'; -/** A modal dialog is open (PR jail, Settings): the board's own keys stand +/** A modal dialog is open (Finish line, Settings): the board's own keys stand * down, so `/` or `j` can't move focus behind the scrim. */ export const modalOpen = () => document.querySelector('[aria-modal="true"]') != null; diff --git a/frontend-v2/src/model/finishLine.test.ts b/frontend-v2/src/model/finishLine.test.ts new file mode 100644 index 00000000..3c78cd3d --- /dev/null +++ b/frontend-v2/src/model/finishLine.test.ts @@ -0,0 +1,211 @@ +import { describe, expect, it } from 'vitest'; +import { + FINISH_COOLDOWN_MS, + finishLineCase, + finishDue, + finishRecord, + parseFinishRecord, + type FinishPull, +} from './finishLine'; + +const LIMITS = { maxOpen: 7, maxDays: 14 }; +const NO_BOTS: ReadonlySet = new Set(); + +let n = 0; +function pull( + login: string, + ageDays: number, + o: { draft?: boolean; deployBlockedBy?: string[]; ready?: boolean; cryo?: boolean } = {} +): FinishPull { + n += 1; + return { + data: { + repo: 'iFixit/ifixit', + number: n, + title: `PR ${n}`, + draft: o.draft ?? false, + state: 'open', + user: { login }, + }, + ageDays, + deployBlockedBy: o.deployBlockedBy ?? [], + status: o.ready ? 'ready' : 'needs_cr', + cryo: o.cryo ?? false, + }; +} + +const many = (count: number, login = 'me', ageDays = 1) => + Array.from({ length: count }, () => pull(login, ageDays)); + +describe('finishLineCase', () => { + const line = (c: ReturnType) => `${c?.why} ${c?.ask}`; + + // [open, ready] -> the why and ask lines; limit 7 + it.each([ + [20, 0, 'You have 20 open PRs, over the limit of 7. Merge or close 13 of these to finish.'], + [ + 10, + 3, + 'You have 10 open PRs, over the limit of 7. Merge or close 3 of these to finish. Start with the ones ready to merge.', + ], + [ + 8, + 1, + 'You have 8 open PRs, over the limit of 7. Merge or close 1 of these to finish. Start with the one ready to merge.', + ], + ])('with %i open and %i ready, says how many and where to start', (open, ready, want) => { + const pulls = [ + ...many(open - ready), + ...Array.from({ length: ready }, () => pull('me', 1, { ready: true })), + ]; + const c = finishLineCase(pulls, 'me', NO_BOTS, LIMITS); + expect(line(c)).toBe(want); + expect(c?.ready).toBe(ready); + }); + + it('never counts a parked PR as ready', () => { + const parked = pull('me', 1, { ready: true, cryo: true }); + const c = finishLineCase([...many(7), parked], 'me', NO_BOTS, LIMITS); + expect(c?.ready).toBe(0); + expect(c?.ask).toBe('Merge or close 1 of these to finish.'); + }); + + it('names the old PRs when they’re part of the count', () => { + // 9 open and 1 old: 2 to go, the old one among them + const c = finishLineCase([...many(8), pull('me', 40)], 'me', NO_BOTS, LIMITS); + expect(line(c)).toBe( + 'You have 9 open PRs, over the limit of 7, and 1 is over 14 days old. Merge or close 2 of these to finish, the one over 14 days among them.' + ); + // 9 open and 3 old: closing the old ones is the whole ask + const three = [...many(6), pull('me', 20), pull('me', 30), pull('me', 40)]; + expect(finishLineCase(three, 'me', NO_BOTS, LIMITS)?.ask).toBe( + 'Merge or close the 3 over 14 days to finish.' + ); + }); + + it('says so when every one is old', () => { + expect(finishLineCase([pull('me', 20), pull('me', 30)], 'me', NO_BOTS, LIMITS)?.why).toBe( + 'You have 2 open PRs, all over 14 days old.' + ); + expect(finishLineCase([pull('me', 20)], 'me', NO_BOTS, LIMITS)?.why).toBe( + 'You have 1 open PR, over 14 days old.' + ); + }); + + it('lists only the old PRs when the count is fine', () => { + const old = pull('me', 30, { ready: true }); + expect(line(finishLineCase([pull('me', 1), old], 'me', NO_BOTS, LIMITS))).toBe( + 'You have 2 open PRs, and 1 is over 14 days old. Merge or close the one over 14 days to finish. Start with the one ready to merge.' + ); + }); + + it('counts what’s left to close, the old ones included', () => { + // 9 open (2 over the limit) and 3 old: closing the 3 old ones frees you + const pulls = [...many(6), pull('me', 20), pull('me', 30), pull('me', 40)]; + expect(finishLineCase(pulls, 'me', NO_BOTS, LIMITS)?.toGo).toBe(3); + // 12 open (5 over) and 1 old: 5 to go, the old one among them + expect(finishLineCase([...many(11), pull('me', 20)], 'me', NO_BOTS, LIMITS)?.toGo).toBe(5); + }); + + it('stays free at the open-PR limit', () => { + expect(finishLineCase(many(7), 'me', NO_BOTS, LIMITS)).toBeNull(); + }); + + it('counts you over past the open-PR limit, listing them oldest first', () => { + const pulls = [...many(7), pull('me', 9)]; + const c = finishLineCase(pulls, 'me', NO_BOTS, LIMITS); + expect(c?.pulls).toHaveLength(8); + expect(c?.pulls[0].ageDays).toBe(9); + expect(c?.count).toBe(8); + expect(c?.toGo).toBe(1); + }); + + it('counts you over for one PR past the age limit, and lists only the old ones', () => { + const old = pull('me', 21); + const c = finishLineCase([pull('me', 2), old, pull('me', 14)], 'me', NO_BOTS, LIMITS); + expect(c?.pulls).toEqual([old]); + expect(c?.count).toBe(3); + expect(c?.over).toEqual([`iFixit/ifixit#${old.data.number}`]); + expect(c?.toGo).toBe(1); + }); + + it('ignores other people’s PRs', () => { + expect( + finishLineCase([...many(9, 'someone'), pull('someone', 90)], 'me', NO_BOTS, LIMITS) + ).toBeNull(); + }); + + it('ignores drafts unless they count', () => { + const drafts = Array.from({ length: 9 }, () => pull('me', 90, { draft: true })); + expect(finishLineCase(drafts, 'me', NO_BOTS, LIMITS)).toBeNull(); + const counted = finishLineCase(drafts, 'me', NO_BOTS, { ...LIMITS, countDrafts: true }); + expect(counted?.pulls).toHaveLength(9); + }); + + it('ignores bots, by suffix and by the config list', () => { + expect(finishLineCase(many(9, 'dependabot[bot]'), 'dependabot[bot]', NO_BOTS, LIMITS)).toBeNull(); + expect(finishLineCase(many(9, 'renovate'), 'renovate', new Set(['renovate']), LIMITS)).toBeNull(); + }); + + it('ignores PRs held for a deploy', () => { + const held = Array.from({ length: 9 }, () => pull('me', 90, { deployBlockedBy: ['qa'] })); + expect(finishLineCase(held, 'me', NO_BOTS, LIMITS)).toBeNull(); + }); +}); + +describe('finishDue', () => { + const HOUR = 60 * 60 * 1000; + const T = 1_000_000_000_000; + const over = (pulls: FinishPull[]) => finishLineCase(pulls, 'me', NO_BOTS, LIMITS); + const recordAt = (pulls: FinishPull[], at: number) => { + const c = over(pulls); + if (!c) throw new Error('expected to be over a limit'); + return finishRecord(c, at); + }; + const eight = many(8); + const shownAt = recordAt(eight, T); + + it('never shows while you are under the limits', () => { + expect(finishDue(over(many(7)), null, T)).toBe(false); + }); + + it('shows the first time', () => { + expect(finishDue(over(eight), null, T)).toBe(true); + }); + + it('does not re-show on a refresh', () => { + expect(finishDue(over(eight), shownAt, T + 1000)).toBe(false); + }); + + it('does not re-show within 4 hours, even when it got worse', () => { + expect(finishDue(over([...eight, pull('me', 1)]), shownAt, T + 3 * HOUR)).toBe(false); + }); + + it('re-shows after 4 hours when you have more open PRs', () => { + expect(finishDue(over([...eight, pull('me', 1)]), shownAt, T + FINISH_COOLDOWN_MS)).toBe(true); + }); + + it('does not re-show after 4 hours when it is the same or better', () => { + expect(finishDue(over(eight), shownAt, T + 24 * HOUR)).toBe(false); + // closing two then opening one is still a net gain of room + const fewer = recordAt([...eight, ...many(2)], T); + expect(finishDue(over([...eight, pull('me', 1)]), fewer, T + 24 * HOUR)).toBe(false); + }); + + it('re-shows after 4 hours when a PR newly crosses the age limit', () => { + const old = pull('me', 20); + const pulls = [pull('me', 1), old]; + const last = recordAt(pulls, T); + expect(finishDue(over(pulls), last, T + 24 * HOUR)).toBe(false); + // same count: the young one aged past 14 days + const aged = { ...pulls[0], ageDays: 15 }; + expect(finishDue(over([aged, old]), last, T + 24 * HOUR)).toBe(true); + }); + + it('ignores a missing or unreadable record (and the old daily key)', () => { + expect(parseFinishRecord(null)).toBeNull(); + expect(parseFinishRecord('2026-10-09')).toBeNull(); + expect(parseFinishRecord('{"at":1}')).toBeNull(); + expect(parseFinishRecord(JSON.stringify(shownAt))).toEqual(shownAt); + }); +}); diff --git a/frontend-v2/src/model/finishLine.ts b/frontend-v2/src/model/finishLine.ts new file mode 100644 index 00000000..b8c6517b --- /dev/null +++ b/frontend-v2/src/model/finishLine.ts @@ -0,0 +1,186 @@ +import type { DerivedPull } from '../../../shared/model/status'; +import { isBotLogin } from '../../../shared/model/visibility'; +import type { PullData } from '../../../shared/types'; +import { rowWord } from './actions'; + +/** Over the finish line's limit with more than this many open PRs of your own. */ +export const FINISH_MAX_OPEN = 7; +/** ...or when any of your open PRs is older than this many days. */ +export const FINISH_MAX_DAYS = 14; + +/** localStorage key holding the FinishRecord of the last time it popped up. */ +export const FINISH_LINE_KEY = 'pd2.finishLine'; +/** After it pops up, it stays away at least this long. */ +export const FINISH_COOLDOWN_MS = 4 * 60 * 60 * 1000; + +/** What it showed you last time, so it only comes back when that got worse. */ +export interface FinishRecord { + /** ms when it showed */ + at: number; + /** how many counted open PRs you had then */ + count: number; + /** repo#number of each PR past the age limit then */ + over: string[]; +} + +/** The fields the rule reads off a derived pull. */ +export interface FinishPull { + data: Pick; + ageDays: number; + /** who holds a deploy block on it; a held PR is done, so it doesn't count */ + deployBlockedBy: string[]; + status: DerivedPull['status']; + /** parked (Cryogenic Storage): kept open on purpose, never "ready" */ + cryo: boolean; +} + +/** Can merge now, the quickest way to the finish line. A parked PR can be + * signed off and green, but My work files it under waiting, so this does too. */ +export const isFinishReady = (p: FinishPull) => p.status === 'ready' && !p.cryo; + +export interface FinishLineCase

    { + /** the PRs that put you there, oldest first */ + pulls: P[]; + /** why you're seeing it, in numbers: "You have 9 open PRs, ..." */ + why: string; + /** exactly what we ask: how many to merge or close, and where to start */ + ask: string; + /** how many of `pulls` can merge now */ + ready: number; + /** how many open PRs counted */ + count: number; + /** repo#number of each counted PR past the age limit */ + over: string[]; + /** how many PRs you still have to merge or close to finish: enough to + * get under the count limit, and every one past the age limit (closing an + * old one counts toward both) */ + toGo: number; +} + +const idOf = (p: FinishPull) => `${p.data.repo}#${p.data.number}`; + +const oldestFirst =

    (a: P, b: P) => b.ageDays - a.ageDays; + +/** + * Your own open, human PRs that aren't held for a deploy, checked against the + * two limits; drafts count only when `countDrafts` is on. Null when you're + * under both. + */ +export function finishLineCase

    ( + pulls: P[], + me: string, + extraBots: ReadonlySet, + limits: { maxOpen: number; maxDays: number; countDrafts?: boolean } +): FinishLineCase

    | null { + const mine = pulls + .filter( + p => + p.data.user.login === me && + p.data.state === 'open' && + (limits.countDrafts || !p.data.draft) && + !p.deployBlockedBy.length && + !isBotLogin(p.data.user.login, extraBots) + ) + .sort(oldestFirst); + const old = mine.filter(p => p.ageDays > limits.maxDays); + const counts = { + count: mine.length, + over: old.map(idOf), + toGo: Math.max(mine.length - limits.maxOpen, old.length), + }; + if (mine.length <= limits.maxOpen && !old.length) return null; + const listed = mine.length > limits.maxOpen ? mine : old; + const ready = listed.filter(isFinishReady).length; + return { + pulls: listed, + why: whyLine(mine.length, limits.maxOpen, old.length, limits.maxDays), + ask: askLine(counts.toGo, old.length, ready, limits.maxDays), + ready, + ...counts, + }; +} + +/** "You have 9 open PRs, over the limit of 7, and 2 are over 14 days old." */ +function whyLine(n: number, maxOpen: number, old: number, maxDays: number): string { + const prs = n === 1 ? '1 open PR' : `${n} open PRs`; + const over = n > maxOpen ? `, over the limit of ${maxOpen}` : ''; + const aged = !old + ? '' + : old === n + ? `, ${n === 1 ? '' : 'all '}over ${maxDays} days old` + : `, and ${old} ${old === 1 ? 'is' : 'are'} over ${maxDays} days old`; + return `You have ${prs}${over}${aged}.`; +} + +/** "Merge or close 2 of these to finish. Start with the ones ready to merge." + * Every PR past the age limit has to go, so the line names them when they + * are part of the count. */ +// ponytail: a ready PR that isn't old doesn't clear an old one; the start +// hint doesn't say which ready ones to pick +function askLine(toGo: number, old: number, ready: number, maxDays: number): string { + const theOld = old === 1 ? `the one over ${maxDays} days` : `the ${old} over ${maxDays} days`; + const what = !old + ? `${toGo} of these to finish` + : old === toGo + ? `${theOld} to finish` + : `${toGo} of these to finish, ${theOld} among them`; + const start = !ready + ? '' + : ready === 1 + ? ' Start with the one ready to merge.' + : ' Start with the ones ready to merge.'; + return `Merge or close ${what}.${start}`; +} + +/** + * The listed PRs split the way My work splits them, by the same rowWord, with + * the ready ones lifted out on top: ready to merge, your move, then waiting + * on others. Each group runs oldest first; the row's word says the rest. + */ +export function finishGroups(pulls: DerivedPull[], me: string) { + const worded = pulls + .map(pull => ({ pull, word: rowWord(pull, me) })) + .sort((a, b) => b.pull.ageDays - a.pull.ageDays); + const rest = worded.filter(w => !isFinishReady(w.pull)); + return { + ready: worded.filter(w => isFinishReady(w.pull)), + move: rest.filter(w => w.word.kind === 'do'), + waiting: rest.filter(w => w.word.kind === 'wait'), + }; +} + +/** The record to store when `found` shows at `now`. */ +export const finishRecord = (found: FinishLineCase, now: number): FinishRecord => ({ + at: now, + count: found.count, + over: found.over, +}); + +/** A stored record, or null when it's missing or unreadable. */ +export function parseFinishRecord(raw: string | null): FinishRecord | null { + try { + const r = JSON.parse(raw ?? 'null') as Partial | null; + if (typeof r?.at !== 'number' || typeof r.count !== 'number' || !Array.isArray(r.over)) { + return null; + } + return { at: r.at, count: r.count, over: r.over }; + } catch { + return null; + } +} + +/** + * Whether it should pop up now: you're over a limit, and either it never + * showed, or it's been at least FINISH_COOLDOWN_MS and things got worse since + * (more open PRs, net, or a PR newly past the age limit). + */ +export function finishDue( + found: FinishLineCase | null, + last: FinishRecord | null, + now: number +): boolean { + if (!found) return false; + if (!last) return true; + if (now - last.at < FINISH_COOLDOWN_MS) return false; + return found.count > last.count || found.over.some(id => !last.over.includes(id)); +} diff --git a/frontend-v2/src/model/jail.test.ts b/frontend-v2/src/model/jail.test.ts deleted file mode 100644 index 0069a243..00000000 --- a/frontend-v2/src/model/jail.test.ts +++ /dev/null @@ -1,200 +0,0 @@ -import { describe, expect, it } from 'vitest'; -import { - JAIL_COOLDOWN_MS, - jailCase, - jailDue, - jailRecord, - parseJailRecord, - type JailPull, -} from './jail'; - -const LIMITS = { maxOpen: 7, maxDays: 14 }; -const NO_BOTS: ReadonlySet = new Set(); - -let n = 0; -function pull( - login: string, - ageDays: number, - o: { draft?: boolean; deployBlockedBy?: string[]; ready?: boolean; cryo?: boolean } = {} -): JailPull { - n += 1; - return { - data: { - repo: 'iFixit/ifixit', - number: n, - title: `PR ${n}`, - draft: o.draft ?? false, - state: 'open', - user: { login }, - }, - ageDays, - deployBlockedBy: o.deployBlockedBy ?? [], - status: o.ready ? 'ready' : 'needs_cr', - cryo: o.cryo ?? false, - }; -} - -const many = (count: number, login = 'me', ageDays = 1) => - Array.from({ length: count }, () => pull(login, ageDays)); - -describe('jailCase', () => { - // [open, ready] -> the why line; limit 7 - it.each([ - [20, 0, '20 open PRs. Parole at 7. Close 13 to get out.'], - [20, 3, '20 open PRs. Parole at 7. Merge your 3 ready ones and close 10 more.'], - [10, 3, '10 open PRs. Parole at 7. Merge your 3 ready ones and you’re out.'], - [12, 6, '12 open PRs. Parole at 7. Merge 5 of your 6 ready ones and you’re out.'], - [8, 1, '8 open PRs. Parole at 7. Merge your ready one and you’re out.'], - [9, 1, '9 open PRs. Parole at 7. Merge your ready one and close 1 more.'], - ])('with %i open and %i ready, points at the ready ones first', (open, ready, why) => { - const pulls = [ - ...many(open - ready), - ...Array.from({ length: ready }, () => pull('me', 1, { ready: true })), - ]; - expect(jailCase(pulls, 'me', NO_BOTS, LIMITS)?.why).toBe(why); - }); - - it('never counts a parked PR as ready', () => { - const parked = pull('me', 1, { ready: true, cryo: true }); - expect(jailCase([...many(7), parked], 'me', NO_BOTS, LIMITS)?.why).toBe( - '8 open PRs. Parole at 7. Close 1 to get out.' - ); - }); - - it('won’t promise you’re out while an old PR would still hold you', () => { - const old = pull('me', 40); - const pulls = [...many(6), old, pull('me', 1, { ready: true })]; - expect(jailCase(pulls, 'me', NO_BOTS, LIMITS)?.why).toBe( - `8 open PRs. Parole at 7. Merge your ready one to get under 7. #${old.data.number} is still past 14 days.` - ); - // an old PR that's ready leaves with the merge, so you are out - const oldReady = [...many(7), pull('me', 40, { ready: true })]; - expect(jailCase(oldReady, 'me', NO_BOTS, LIMITS)?.why).toBe( - '8 open PRs. Parole at 7. Merge your ready one and you’re out.' - ); - }); - - it('counts what’s left to close, the old ones included', () => { - // 9 open (2 over the limit) and 3 old: closing the 3 old ones frees you - const pulls = [...many(6), pull('me', 20), pull('me', 30), pull('me', 40)]; - expect(jailCase(pulls, 'me', NO_BOTS, LIMITS)?.toGo).toBe(3); - // 12 open (5 over) and 1 old: 5 to go, the old one among them - expect(jailCase([...many(11), pull('me', 20)], 'me', NO_BOTS, LIMITS)?.toGo).toBe(5); - }); - - it('says merge when the one old PR is ready, and counts several', () => { - const old = pull('me', 30, { ready: true }); - expect(jailCase([old], 'me', NO_BOTS, LIMITS)?.why).toBe( - `#${old.data.number} has been open 30 days and it’s ready. Merge it to get out.` - ); - expect(jailCase([pull('me', 20), pull('me', 15)], 'me', NO_BOTS, LIMITS)?.why).toBe( - '2 PRs open past 14 days. Merge or close them to get out.' - ); - }); - - it('stays free at the open-PR limit', () => { - expect(jailCase(many(7), 'me', NO_BOTS, LIMITS)).toBeNull(); - }); - - it('jails past the open-PR limit, listing them oldest first', () => { - const pulls = [...many(7), pull('me', 9)]; - const c = jailCase(pulls, 'me', NO_BOTS, LIMITS); - expect(c?.pulls).toHaveLength(8); - expect(c?.pulls[0].ageDays).toBe(9); - expect(c?.why).toBe('8 open PRs. Parole at 7. Close 1 to get out.'); - expect(c?.count).toBe(8); - expect(c?.toGo).toBe(1); - }); - - it('jails one PR past the age limit, and lists only the old ones', () => { - const old = pull('me', 21); - const c = jailCase([pull('me', 2), old, pull('me', 14)], 'me', NO_BOTS, LIMITS); - expect(c?.pulls).toEqual([old]); - expect(c?.why).toBe( - `#${old.data.number} has been open 21 days. Parole at 14 days. Merge or close it to get out.` - ); - expect(c?.count).toBe(3); - expect(c?.over).toEqual([`iFixit/ifixit#${old.data.number}`]); - expect(c?.toGo).toBe(1); - }); - - it('ignores other people’s PRs', () => { - expect( - jailCase([...many(9, 'someone'), pull('someone', 90)], 'me', NO_BOTS, LIMITS) - ).toBeNull(); - }); - - it('ignores drafts unless they count', () => { - const drafts = Array.from({ length: 9 }, () => pull('me', 90, { draft: true })); - expect(jailCase(drafts, 'me', NO_BOTS, LIMITS)).toBeNull(); - const counted = jailCase(drafts, 'me', NO_BOTS, { ...LIMITS, countDrafts: true }); - expect(counted?.pulls).toHaveLength(9); - }); - - it('ignores bots, by suffix and by the config list', () => { - expect(jailCase(many(9, 'dependabot[bot]'), 'dependabot[bot]', NO_BOTS, LIMITS)).toBeNull(); - expect(jailCase(many(9, 'renovate'), 'renovate', new Set(['renovate']), LIMITS)).toBeNull(); - }); - - it('ignores PRs held for a deploy', () => { - const held = Array.from({ length: 9 }, () => pull('me', 90, { deployBlockedBy: ['qa'] })); - expect(jailCase(held, 'me', NO_BOTS, LIMITS)).toBeNull(); - }); -}); - -describe('jailDue', () => { - const HOUR = 60 * 60 * 1000; - const T = 1_000_000_000_000; - const over = (pulls: JailPull[]) => jailCase(pulls, 'me', NO_BOTS, LIMITS); - const recordAt = (pulls: JailPull[], at: number) => { - const c = over(pulls); - if (!c) throw new Error('expected to be over a limit'); - return jailRecord(c, at); - }; - const eight = many(8); - const shownAt = recordAt(eight, T); - - it('never shows while you are under the limits', () => { - expect(jailDue(over(many(7)), null, T)).toBe(false); - }); - - it('shows the first time', () => { - expect(jailDue(over(eight), null, T)).toBe(true); - }); - - it('does not re-show on a refresh', () => { - expect(jailDue(over(eight), shownAt, T + 1000)).toBe(false); - }); - - it('does not re-show within 4 hours, even when it got worse', () => { - expect(jailDue(over([...eight, pull('me', 1)]), shownAt, T + 3 * HOUR)).toBe(false); - }); - - it('re-shows after 4 hours when you have more open PRs', () => { - expect(jailDue(over([...eight, pull('me', 1)]), shownAt, T + JAIL_COOLDOWN_MS)).toBe(true); - }); - - it('does not re-show after 4 hours when it is the same or better', () => { - expect(jailDue(over(eight), shownAt, T + 24 * HOUR)).toBe(false); - // closing two then opening one is still a net gain of room - const fewer = recordAt([...eight, ...many(2)], T); - expect(jailDue(over([...eight, pull('me', 1)]), fewer, T + 24 * HOUR)).toBe(false); - }); - - it('re-shows after 4 hours when a PR newly crosses the age limit', () => { - const old = pull('me', 20); - const pulls = [pull('me', 1), old]; - const last = recordAt(pulls, T); - expect(jailDue(over(pulls), last, T + 24 * HOUR)).toBe(false); - // same count: the young one aged past 14 days - const aged = { ...pulls[0], ageDays: 15 }; - expect(jailDue(over([aged, old]), last, T + 24 * HOUR)).toBe(true); - }); - - it('ignores a missing or unreadable record (and the old daily key)', () => { - expect(parseJailRecord(null)).toBeNull(); - expect(parseJailRecord('2026-10-09')).toBeNull(); - expect(parseJailRecord('{"at":1}')).toBeNull(); - expect(parseJailRecord(JSON.stringify(shownAt))).toEqual(shownAt); - }); -}); diff --git a/frontend-v2/src/model/jail.ts b/frontend-v2/src/model/jail.ts deleted file mode 100644 index 65f5504b..00000000 --- a/frontend-v2/src/model/jail.ts +++ /dev/null @@ -1,194 +0,0 @@ -import type { DerivedPull } from '../../../shared/model/status'; -import { isBotLogin } from '../../../shared/model/visibility'; -import type { PullData } from '../../../shared/types'; -import { rowWord } from './actions'; - -/** Jail when you have more than this many open PRs of your own. */ -export const JAIL_MAX_OPEN = 7; -/** Jail when any of your open PRs is older than this many days. */ -export const JAIL_MAX_DAYS = 14; - -/** localStorage key holding the JailRecord of the last time jail showed. */ -export const JAIL_KEY = 'pd2.jail'; -/** After jail shows, it stays away at least this long. */ -export const JAIL_COOLDOWN_MS = 4 * 60 * 60 * 1000; - -/** What jail showed you last time, so it only comes back when that got worse. */ -export interface JailRecord { - /** ms when it showed */ - at: number; - /** how many counted open PRs you had then */ - count: number; - /** repo#number of each PR past the age limit then */ - over: string[]; -} - -/** The fields the rule reads off a derived pull. */ -export interface JailPull { - data: Pick; - ageDays: number; - /** who holds a deploy block on it; a held PR is done, so it doesn't count */ - deployBlockedBy: string[]; - status: DerivedPull['status']; - /** parked (Cryogenic Storage): kept open on purpose, never "ready" */ - cryo: boolean; -} - -/** Can merge now, the quickest way out. A parked PR can be signed off and - * green, but My work files it under waiting, so jail does too. */ -export const isJailReady = (p: JailPull) => p.status === 'ready' && !p.cryo; - -export interface JailCase

    { - /** the PRs that put you there, oldest first */ - pulls: P[]; - /** one line saying why, and how to get out */ - why: string; - /** how many open PRs counted */ - count: number; - /** repo#number of each counted PR past the age limit */ - over: string[]; - /** how many PRs you still have to merge or close to be free: enough to - * get under the count limit, and every one past the age limit (closing an - * old one counts toward both) */ - toGo: number; -} - -const idOf = (p: JailPull) => `${p.data.repo}#${p.data.number}`; - -const oldestFirst =

    (a: P, b: P) => b.ageDays - a.ageDays; - -/** - * Your own open, human PRs that aren't held for a deploy, checked against the - * two limits; drafts count only when `countDrafts` is on. Null when you're - * under both. - */ -export function jailCase

    ( - pulls: P[], - me: string, - extraBots: ReadonlySet, - limits: { maxOpen: number; maxDays: number; countDrafts?: boolean } -): JailCase

    | null { - const mine = pulls - .filter( - p => - p.data.user.login === me && - p.data.state === 'open' && - (limits.countDrafts || !p.data.draft) && - !p.deployBlockedBy.length && - !isBotLogin(p.data.user.login, extraBots) - ) - .sort(oldestFirst); - const old = mine.filter(p => p.ageDays > limits.maxDays); - const counts = { - count: mine.length, - over: old.map(idOf), - toGo: Math.max(mine.length - limits.maxOpen, old.length), - }; - if (mine.length > limits.maxOpen) { - const why = countWhy( - mine.length, - limits.maxOpen, - mine.filter(isJailReady).length, - old.filter(p => !isJailReady(p)), - limits.maxDays - ); - return { pulls: mine, why, ...counts }; - } - if (old.length) { - return { pulls: old, why: ageWhy(old, limits.maxDays), ...counts }; - } - return null; -} - -/** The count line, pointing at the ready PRs first since merging one is - * the quickest way to lower the count. `stillOld` are the PRs past the age - * limit that merging the ready ones won't clear, so it never promises - * "you're out" while one is left. */ -// ponytail: when more are ready than needed, an old ready PR left unmerged -// still holds you; the line doesn't say which ready ones to pick -function countWhy( - n: number, - max: number, - ready: number, - stillOld: JailPull[], - maxDays: number -): string { - const need = n - max; - const head = `${n} open PRs. Parole at ${max}.`; - const yours = ready === 1 ? 'your ready one' : `your ${ready} ready ones`; - const out = stillOld.length ? `to get under ${max}` : 'and you’re out'; - const tail = !stillOld.length - ? '' - : stillOld.length === 1 - ? ` #${stillOld[0].data.number} is still past ${maxDays} days.` - : ` ${stillOld.length} PRs are still past ${maxDays} days.`; - if (!ready) - return `${head} Close ${need} to get ${stillOld.length ? `under ${max}` : 'out'}.${tail}`; - if (ready < need) return `${head} Merge ${yours} and close ${need - ready} more.${tail}`; - if (ready === need) return `${head} Merge ${yours} ${out}.${tail}`; - return `${head} Merge ${need} of ${yours} ${out}.${tail}`; -} - -function ageWhy(old: JailPull[], maxDays: number): string { - if (old.length > 1) { - return `${old.length} PRs open past ${maxDays} days. Merge or close them to get out.`; - } - const [p] = old; - const head = `#${p.data.number} has been open ${p.ageDays} days`; - return isJailReady(p) - ? `${head} and it’s ready. Merge it to get out.` - : `${head}. Parole at ${maxDays} days. Merge or close it to get out.`; -} - -/** - * A jail's PRs split the way My work splits them, by the same rowWord, with - * the ready ones lifted out on top: ready to merge, your move, then waiting - * on others. Each group runs oldest first; the row's word says the rest. - */ -export function jailGroups(pulls: DerivedPull[], me: string) { - const worded = pulls - .map(pull => ({ pull, word: rowWord(pull, me) })) - .sort((a, b) => b.pull.ageDays - a.pull.ageDays); - const rest = worded.filter(w => !isJailReady(w.pull)); - return { - ready: worded.filter(w => isJailReady(w.pull)), - move: rest.filter(w => w.word.kind === 'do'), - waiting: rest.filter(w => w.word.kind === 'wait'), - }; -} - -/** The record to store when `jail` shows at `now`. */ -export const jailRecord = (jail: JailCase, now: number): JailRecord => ({ - at: now, - count: jail.count, - over: jail.over, -}); - -/** A stored record, or null when it's missing or unreadable. */ -export function parseJailRecord(raw: string | null): JailRecord | null { - try { - const r = JSON.parse(raw ?? 'null') as Partial | null; - if (typeof r?.at !== 'number' || typeof r.count !== 'number' || !Array.isArray(r.over)) { - return null; - } - return { at: r.at, count: r.count, over: r.over }; - } catch { - return null; - } -} - -/** - * Whether jail should drop now: you're over a limit, and either it never - * showed, or it's been at least JAIL_COOLDOWN_MS and things got worse since - * (more open PRs, net, or a PR newly past the age limit). - */ -export function jailDue( - jail: JailCase | null, - last: JailRecord | null, - now: number -): boolean { - if (!jail) return false; - if (!last) return true; - if (now - last.at < JAIL_COOLDOWN_MS) return false; - return jail.count > last.count || jail.over.some(id => !last.over.includes(id)); -} diff --git a/frontend-v2/src/settings.test.ts b/frontend-v2/src/settings.test.ts index f66587c4..5f865ff5 100644 --- a/frontend-v2/src/settings.test.ts +++ b/frontend-v2/src/settings.test.ts @@ -1,50 +1,9 @@ -// @vitest-environment jsdom -import { afterEach, describe, expect, it, vi } from 'vitest'; -import { DEFAULT_SETTINGS, migrateSettings } from './settings'; +import { describe, expect, it } from 'vitest'; +import { DEFAULT_SETTINGS } from './settings'; describe('Finish line is opt-in', () => { it('shows only the header count by default, and closes without a hold', () => { - expect(DEFAULT_SETTINGS.jailMode).toBe('manual'); - expect(DEFAULT_SETTINGS.jailHoldToClose).toBe(false); - }); - - it('moves a saved popup from before to the new default, once', () => { - const old = { theme: 'dark' as const, jailMode: 'auto' as const }; - const once = migrateSettings(old); - expect(once).toEqual({ theme: 'dark', jailMode: 'manual', settingsVersion: 1 }); - // turned back on after the move, it stays on - const chosen = { ...once, jailMode: 'auto' as const }; - expect(migrateSettings(chosen)).toBe(chosen); - }); - - it('keeps Off, and carries over the day-old on/off switch', () => { - expect(migrateSettings({ jailMode: 'off' }).jailMode).toBe('off'); - expect(migrateSettings({ jailOn: false, jailMode: 'auto' })).toEqual({ - jailMode: 'off', - settingsVersion: 1, - }); - expect(migrateSettings({ jailOn: true, jailMode: 'auto' })).toEqual({ - jailMode: 'manual', - settingsVersion: 1, - }); - }); -}); - -describe('the saved blob on load', () => { - afterEach(() => { - localStorage.clear(); - vi.resetModules(); - }); - - it('is migrated before the store reads it', async () => { - localStorage.setItem('pd2.settings', JSON.stringify({ jailMode: 'auto', theme: 'dark' })); - vi.resetModules(); - const { getSettings } = await import('./settings'); - expect(getSettings().jailMode).toBe('manual'); - expect(getSettings().theme).toBe('dark'); - expect(JSON.parse(localStorage.getItem('pd2.settings') ?? '{}')).toMatchObject({ - jailMode: 'manual', - settingsVersion: 1, - }); + expect(DEFAULT_SETTINGS.finishLineMode).toBe('manual'); + expect(DEFAULT_SETTINGS.finishLineHoldToClose).toBe(false); }); }); diff --git a/frontend-v2/src/settings.ts b/frontend-v2/src/settings.ts index 5dce2d35..a3fc0a48 100644 --- a/frontend-v2/src/settings.ts +++ b/frontend-v2/src/settings.ts @@ -1,6 +1,6 @@ import { STARVE_DAYS } from '../../shared/model/status'; -import { JAIL_MAX_DAYS, JAIL_MAX_OPEN } from './model/jail'; -import { createPersistentStore, readStorage, writeStorage } from './storage'; +import { FINISH_MAX_DAYS, FINISH_MAX_OPEN } from './model/finishLine'; +import { createPersistentStore } from './storage'; /** one named review circle; DEFAULT_TEAM_NAME is what quick add-to-team * gestures (the row kebab, the People filter) create when no roster exists */ @@ -111,21 +111,19 @@ export interface Settings { * or age tooltip) before it opens, so brushing the pointer across the board * doesn't flash panels open. 0 = open instantly; a click always bypasses it. */ hoverDelayMs: number; - /** Finish line (once "PR jail", hence the stored keys): 'auto' pops it up + /** Finish line: 'auto' pops it up * when you're over the limits, 'manual' only shows the header count * (click to open), 'off' shows neither. Opt-in: 'manual' by default. */ - jailMode: 'auto' | 'manual' | 'off'; + finishLineMode: 'auto' | 'manual' | 'off'; /** Finish line counts you over when you have more than this many open PRs */ - jailMaxOpen: number; + finishLineMaxOpen: number; /** ...or when any of them has been open longer than this many days */ - jailMaxDays: number; + finishLineMaxDays: number; /** whether your draft PRs count toward Finish line */ - jailCountDrafts: boolean; + finishLineCountDrafts: boolean; /** Finish line closes only by holding a button down; Esc and the close * button do nothing. Off by default, for people who want the friction. */ - jailHoldToClose: boolean; - /** which one-time migrations (migrateSettings) this saved blob has had */ - settingsVersion: number; + finishLineHoldToClose: boolean; } /** The heaviest age text tier (red), derived from ageWarnDays rather than @@ -178,54 +176,14 @@ export const DEFAULT_SETTINGS: Settings = { repoQueueCap: 15, openPrsNewTab: true, hoverDelayMs: 250, - jailMode: 'manual', - jailMaxOpen: JAIL_MAX_OPEN, - jailMaxDays: JAIL_MAX_DAYS, - jailCountDrafts: false, - jailHoldToClose: false, - settingsVersion: 1, + finishLineMode: 'manual', + finishLineMaxOpen: FINISH_MAX_OPEN, + finishLineMaxDays: FINISH_MAX_DAYS, + finishLineCountDrafts: false, + finishLineHoldToClose: false, }; -type SavedSettings = Partial & { jailOn?: boolean }; - -/** - * Brings a saved blob up to date; the same object when there's nothing to do. - * - jailOn (an on/off switch, live for a day) became jailMode; an Off carries over. - * - Version 1 made Finish line opt-in. The store saves every field on any - * change, so a saved 'auto' can't tell a choice from the old default: a - * blob from before moves 'auto' to the new default once. Someone who wants - * the popup turns it back on, and it stays. - */ -export function migrateSettings(saved: SavedSettings): SavedSettings { - let next = saved; - if (next.jailOn !== undefined) { - const { jailOn, ...rest } = next; - next = jailOn ? rest : { ...rest, jailMode: 'off' }; - } - if ((next.settingsVersion ?? 0) < 1) { - next = { - ...next, - jailMode: next.jailMode === 'off' ? 'off' : DEFAULT_SETTINGS.jailMode, - settingsVersion: 1, - }; - } - return next; -} - -const SETTINGS_KEY = 'pd2.settings'; -{ - let saved: SavedSettings | null = null; - try { - saved = JSON.parse(readStorage(SETTINGS_KEY) ?? 'null') as SavedSettings | null; - } catch { - // unreadable: the store falls back to the defaults - } - if (saved) { - const next = migrateSettings(saved); - if (next !== saved) writeStorage(SETTINGS_KEY, JSON.stringify(next)); - } -} -const store = createPersistentStore(SETTINGS_KEY, DEFAULT_SETTINGS); +const store = createPersistentStore('pd2.settings', DEFAULT_SETTINGS); /** Union of every personal roster, sorted — "your people": the set that * leads the review queues and wears the teammate corner heart. */ diff --git a/frontend-v2/src/styles.css b/frontend-v2/src/styles.css index ff41a068..a1d15432 100644 --- a/frontend-v2/src/styles.css +++ b/frontend-v2/src/styles.css @@ -1007,10 +1007,10 @@ input[type="checkbox"] { font-weight: 600; } -/* PR jail's snooze key (components/JailMode.tsx): a slow wiggle in the - keyhole while it waits to be held. Transform only, and still when motion - is reduced. */ -@keyframes jail-key-wiggle { +/* Finish line's hold button (components/FinishLine.tsx): the flag gives a + slow wiggle while it waits to be held. Transform only, and still when + motion is reduced. */ +@keyframes finish-flag-wiggle { 0%, 60%, 100% { @@ -1026,31 +1026,12 @@ input[type="checkbox"] { rotate: -6deg; } } -.jail-key-wiggle { - animation: jail-key-wiggle 2.8s ease-in-out infinite; -} -/* ...and the shackle opening once it has turned: up out of the body, then - over its right leg (x 30) to the other side, like a padlock swinging - open. Reduced motion lands on the open lock with no swing. */ -@keyframes jail-shackle-open { - from { - transform: translateY(0) scaleX(1); - } - 35% { - transform: translateY(-5px) scaleX(1); - } - to { - transform: translateY(-5px) scaleX(-1); - } -} -.jail-shackle-open { - transform-origin: 30px 18px; - transform: translateY(-5px) scaleX(-1); - animation: jail-shackle-open 600ms cubic-bezier(0.22, 1, 0.36, 1); +.finish-flag-wiggle { + animation: finish-flag-wiggle 2.8s ease-in-out infinite; } /* ...and a merged or closed PR's row sliding out of the list while the countdown ticks. */ -@keyframes jail-row-leave { +@keyframes finish-row-leave { to { opacity: 0; transform: translateX(1.5rem); @@ -1058,17 +1039,16 @@ input[type="checkbox"] { padding-block: 0; } } -.jail-row-leave { +.finish-row-leave { max-height: 8rem; overflow: hidden; - animation: jail-row-leave 450ms cubic-bezier(0.5, 0, 0.75, 0) forwards; + animation: finish-row-leave 450ms cubic-bezier(0.5, 0, 0.75, 0) forwards; } @media (prefers-reduced-motion: reduce) { - .jail-key-wiggle, - .jail-shackle-open { + .finish-flag-wiggle { animation: none; } - .jail-row-leave { + .finish-row-leave { animation-duration: 1ms; } } From 3d085bd03343fc2835e63d61df3c2156a89aa66c Mon Sep 17 00:00:00 2001 From: Jarred Stelfox Date: Mon, 12 Oct 2026 13:12:32 -0700 Subject: [PATCH 3/6] Lead Finish line with the action and its numbers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit People read "6 to the finish line" and still asked what the finish line was and what to do. The top of the list now says the action with its real number ("Finish 2 PRs", "All finished" when done) and a small row of the numbers behind it that you can act on: "9 open PRs · aim for 7", "2 PRs older than 14 days", "3 PRs ready to merge", each only when above zero. Waiting on others stays out of it. Each number is a button: it opens the groups holding the PRs it counts and jumps to the first. The groups now fold on their titles so that has something to open. The header badge says the action too ("2 to finish", "Finish 2 of your open PRs" on hover). The why and ask sentences are gone, "Only you see this." is one quiet line under the list, and the robot only shows when you're done. Co-Authored-By: Claude Opus 5.5 --- .../src/components/FinishLine.test.tsx | 130 +++++++-- frontend-v2/src/components/FinishLine.tsx | 269 +++++++++++++----- frontend-v2/src/components/Lane.tsx | 4 +- frontend-v2/src/components/Legend.tsx | 12 +- frontend-v2/src/components/Settings.tsx | 20 +- frontend-v2/src/model/finishLine.test.ts | 110 ++++--- frontend-v2/src/model/finishLine.ts | 81 +++--- 7 files changed, 409 insertions(+), 217 deletions(-) diff --git a/frontend-v2/src/components/FinishLine.test.tsx b/frontend-v2/src/components/FinishLine.test.tsx index c8a77936..2fd1403e 100644 --- a/frontend-v2/src/components/FinishLine.test.tsx +++ b/frontend-v2/src/components/FinishLine.test.tsx @@ -76,12 +76,20 @@ function render(pulls: DerivedPull[]) { act(() => r.render()); } -const why = () => document.getElementById('finish-why')?.textContent; -const ask = () => document.getElementById('finish-ask')?.textContent; -const listed = () => document.querySelectorAll('[role="dialog"] li').length; +/** the stats row's buttons, by their words */ +const stats = () => + [...document.querySelectorAll('#finish-stats button')].map( + b => b.textContent + ); +const stat = (text: string) => + [...document.querySelectorAll('#finish-stats button')].find( + b => b.textContent === text + ); +const listed = () => document.querySelectorAll('[role="dialog"] [id$="-rows"] li').length; const dialog = () => document.querySelector('[role="dialog"]'); const countdown = () => document.getElementById('finish-title')?.textContent; -const badge = () => document.querySelector('[aria-label*="to the finish line"]'); +const badge = () => + document.querySelector('[aria-label^="Finish"][aria-label$="open PRs"]'); const esc = () => act(() => { document.body.dispatchEvent( @@ -94,27 +102,27 @@ describe('FinishLine while it is up', () => { it('follows new PRs and merges live', () => { const first = many(8); render(first); - expect(why()).toMatch(/^You have 8 open PRs/); + expect(stats()).toEqual(['8 open PRs · aim for 7']); expect(listed()).toBe(8); // a new PR arrives over the socket render([...first, pull()]); - expect(why()).toMatch(/^You have 9 open PRs/); + expect(stats()).toEqual(['9 open PRs · aim for 7']); expect(listed()).toBe(9); - expect(countdown()).toBe('2 to the finish line'); + expect(countdown()).toBe('Finish 2 PRs'); // merges bring you under the limits: it says so, then lets you go vi.useFakeTimers(); render(first.slice(0, 3)); - expect(why()).toBe('Your open PRs are all under your limits. Nice work.'); - expect(countdown()).toBe('You’re at the finish line'); + expect(stats()).toEqual([]); + expect(countdown()).toBe('All finished'); expect(listed()).toBe(0); act(() => vi.advanceTimersByTime(2500)); expect(dialog()).toBeNull(); }); - it('lifts ready PRs to the top and counts them in the why line', () => { + it('lifts ready PRs to the top and counts them in the stats', () => { const ready = pull(2, { ready: true }); render([...many(7), ready]); // the rest are mine to stamp under self-review @@ -122,10 +130,9 @@ describe('FinishLine while it is up', () => { expect(document.querySelector('[role="dialog"] li a')?.textContent).toContain( ready.data.title ); - expect(why()).toBe( - 'You have 8 open PRs, over the limit of 7. Only you see this: it’s about your own PRs, and nobody else is told.' - ); - expect(ask()).toBe('Merge or close 1 of these to finish. Start with the one ready to merge.'); + expect(countdown()).toBe('Finish 1 PR'); + expect(stats()).toEqual(['8 open PRs · aim for 7', '1 PR ready to merge']); + expect(dialog()?.textContent).toContain('Only you see this.'); }); it('only shows the badge until you turn the popup on', () => { @@ -139,9 +146,10 @@ describe('FinishLine while it is up', () => { setSettings({ finishLineMode: 'manual' }); render(many(9)); expect(document.querySelector('[role="dialog"]')).toBeNull(); - expect(badge()?.getAttribute('aria-label')).toMatch(/^2 to the finish line: You have 9/); + expect(badge()?.getAttribute('aria-label')).toBe('Finish 2 of your open PRs'); + expect(badge()?.textContent).toBe('2 to finish'); act(() => badge()?.click()); - expect(why()).toMatch(/^You have 9 open PRs/); + expect(countdown()).toBe('Finish 2 PRs'); }); it('stays away, badge and all, when it is off', () => { @@ -154,9 +162,9 @@ describe('FinishLine while it is up', () => { it('with the hold setting, closes only when the button is held down', () => { vi.useFakeTimers(); render(many(8)); - const hold = [ - ...document.querySelectorAll('[role="dialog"] button'), - ].find(b => b.textContent?.includes('Hold to close')); + const hold = [...document.querySelectorAll('[role="dialog"] button')].find( + b => b.textContent?.includes('Hold to close') + ); // it has focus from the start, so the keyboard can hold it right away expect(document.activeElement).toBe(hold); const key = (type: string, k = 'Enter') => @@ -180,9 +188,9 @@ describe('FinishLine while it is up', () => { vi.useFakeTimers(); const nine = many(9); render(nine); - expect(countdown()).toBe('2 to the finish line'); + expect(countdown()).toBe('Finish 2 PRs'); render(nine.slice(1)); - expect(countdown()).toBe('1 to the finish line'); + expect(countdown()).toBe('Finish 1 PR'); // still there for the slide, then gone expect(document.querySelectorAll('.finish-row-leave')).toHaveLength(1); expect(listed()).toBe(9); @@ -293,9 +301,11 @@ describe('FinishLine while it is up', () => { expect(dialog()).toBeNull(); act(() => window.dispatchEvent(new Event('pd2:preview-finish-line'))); expect(dialog()).not.toBeNull(); - act(() => document.querySelector('[data-hold]')?.dispatchEvent( - new MouseEvent('click', { bubbles: true, detail: 0 }) - )); + act(() => + document + .querySelector('[data-hold]') + ?.dispatchEvent(new MouseEvent('click', { bubbles: true, detail: 0 })) + ); expect(dialog()).toBeNull(); act(() => { location.hash = 'finish=1'; @@ -320,7 +330,10 @@ describe('FinishLine while it is up', () => { }); it('keeps the last showing while it doesn’t know who you are', () => { - localStorage.setItem('pd2.finishLine', JSON.stringify({ at: Date.now(), count: 9, over: [] })); + localStorage.setItem( + 'pd2.finishLine', + JSON.stringify({ at: Date.now(), count: 9, over: [] }) + ); const r = (root ??= createRoot(host)); act(() => r.render()); expect(localStorage.getItem('pd2.finishLine')).not.toBeNull(); @@ -333,7 +346,10 @@ describe('FinishLine while it is up', () => { act(() => r.render() ); - localStorage.setItem('pd2.finishLine', JSON.stringify({ at: Date.now(), count: 8, over: [] })); + localStorage.setItem( + 'pd2.finishLine', + JSON.stringify({ at: Date.now(), count: 8, over: [] }) + ); act(() => r.render()); expect(dialog()).toBeNull(); }); @@ -381,9 +397,7 @@ describe('FinishLine while it is up', () => { window.addEventListener('pd2:open-settings', listen); render(many(9)); act(() => - document - .querySelector('[aria-label="Finish line settings"]') - ?.click() + document.querySelector('[aria-label="Finish line settings"]')?.click() ); window.removeEventListener('pd2:open-settings', listen); expect(asked).toEqual(['finish-line']); @@ -391,6 +405,64 @@ describe('FinishLine while it is up', () => { }); }); +describe('FinishLine stats', () => { + beforeEach(() => setSettings({ finishLineHoldToClose: false })); + + it('shows only the numbers above zero, old PRs alone when the count is fine', () => { + render([pull(20), pull(30), pull(1)]); + expect(countdown()).toBe('Finish 2 PRs'); + expect(stats()).toEqual(['2 PRs older than 14 days']); + // the list is the old ones only + expect(listed()).toBe(2); + }); + + it('leaves waiting on others out of the stats', () => { + render([pull(2, { asked: ['bob'] }), ...many(7)]); + expect(headers()).toEqual(['Your move', 'Waiting on others']); + expect(stats()).toEqual(['8 open PRs · aim for 7']); + }); + + it('opens a folded group when its stat is clicked, and jumps to it', () => { + const scrolled = vi.fn(); + Element.prototype.scrollIntoView = scrolled; + render([pull(2, { ready: true }), pull(20, { asked: ['bob'] }), ...many(7)]); + expect(stats()).toEqual([ + '9 open PRs · aim for 7', + '1 PR older than 14 days', + '1 PR ready to merge', + ]); + const toggle = (title: string) => + [...document.querySelectorAll('h3 button')].find( + b => b.textContent === title + ); + const rows = (title: string) => + document.getElementById(toggle(title)?.getAttribute('aria-controls') ?? ''); + // fold two groups by their titles + act(() => toggle('Waiting on others')?.click()); + act(() => toggle('Ready to merge')?.click()); + expect(toggle('Waiting on others')?.getAttribute('aria-expanded')).toBe('false'); + expect(rows('Waiting on others')?.hidden).toBe(true); + expect(rows('Ready to merge')?.hidden).toBe(true); + // the old PR waits on bob: its stat opens that group, and only that one + act(() => stat('1 PR older than 14 days')?.click()); + expect(rows('Waiting on others')?.hidden).toBe(false); + expect(rows('Ready to merge')?.hidden).toBe(true); + expect(scrolled.mock.contexts.at(-1)).toBe(rows('Waiting on others')?.parentElement); + expect(document.activeElement).toBe(toggle('Waiting on others')); + act(() => stat('1 PR ready to merge')?.click()); + expect(rows('Ready to merge')?.hidden).toBe(false); + // the open count opens them all + act(() => toggle('Ready to merge')?.click()); + act(() => toggle('Your move')?.click()); + act(() => stat('9 open PRs · aim for 7')?.click()); + expect( + [...document.querySelectorAll('[role="dialog"] ul[id]')].every( + u => !(u as HTMLElement).hidden + ) + ).toBe(true); + }); +}); + describe('Settings, opened from the cog', () => { it('scrolls to and focuses the Finish line settings', async () => { const { Settings } = await import('./Settings'); diff --git a/frontend-v2/src/components/FinishLine.tsx b/frontend-v2/src/components/FinishLine.tsx index 0a189bdb..790f2915 100644 --- a/frontend-v2/src/components/FinishLine.tsx +++ b/frontend-v2/src/components/FinishLine.tsx @@ -1,6 +1,6 @@ import { useEffect, useMemo, useRef, useState } from 'react'; import { createPortal } from 'react-dom'; -import { Flag, Settings as SettingsIcon, X } from 'lucide-react'; +import { ChevronRight, Flag, Settings as SettingsIcon, X } from 'lucide-react'; import type { DerivedPull } from '../../../shared/model/status'; import { githubUrl } from '../../../shared/format'; import { askedOf, askForInputHint, rowNote, rowWord } from '../model/actions'; @@ -10,13 +10,16 @@ import { finishDue, finishGroups, finishRecord, + finishStats, parseFinishRecord, + prCount, FINISH_LINE_KEY, type FinishLineCase, + type FinishStat, } from '../model/finishLine'; import { useSettings } from '../settings'; import { readStorage, writeStorage } from '../storage'; -import { HeaderIconButton, QuietButton, RepoRef } from './bits'; +import { QuietButton, RepoRef } from './bits'; import { Icon } from './Icon'; import { modalOpen } from '../hooks'; import { GroupHeader } from './Lane'; @@ -197,16 +200,20 @@ export function FinishLine({ return ( <> {found && ( - show(found)} - className="relative" + className="pressable inline-flex h-8 items-center gap-1.5 rounded-lg border border-line bg-surface px-2.5 text-[13px] font-medium whitespace-nowrap text-ink-2 hover:text-brand" > - - {found.toGo > 9 ? '9+' : found.toGo} + + + {found.toGo} to + finish - + )} {shown && createPortal( @@ -216,6 +223,7 @@ export function FinishLine({ (shown === 'preview' ? sampleCase(pulls, me, extraBots, settings) : FREE) } me={me} + maxOpen={settings.finishLineMaxOpen} maxDays={settings.finishLineMaxDays} hold={settings.finishLineHoldToClose} openIds={openIds} @@ -232,17 +240,15 @@ export function FinishLine({ ); } -/** How long "You're at the finish line" shows before it closes on its own. */ +/** How long "All finished" shows before it closes on its own. */ const FREED_MS = 2500; -/** "3 to the finish line", the countdown's words everywhere it shows. */ -const toFinish = (n: number) => `${n} to the finish line`; +/** The badge's full words, for its hover and screen readers. */ +const finishLabel = (n: number) => `Finish ${n} of your open PRs`; /** Merged or closed your way under the limits while it was up. */ const FREE: FinishLineCase = { pulls: [], - why: 'Your open PRs are all under your limits. Nice work.', - ask: '', ready: 0, count: 0, over: [], @@ -251,7 +257,7 @@ const FREE: FinishLineCase = { /** The preview when you're under the limits: your PRs as it counts them (any * open one trips a limit of 0), or the board's oldest when you have none, - * so there's something to look at. */ + * so there's something to look at. Nothing to finish: toGo is 0. */ function sampleCase( pulls: DerivedPull[], me: string, @@ -263,14 +269,12 @@ function sampleCase( maxDays: settings.finishLineMaxDays, countDrafts: settings.finishLineCountDrafts, }); - if (mine) return { ...mine, why: 'Preview: your open PRs, as it would list them.', ask: '' }; + if (mine) return { ...mine, toGo: 0 }; return { pulls: pulls .filter(p => p.data.state === 'open') .sort((a, b) => b.ageDays - a.ageDays) .slice(0, 6), - why: 'Preview: these are sample PRs from the board.', - ask: '', ready: 0, count: 0, over: [], @@ -288,15 +292,22 @@ const LEAVE_MS = 450; const iconButton = 'pressable inline-flex h-8 w-8 items-center justify-center rounded-lg text-ink-3 hover:bg-muted hover:text-ink'; +/** The list's groups, in order: the quickest way to finish first. */ +const GROUP_KEYS = ['ready', 'move', 'waiting'] as const; +type GroupKey = typeof GROUP_KEYS[number]; +const groupDomId = (k: GroupKey) => `finish-group-${k}`; + /** The modal over the board: a centered card on a desktop, a sheet up from - * the bottom on a phone. Its header stays put while the list scrolls under - * it. Esc or the close button puts it away; with `hold` on, neither does, and - * the way out besides getting under the limits is holding the button at the - * bottom down for a couple of seconds, by pointer, finger, Enter or Space. - * The cog opens Settings at this feature's own settings. */ + * the bottom on a phone. Its top is the action and its numbers ("Finish 2 + * PRs", then the stats that say why); the list scrolls under it. Esc or the + * close button puts it away; with `hold` on, neither does, and the way out + * besides getting under the limits is holding the button at the bottom down + * for a couple of seconds, by pointer, finger, Enter or Space. The cog opens + * Settings at this feature's own settings. */ function FinishModal({ found, me, + maxOpen, maxDays, hold, openIds, @@ -304,6 +315,7 @@ function FinishModal({ }: { found: FinishLineCase; me: string; + maxOpen: number; maxDays: number; hold: boolean; openIds: ReadonlySet; @@ -311,6 +323,26 @@ function FinishModal({ }) { const rootRef = useRef(null); const free = found === FREE; + const stats = finishStats(found, { maxOpen, maxDays }); + // groups folded by hand; a stat opens the ones holding what it counts + const [closed, setClosed] = useState>(new Set()); + const toggle = (k: GroupKey) => + setClosed(c => { + const next = new Set(c); + if (!next.delete(k)) next.add(k); + return next; + }); + const goTo = (stat: FinishStat) => { + const groups = finishGroups(found.pulls, me); + const keys = GROUP_KEYS.filter(k => groups[k].some(r => stat.counts(r.pull))); + if (!keys.length) return; + setClosed(c => new Set([...c].filter(k => !keys.includes(k)))); + const el = document.getElementById(groupDomId(keys[0])); + el?.scrollIntoView?.({ block: 'start' }); + el?.querySelector('[aria-expanded]')?.focus({ + preventScroll: true, + }); + }; // the key handler is set up once; it reads these fresh const latest = useRef({ hold, onDone }); latest.current = { hold, onDone }; @@ -371,30 +403,45 @@ function FinishModal({ role="dialog" aria-modal="true" aria-labelledby="finish-title" - aria-describedby="finish-why finish-ask" + aria-describedby="finish-stats" className="fixed inset-0 z-[200] flex flex-col items-center justify-end bg-ink/40 pt-8 outline-none sm:justify-start sm:px-4 sm:pt-[8vh] sm:pb-8" >

    -
    - +
    + {free && }
    - {/* the countdown: ticks down live as you merge or close */} -

    + {/* the action with its real number, ticking down live as + you merge or close */} +

    {free - ? 'You’re at the finish line' + ? 'All finished' : found.toGo - ? toFinish(found.toGo) - : 'Finish line'} + ? `Finish ${prCount(found.toGo)}` + : 'Nothing to finish'}

    -

    - {found.why} - {found.count > 0 && - ' Only you see this: it’s about your own PRs, and nobody else is told.'} -

    + {/* why, in numbers you can act on: each one jumps to its PRs */} + {!free && stats.length > 0 && ( +
      + {stats.map(stat => ( +
    • + +
      • + ))} +
    + )}
    - {found.ask && ( -

    - {found.ask} -

    - )} {!free && ( <> {/* the board's own sticky group headers, pinned to the top of this list on the card's color instead of under the app header on the canvas */}
    - +
    +

    Only you see this.

    {hold && } )} @@ -561,7 +613,13 @@ function HoldToClose({ onDone }: { onDone: () => void }) { - {opened ? 'Closing' : holding ? 'Keep holding…' : tapped ? 'Hold it down' : 'Hold to close'} + {opened + ? 'Closing' + : holding + ? 'Keep holding…' + : tapped + ? 'Hold it down' + : 'Hold to close'} ); @@ -640,11 +698,16 @@ function FinishList({ me, maxDays, openIds, + closed, + onToggle, }: { pulls: DerivedPull[]; me: string; maxDays: number; openIds: ReadonlySet; + /** the groups folded shut */ + closed: ReadonlySet; + onToggle: (k: GroupKey) => void; }) { const leaving = useLeaving(pulls, openIds); const gone = new Set(leaving.map(pullId)); @@ -660,9 +723,21 @@ function FinishList({ const asked = (rows: { pull: DerivedPull }[]) => live(rows).filter(r => askedOf(r.pull).length); const nudgeRows = asked([...groups.move, ...groups.waiting]); const sections = [ - { title: STAGE_WORDS.ready, sub: 'Merge these first', rows: groups.ready, ready: true }, - { title: 'Your move', sub: 'Waiting on you', rows: groups.move }, { + key: 'ready' as const, + title: STAGE_WORDS.ready, + sub: 'Merge these first', + rows: groups.ready, + ready: true, + }, + { + key: 'move' as const, + title: 'Your move', + sub: 'Waiting on you', + rows: groups.move, + }, + { + key: 'waiting' as const, title: 'Waiting on others', sub: 'Nudge whoever you asked, or close what you don’t need', rows: groups.waiting, @@ -671,32 +746,64 @@ function FinishList({ const nudgeOn = sections.find(g => !g.ready && asked(g.rows).length)?.title; return (
    - {sections.map(g => ( -
    - {mine && ( - : undefined} - /> - )} -
      - {g.rows.map(({ pull }) => ( - { + const open = !mine || !closed.has(g.key); + return ( +
      + {mine && ( + onToggle(g.key)} + className="hit -ml-0.5 inline-flex items-center gap-1 rounded border-0 bg-transparent p-0 text-left font-semibold text-inherit" + > + + {g.title} + + } + sub={g.sub} + count={live(g.rows).length} + headerExtra={ + g.title === nudgeOn ? : undefined + } /> - ))} -
    -
    - ))} + )} + +
    + ); + })} ); } @@ -814,7 +921,11 @@ function FinishMark() { className="fill-surface stroke-ink-2" strokeWidth={2} /> - + {/* body, one arm up at the flag, one down */} - + ); diff --git a/frontend-v2/src/components/Lane.tsx b/frontend-v2/src/components/Lane.tsx index eed26c64..3aedf2d5 100644 --- a/frontend-v2/src/components/Lane.tsx +++ b/frontend-v2/src/components/Lane.tsx @@ -112,7 +112,9 @@ export function GroupHeader({ headerExtra, level = 2, }: { - title: string; + /** a string, or a node when the title is itself a control (Finish + * line's groups fold on their title) */ + title: ReactNode; /** plain string for most lanes; a ReactNode when the sub-line itself is * the door to more detail (Review's queue wraps its ranking one-liner in * a popover — existing text becomes interactive, zero new chrome) */ diff --git a/frontend-v2/src/components/Legend.tsx b/frontend-v2/src/components/Legend.tsx index 351cbba6..c1942a0f 100644 --- a/frontend-v2/src/components/Legend.tsx +++ b/frontend-v2/src/components/Legend.tsx @@ -163,18 +163,18 @@ export function Legend() { - - - 2 + + + + 2 to finish } - def="shows in the header when your own open PRs pass your limits (by default more than 7 open, or one open over 14 days): the number is how many to merge or close. Click it for the list, ready ones first. Only you see it; nobody else is told" + def="shows in the header when your own open PRs pass your limits (by default more than 7 open, or one open over 14 days): how many to merge or close. Click it for the list, ready ones first. Only you see it; nobody else is told" /> Settings} - def="under Finish line: Pop up opens the list by itself when you go over (off by default), Header count only waits for a click, Off hides it. Esc or ✕ closes the list" + def="under Finish line, or the cog in the list: Pop up opens the list by itself when you go over (off by default), Header count only waits for a click, Off hides it. Esc or ✕ closes the list" /> diff --git a/frontend-v2/src/components/Settings.tsx b/frontend-v2/src/components/Settings.tsx index e5a5026e..0104260c 100644 --- a/frontend-v2/src/components/Settings.tsx +++ b/frontend-v2/src/components/Settings.tsx @@ -131,7 +131,9 @@ export function Settings({ // whose detail names a Group's id opens at that group (Finish line's cog) useEffect(() => { const openIt = (e: Event) => { - setSection(typeof (e as CustomEvent).detail === 'string' ? (e as CustomEvent).detail : null); + setSection( + typeof (e as CustomEvent).detail === 'string' ? (e as CustomEvent).detail : null + ); setOpen(true); }; window.addEventListener('pd2:open-settings', openIt); @@ -395,12 +397,12 @@ export function Settings({ - A count in the header of your own open PRs past your limits, and a list of - them to merge or close. Only you see it. + A count in the header of your own open PRs past your limits, and a list + of them to merge or close. Only you see it. set({ finishLineMaxDays })} /> - + set({ finishLineHoldToClose: v === 'on' })} /> - + { // close Settings first so the list isn't stacked on it diff --git a/frontend-v2/src/model/finishLine.test.ts b/frontend-v2/src/model/finishLine.test.ts index 3c78cd3d..43f4a901 100644 --- a/frontend-v2/src/model/finishLine.test.ts +++ b/frontend-v2/src/model/finishLine.test.ts @@ -4,6 +4,7 @@ import { finishLineCase, finishDue, finishRecord, + finishStats, parseFinishRecord, type FinishPull, } from './finishLine'; @@ -15,7 +16,12 @@ let n = 0; function pull( login: string, ageDays: number, - o: { draft?: boolean; deployBlockedBy?: string[]; ready?: boolean; cryo?: boolean } = {} + o: { + draft?: boolean; + deployBlockedBy?: string[]; + ready?: boolean; + cryo?: boolean; + } = {} ): FinishPull { n += 1; return { @@ -38,65 +44,14 @@ const many = (count: number, login = 'me', ageDays = 1) => Array.from({ length: count }, () => pull(login, ageDays)); describe('finishLineCase', () => { - const line = (c: ReturnType) => `${c?.why} ${c?.ask}`; - - // [open, ready] -> the why and ask lines; limit 7 - it.each([ - [20, 0, 'You have 20 open PRs, over the limit of 7. Merge or close 13 of these to finish.'], - [ - 10, - 3, - 'You have 10 open PRs, over the limit of 7. Merge or close 3 of these to finish. Start with the ones ready to merge.', - ], - [ - 8, - 1, - 'You have 8 open PRs, over the limit of 7. Merge or close 1 of these to finish. Start with the one ready to merge.', - ], - ])('with %i open and %i ready, says how many and where to start', (open, ready, want) => { - const pulls = [ - ...many(open - ready), - ...Array.from({ length: ready }, () => pull('me', 1, { ready: true })), - ]; - const c = finishLineCase(pulls, 'me', NO_BOTS, LIMITS); - expect(line(c)).toBe(want); - expect(c?.ready).toBe(ready); + it('counts the ready ones it lists', () => { + const pulls = [...many(7), pull('me', 1, { ready: true }), pull('me', 2, { ready: true })]; + expect(finishLineCase(pulls, 'me', NO_BOTS, LIMITS)?.ready).toBe(2); }); it('never counts a parked PR as ready', () => { const parked = pull('me', 1, { ready: true, cryo: true }); - const c = finishLineCase([...many(7), parked], 'me', NO_BOTS, LIMITS); - expect(c?.ready).toBe(0); - expect(c?.ask).toBe('Merge or close 1 of these to finish.'); - }); - - it('names the old PRs when they’re part of the count', () => { - // 9 open and 1 old: 2 to go, the old one among them - const c = finishLineCase([...many(8), pull('me', 40)], 'me', NO_BOTS, LIMITS); - expect(line(c)).toBe( - 'You have 9 open PRs, over the limit of 7, and 1 is over 14 days old. Merge or close 2 of these to finish, the one over 14 days among them.' - ); - // 9 open and 3 old: closing the old ones is the whole ask - const three = [...many(6), pull('me', 20), pull('me', 30), pull('me', 40)]; - expect(finishLineCase(three, 'me', NO_BOTS, LIMITS)?.ask).toBe( - 'Merge or close the 3 over 14 days to finish.' - ); - }); - - it('says so when every one is old', () => { - expect(finishLineCase([pull('me', 20), pull('me', 30)], 'me', NO_BOTS, LIMITS)?.why).toBe( - 'You have 2 open PRs, all over 14 days old.' - ); - expect(finishLineCase([pull('me', 20)], 'me', NO_BOTS, LIMITS)?.why).toBe( - 'You have 1 open PR, over 14 days old.' - ); - }); - - it('lists only the old PRs when the count is fine', () => { - const old = pull('me', 30, { ready: true }); - expect(line(finishLineCase([pull('me', 1), old], 'me', NO_BOTS, LIMITS))).toBe( - 'You have 2 open PRs, and 1 is over 14 days old. Merge or close the one over 14 days to finish. Start with the one ready to merge.' - ); + expect(finishLineCase([...many(7), parked], 'me', NO_BOTS, LIMITS)?.ready).toBe(0); }); it('counts what’s left to close, the old ones included', () => { @@ -138,13 +93,20 @@ describe('finishLineCase', () => { it('ignores drafts unless they count', () => { const drafts = Array.from({ length: 9 }, () => pull('me', 90, { draft: true })); expect(finishLineCase(drafts, 'me', NO_BOTS, LIMITS)).toBeNull(); - const counted = finishLineCase(drafts, 'me', NO_BOTS, { ...LIMITS, countDrafts: true }); + const counted = finishLineCase(drafts, 'me', NO_BOTS, { + ...LIMITS, + countDrafts: true, + }); expect(counted?.pulls).toHaveLength(9); }); it('ignores bots, by suffix and by the config list', () => { - expect(finishLineCase(many(9, 'dependabot[bot]'), 'dependabot[bot]', NO_BOTS, LIMITS)).toBeNull(); - expect(finishLineCase(many(9, 'renovate'), 'renovate', new Set(['renovate']), LIMITS)).toBeNull(); + expect( + finishLineCase(many(9, 'dependabot[bot]'), 'dependabot[bot]', NO_BOTS, LIMITS) + ).toBeNull(); + expect( + finishLineCase(many(9, 'renovate'), 'renovate', new Set(['renovate']), LIMITS) + ).toBeNull(); }); it('ignores PRs held for a deploy', () => { @@ -182,7 +144,9 @@ describe('finishDue', () => { }); it('re-shows after 4 hours when you have more open PRs', () => { - expect(finishDue(over([...eight, pull('me', 1)]), shownAt, T + FINISH_COOLDOWN_MS)).toBe(true); + expect(finishDue(over([...eight, pull('me', 1)]), shownAt, T + FINISH_COOLDOWN_MS)).toBe( + true + ); }); it('does not re-show after 4 hours when it is the same or better', () => { @@ -209,3 +173,29 @@ describe('finishDue', () => { expect(parseFinishRecord(JSON.stringify(shownAt))).toEqual(shownAt); }); }); + +describe('finishStats', () => { + const texts = (c: Parameters[0]) => finishStats(c, LIMITS).map(s => s.text); + + it('shows each number only when it is above zero', () => { + expect(texts({ count: 9, over: ['a', 'b'], ready: 3 })).toEqual([ + '9 open PRs · aim for 7', + '2 PRs older than 14 days', + '3 PRs ready to merge', + ]); + expect(texts({ count: 9, over: [], ready: 0 })).toEqual(['9 open PRs · aim for 7']); + expect(texts({ count: 3, over: ['a'], ready: 1 })).toEqual([ + '1 PR older than 14 days', + '1 PR ready to merge', + ]); + expect(texts({ count: 7, over: [], ready: 0 })).toEqual([]); + }); + + it('points each number at the PRs it counts', () => { + const [, old, ready] = finishStats({ count: 9, over: ['a'], ready: 1 }, LIMITS); + expect(old.counts(pull('me', 15))).toBe(true); + expect(old.counts(pull('me', 14))).toBe(false); + expect(ready.counts(pull('me', 1, { ready: true }))).toBe(true); + expect(ready.counts(pull('me', 1))).toBe(false); + }); +}); diff --git a/frontend-v2/src/model/finishLine.ts b/frontend-v2/src/model/finishLine.ts index b8c6517b..bb79c05a 100644 --- a/frontend-v2/src/model/finishLine.ts +++ b/frontend-v2/src/model/finishLine.ts @@ -41,10 +41,6 @@ export const isFinishReady = (p: FinishPull) => p.status === 'ready' && !p.cryo; export interface FinishLineCase

    { /** the PRs that put you there, oldest first */ pulls: P[]; - /** why you're seeing it, in numbers: "You have 9 open PRs, ..." */ - why: string; - /** exactly what we ask: how many to merge or close, and where to start */ - ask: string; /** how many of `pulls` can merge now */ ready: number; /** how many open PRs counted */ @@ -93,45 +89,11 @@ export function finishLineCase

    ( const ready = listed.filter(isFinishReady).length; return { pulls: listed, - why: whyLine(mine.length, limits.maxOpen, old.length, limits.maxDays), - ask: askLine(counts.toGo, old.length, ready, limits.maxDays), ready, ...counts, }; } -/** "You have 9 open PRs, over the limit of 7, and 2 are over 14 days old." */ -function whyLine(n: number, maxOpen: number, old: number, maxDays: number): string { - const prs = n === 1 ? '1 open PR' : `${n} open PRs`; - const over = n > maxOpen ? `, over the limit of ${maxOpen}` : ''; - const aged = !old - ? '' - : old === n - ? `, ${n === 1 ? '' : 'all '}over ${maxDays} days old` - : `, and ${old} ${old === 1 ? 'is' : 'are'} over ${maxDays} days old`; - return `You have ${prs}${over}${aged}.`; -} - -/** "Merge or close 2 of these to finish. Start with the ones ready to merge." - * Every PR past the age limit has to go, so the line names them when they - * are part of the count. */ -// ponytail: a ready PR that isn't old doesn't clear an old one; the start -// hint doesn't say which ready ones to pick -function askLine(toGo: number, old: number, ready: number, maxDays: number): string { - const theOld = old === 1 ? `the one over ${maxDays} days` : `the ${old} over ${maxDays} days`; - const what = !old - ? `${toGo} of these to finish` - : old === toGo - ? `${theOld} to finish` - : `${toGo} of these to finish, ${theOld} among them`; - const start = !ready - ? '' - : ready === 1 - ? ' Start with the one ready to merge.' - : ' Start with the ones ready to merge.'; - return `Merge or close ${what}.${start}`; -} - /** * The listed PRs split the way My work splits them, by the same rowWord, with * the ready ones lifted out on top: ready to merge, your move, then waiting @@ -184,3 +146,46 @@ export function finishDue( if (now - last.at < FINISH_COOLDOWN_MS) return false; return found.count > last.count || found.over.some(id => !last.over.includes(id)); } + +/** One number at the top of the list you can act on, and which PRs it + * points at, so clicking it can open their group. */ +export interface FinishStat { + key: 'open' | 'old' | 'ready'; + text: string; + /** the PRs this number counts */ + counts: (p: FinishPull) => boolean; +} + +/** "1 PR", "2 PRs" */ +export const prCount = (n: number) => (n === 1 ? '1 PR' : `${n} PRs`); + +/** + * The numbers you can act on, each only when above zero: how many open + * against the aim, how many past the age limit, how many can merge now. + * Waiting on others is left out: there's nothing to do about it here. + */ +export function finishStats( + found: Pick, 'count' | 'over' | 'ready'>, + limits: { maxOpen: number; maxDays: number } +): FinishStat[] { + const stats: FinishStat[] = []; + if (found.count > limits.maxOpen) + stats.push({ + key: 'open', + text: `${found.count} open PRs · aim for ${limits.maxOpen}`, + counts: () => true, + }); + if (found.over.length) + stats.push({ + key: 'old', + text: `${prCount(found.over.length)} older than ${limits.maxDays} days`, + counts: p => p.ageDays > limits.maxDays, + }); + if (found.ready) + stats.push({ + key: 'ready', + text: `${prCount(found.ready)} ready to merge`, + counts: isFinishReady, + }); + return stats; +} From af6cdece4a65d9d9bef0832b17498cc65f4a6511 Mon Sep 17 00:00:00 2001 From: Jarred Stelfox Date: Mon, 12 Oct 2026 13:16:59 -0700 Subject: [PATCH 4/6] Say what "All finished" means, dim in dark mode Once the robot sat beside "All finished" alone, the card was a tall picture and a title, with nothing saying what was finished. A short line says it: none of your open PRs are past your limits. The scrim was ink at 40%, which in dark mode is a light ink and washed the board pale instead of dimming it. It now matches Settings' black at 30%. Co-Authored-By: Claude Opus 5.5 --- frontend-v2/src/components/FinishLine.test.tsx | 1 + frontend-v2/src/components/FinishLine.tsx | 11 ++++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/frontend-v2/src/components/FinishLine.test.tsx b/frontend-v2/src/components/FinishLine.test.tsx index 2fd1403e..cb41cb00 100644 --- a/frontend-v2/src/components/FinishLine.test.tsx +++ b/frontend-v2/src/components/FinishLine.test.tsx @@ -117,6 +117,7 @@ describe('FinishLine while it is up', () => { render(first.slice(0, 3)); expect(stats()).toEqual([]); expect(countdown()).toBe('All finished'); + expect(dialog()?.textContent).toContain('None of your open PRs are past your limits.'); expect(listed()).toBe(0); act(() => vi.advanceTimersByTime(2500)); expect(dialog()).toBeNull(); diff --git a/frontend-v2/src/components/FinishLine.tsx b/frontend-v2/src/components/FinishLine.tsx index 790f2915..e223ed5f 100644 --- a/frontend-v2/src/components/FinishLine.tsx +++ b/frontend-v2/src/components/FinishLine.tsx @@ -404,10 +404,10 @@ function FinishModal({ aria-modal="true" aria-labelledby="finish-title" aria-describedby="finish-stats" - className="fixed inset-0 z-[200] flex flex-col items-center justify-end bg-ink/40 pt-8 outline-none sm:justify-start sm:px-4 sm:pt-[8vh] sm:pb-8" + className="fixed inset-0 z-[200] flex flex-col items-center justify-end bg-black/30 pt-8 outline-none sm:justify-start sm:px-4 sm:pt-[8vh] sm:pb-8" >

    -
    +
    {free && }
    {/* the action with its real number, ticking down live as @@ -423,6 +423,11 @@ function FinishModal({ ? `Finish ${prCount(found.toGo)}` : 'Nothing to finish'} + {free && ( +

    + None of your open PRs are past your limits. +

    + )} {/* why, in numbers you can act on: each one jumps to its PRs */} {!free && stats.length > 0 && (
      )}
    -
    +
    - )} - {shown && - createPortal( - { - // what you saw last is what the next showing compares - // against, so PRs opened while it was up don't re-drop it - if (shown === 'real' && latest.current) remember(latest.current); - setShown(null); - }} - />, - document.body - )} - - ); + const settings = useSettings(); + // read on the first render: the app rewrites the hash right after and + // drops the unknown param + const [preview, setPreview] = useState(hasPreviewFlag); + // which list is open, not a copy of it: the list below is read live, so a + // merge or a new PR shows up while it's open + const [shown, setShown] = useState<"real" | "preview" | null>(null); + // the last showing, kept here too so blocked storage still holds it for + // this page load; another tab's showing arrives by the storage event + const record = useRef(parseFinishRecord(readStorage(FINISH_LINE_KEY))); + + // where you stand whatever the mode, for the preview: with it off, a + // preview still shows your real numbers + const real = useMemo( + () => + // no case until we know who you are: an empty `me` (a failed whoami) + // would read as "under the limits" and wipe the last showing + initialized && me + ? finishLineCase(pulls, me, extraBots, { + maxOpen: settings.finishLineMaxOpen, + maxDays: settings.finishLineMaxDays, + countDrafts: settings.finishLineCountDrafts, + }) + : null, + [initialized, pulls, me, extraBots, settings] + ); + const found = settings.finishLineMode === "off" ? null : real; + + // under the limits again: zero what the last showing counted, so going + // over later is worse than it (a fresh offense), but keep when it showed, + // so it still comes back at most every 4 hours + useEffect(() => { + const last = record.current; + if ( + !initialized || + !me || + found || + settings.finishLineMode === "off" || + !last + ) + return; + if (!last.count && !last.over.length) return; + record.current = { at: last.at, count: 0, over: [] }; + writeStorage(FINISH_LINE_KEY, JSON.stringify(record.current)); + }, [initialized, me, found, settings.finishLineMode]); + + // the latest case, for closing: the button's hold started seconds ago, and + // what you saw last is what the next showing compares against + const latest = useRef(found); + latest.current = found; + + // open PRs on the whole board, so a row leaving the list only slides + // out as merged when it really closed (not when the rule switched) + const openIds = useMemo( + () => new Set(pulls.filter((p) => p.data.state === "open").map(pullId)), + [pulls] + ); + + const remember = (c: FinishLineCase) => { + record.current = finishRecord(c, Date.now()); + writeStorage(FINISH_LINE_KEY, JSON.stringify(record.current)); + }; + const show = (c: FinishLineCase) => { + remember(c); + setShown("real"); + }; + + useEffect(() => { + const onHash = () => { + if (hasPreviewFlag()) setPreview(true); + }; + // Settings' "Preview it": an event, since setting the same hash + // twice fires nothing + const onPreview = () => setPreview(true); + window.addEventListener("pd2:preview-finish-line", onPreview); + const onStorage = (e: StorageEvent) => { + if (e.key === FINISH_LINE_KEY) + record.current = parseFinishRecord(e.newValue); + }; + window.addEventListener("hashchange", onHash); + window.addEventListener("storage", onStorage); + return () => { + window.removeEventListener("hashchange", onHash); + window.removeEventListener("pd2:preview-finish-line", onPreview); + window.removeEventListener("storage", onStorage); + }; + }, []); + + // the preview never writes the record, so it can't stand in for a real one + useEffect(() => { + if (!initialized || !preview) return; + setPreview(false); + setShown("preview"); + }, [initialized, preview]); + + // checked on load, on every data change, and when the tab comes back; a + // hidden tab or a focused field waits for the next one, so it never lands + // mid-typing or out of sight + useEffect(() => { + if (!found || shown || settings.finishLineMode !== "auto") return; + const check = () => { + if (document.visibilityState === "hidden") return; + // another dialog is up (Settings), or a drag is under way: it + // would land on top of it, and swallow the Esc that cancels a drag + if (modalOpen() || pointerBusy()) return; + if (document.activeElement?.matches("input, textarea, select")) return; + // read the record fresh: a second visible window may have shown it + // a moment ago, before its storage event reached this one + record.current = + parseFinishRecord(readStorage(FINISH_LINE_KEY)) ?? record.current; + if (!finishDue(found, record.current, Date.now())) return; + show(found); + }; + check(); + document.addEventListener("visibilitychange", check); + window.addEventListener("focus", check); + return () => { + document.removeEventListener("visibilitychange", check); + window.removeEventListener("focus", check); + }; + }, [found, shown, settings.finishLineMode]); + + // you got under the limits while it was up: it says so, then lets you go + const freed = shown === "real" && !found; + useEffect(() => { + if (!freed) return; + const t = window.setTimeout(() => setShown(null), FREED_MS); + return () => clearTimeout(t); + }, [freed]); + + return ( + <> + {found && ( + // the action, said as a number and a verb: "2 to finish" + + )} + {shown && + createPortal( + { + // what you saw last is what the next showing compares + // against, so PRs opened while it was up don't re-drop it + if (shown === "real" && latest.current) remember(latest.current); + setShown(null); + }} + />, + document.body + )} + + ); } /** How long "All finished" shows before it closes on its own. */ @@ -248,38 +262,38 @@ const finishLabel = (n: number) => `Finish ${n} of your open PRs`; /** Merged or closed your way under the limits while it was up. */ const FREE: FinishLineCase = { - pulls: [], - ready: 0, - count: 0, - over: [], - toGo: 0, + pulls: [], + ready: 0, + count: 0, + over: [], + toGo: 0, }; /** The preview when you're under the limits: your PRs as it counts them (any * open one trips a limit of 0), or the board's oldest when you have none, * so there's something to look at. Nothing to finish: toGo is 0. */ function sampleCase( - pulls: DerivedPull[], - me: string, - extraBots: ReadonlySet, - settings: { finishLineMaxDays: number; finishLineCountDrafts: boolean } + pulls: DerivedPull[], + me: string, + extraBots: ReadonlySet, + settings: { finishLineMaxDays: number; finishLineCountDrafts: boolean } ): FinishLineCase { - const mine = finishLineCase(pulls, me, extraBots, { - maxOpen: 0, - maxDays: settings.finishLineMaxDays, - countDrafts: settings.finishLineCountDrafts, - }); - if (mine) return { ...mine, toGo: 0 }; - return { - pulls: pulls - .filter(p => p.data.state === 'open') - .sort((a, b) => b.ageDays - a.ageDays) - .slice(0, 6), - ready: 0, - count: 0, - over: [], - toGo: 0, - }; + const mine = finishLineCase(pulls, me, extraBots, { + maxOpen: 0, + maxDays: settings.finishLineMaxDays, + countDrafts: settings.finishLineCountDrafts, + }); + if (mine) return { ...mine, toGo: 0 }; + return { + pulls: pulls + .filter((p) => p.data.state === "open") + .sort((a, b) => b.ageDays - a.ageDays) + .slice(0, 6), + ready: 0, + count: 0, + over: [], + toGo: 0, + }; } /** How long "Hold to close" has to be held down. */ @@ -290,12 +304,7 @@ const OPEN_MS = 700; const LEAVE_MS = 450; const iconButton = - 'pressable inline-flex h-8 w-8 items-center justify-center rounded-lg text-ink-3 hover:bg-muted hover:text-ink'; - -/** The list's groups, in order: the quickest way to finish first. */ -const GROUP_KEYS = ['ready', 'move', 'waiting'] as const; -type GroupKey = typeof GROUP_KEYS[number]; -const groupDomId = (k: GroupKey) => `finish-group-${k}`; + "pressable inline-flex h-8 w-8 items-center justify-center rounded-lg text-ink-3 hover:bg-muted hover:text-ink"; /** The modal over the board: a centered card on a desktop, a sheet up from * the bottom on a phone. Its top is the action and its numbers ("Finish 2 @@ -305,393 +314,406 @@ const groupDomId = (k: GroupKey) => `finish-group-${k}`; * for a couple of seconds, by pointer, finger, Enter or Space. The cog opens * Settings at this feature's own settings. */ function FinishModal({ - found, - me, - maxOpen, - maxDays, - hold, - openIds, - onDone, + found, + me, + maxOpen, + maxDays, + hold, + openIds, + onDone, }: { - found: FinishLineCase; - me: string; - maxOpen: number; - maxDays: number; - hold: boolean; - openIds: ReadonlySet; - onDone: () => void; + found: FinishLineCase; + me: string; + maxOpen: number; + maxDays: number; + hold: boolean; + openIds: ReadonlySet; + onDone: () => void; }) { - const rootRef = useRef(null); - const free = found === FREE; - const stats = finishStats(found, { maxOpen, maxDays }); - // groups folded by hand; a stat opens the ones holding what it counts - const [closed, setClosed] = useState>(new Set()); - const toggle = (k: GroupKey) => - setClosed(c => { - const next = new Set(c); - if (!next.delete(k)) next.add(k); - return next; - }); - const goTo = (stat: FinishStat) => { - const groups = finishGroups(found.pulls, me); - const keys = GROUP_KEYS.filter(k => groups[k].some(r => stat.counts(r.pull))); - if (!keys.length) return; - setClosed(c => new Set([...c].filter(k => !keys.includes(k)))); - const el = document.getElementById(groupDomId(keys[0])); - el?.scrollIntoView?.({ block: 'start' }); - el?.querySelector('[aria-expanded]')?.focus({ - preventScroll: true, - }); - }; - // the key handler is set up once; it reads these fresh - const latest = useRef({ hold, onDone }); - latest.current = { hold, onDone }; - - // "You're at the finish line" unmounts the focused button: keep focus in - // the dialog - useEffect(() => { - if (free) rootRef.current?.focus({ preventScroll: true }); - }, [free]); - - useEffect(() => { - const root = rootRef.current; - // focus goes back where it was (the header flag, say) when it closes - const before = document.activeElement as HTMLElement | null; - // focus starts on the hold button, so holding Enter or Space works - // right away; on the close button when there's no hold - const start = root?.querySelector('[data-hold], [data-close]'); - (start ?? root)?.focus({ preventScroll: true }); - const onKey = (e: KeyboardEvent) => { - // Esc closes it (unless you asked to hold), and must not reach a - // dialog behind this one (Settings) either: listening in the - // capture phase runs first - if (e.key === 'Escape') { - e.preventDefault(); - e.stopPropagation(); - if (!latest.current.hold) latest.current.onDone(); - } - if (e.key !== 'Tab' || !root) return; - const focusables = root.querySelectorAll('a[href], button'); - if (!focusables.length) return; - const first = focusables[0]; - const last = focusables[focusables.length - 1]; - if (e.shiftKey && document.activeElement === first) { - e.preventDefault(); - last.focus(); - } else if (!e.shiftKey && document.activeElement === last) { - e.preventDefault(); - first.focus(); - } else if (!root.contains(document.activeElement)) { - e.preventDefault(); - first.focus(); - } - }; - document.addEventListener('keydown', onKey, true); - const prevOverflow = document.body.style.overflow; - document.body.style.overflow = 'hidden'; - return () => { - document.removeEventListener('keydown', onKey, true); - document.body.style.overflow = prevOverflow; - before?.focus?.({ preventScroll: true }); - }; - }, []); - - return ( -
    -
    -
    - {free && } -
    - {/* the action with its real number, ticking down live as + const rootRef = useRef(null); + const free = found === FREE; + const stats = finishStats(found, { maxOpen, maxDays }); + // a stat jumps to the first PR it counts, in the list's order + const goTo = (stat: FinishStat) => { + const ids = new Set(found.pulls.filter(stat.counts).map(pullId)); + const row = [ + ...(rootRef.current?.querySelectorAll("li[data-pull]") ?? + []), + ].find((li) => ids.has(li.dataset.pull ?? "")); + row?.scrollIntoView?.({ block: "nearest" }); + row?.querySelector("a[href]")?.focus({ preventScroll: true }); + }; + // the key handler is set up once; it reads these fresh + const latest = useRef({ hold, onDone }); + latest.current = { hold, onDone }; + + // "You're at the finish line" unmounts the focused button: keep focus in + // the dialog + useEffect(() => { + if (free) rootRef.current?.focus({ preventScroll: true }); + }, [free]); + + useEffect(() => { + const root = rootRef.current; + // focus goes back where it was (the header flag, say) when it closes + const before = document.activeElement as HTMLElement | null; + // focus starts on the hold button, so holding Enter or Space works + // right away; with no hold, on the dialog itself, so a stray Enter + // can't close it before it's read + const start = root?.querySelector("[data-hold]"); + (start ?? root)?.focus({ preventScroll: true }); + const onKey = (e: KeyboardEvent) => { + // Esc closes it (unless you asked to hold), and must not reach a + // dialog behind this one (Settings) either: listening in the + // capture phase runs first + if (e.key === "Escape") { + e.preventDefault(); + e.stopPropagation(); + if (!latest.current.hold) latest.current.onDone(); + } + if (e.key !== "Tab" || !root) return; + // only what Tab can reach: not a row sliding out, nor anything + // hidden or not rendered, or the wrap below never sees the real last + const focusables = [ + ...root.querySelectorAll("a[href], button"), + ].filter( + (e) => + !e.closest('[hidden], [aria-hidden="true"]') && + (e.checkVisibility?.() ?? true) + ); + if (!focusables.length) return; + const first = focusables[0]; + const last = focusables[focusables.length - 1]; + if (e.shiftKey && document.activeElement === first) { + e.preventDefault(); + last.focus(); + } else if (!e.shiftKey && document.activeElement === last) { + e.preventDefault(); + first.focus(); + } else if (!root.contains(document.activeElement)) { + e.preventDefault(); + first.focus(); + } + }; + document.addEventListener("keydown", onKey, true); + const prevOverflow = document.body.style.overflow; + document.body.style.overflow = "hidden"; + return () => { + document.removeEventListener("keydown", onKey, true); + document.body.style.overflow = prevOverflow; + before?.focus?.({ preventScroll: true }); + }; + }, []); + + return ( +
    +
    +
    + {free && } +
    + {/* the action with its real number, ticking down live as you merge or close */} -

    - {free - ? 'All finished' - : found.toGo - ? `Finish ${prCount(found.toGo)}` - : 'Nothing to finish'} -

    - {free && ( -

    - None of your open PRs are past your limits. -

    - )} - {/* why, in numbers you can act on: each one jumps to its PRs */} - {!free && stats.length > 0 && ( -
      - {stats.map(stat => ( -
    • - -
      • - ))} -
    - )} -
    -
    - - {!hold && ( - - )} -
    -
    - {!free && ( - <> - {/* the board's own sticky group headers, pinned to the top +

    + {free + ? "All finished" + : found.toGo + ? `Finish ${prCount(found.toGo)}` + : "Nothing to finish"} +

    + {free && ( +

    + None of your open PRs are past your limits. +

    + )} + {/* why, in numbers you can act on: each one jumps to its PRs */} + {!free && stats.length > 0 && ( +

    + {stats.map((stat, i) => ( + + {i > 0 && · } + goTo(stat)}>{stat.text} + + ))} +

    + )} +
    +
    + + {!hold && ( + + )} +
    +
    + {!free && ( + <> + {/* the board's own sticky group headers, pinned to the top of this list on the card's color instead of under the app header on the canvas */} -
    - -
    -

    Only you see this.

    - {hold && } - - )} -
    -
    - ); +
    + +
    +

    Only you see this.

    + {hold && } + + )} +
    + + ); } /** The hold button: hold it down for HOLD_MS, by pointer, finger, Enter or * Space. The ring fills as you hold; letting go early springs it back, and a * quick click says to hold it. At the end it closes. */ function HoldToClose({ onDone }: { onDone: () => void }) { - const [holding, setHolding] = useState(false); - const [opened, setOpened] = useState(false); - // a click too short to be a hold: the label teaches the gesture - const [tapped, setTapped] = useState(false); - const timer = useRef(undefined); - const tapTimer = useRef(undefined); - const since = useRef(0); - // when a key was last let go: Firefox clicks a button on Space's keyup, - // and that click must not pass for assistive tech's lone click - const keyUpAt = useRef(0); - const start = () => { - if (timer.current !== undefined || opened) return; - since.current = Date.now(); - setHolding(true); - setTapped(false); - timer.current = window.setTimeout(() => { - setOpened(true); - timer.current = window.setTimeout(onDone, OPEN_MS); - }, HOLD_MS); - }; - const stop = () => { - if (opened || timer.current === undefined) return; + const [holding, setHolding] = useState(false); + const [opened, setOpened] = useState(false); + // a click too short to be a hold: the label teaches the gesture + const [tapped, setTapped] = useState(false); + const timer = useRef(undefined); + const tapTimer = useRef(undefined); + const since = useRef(0); + // when a key was last let go: Firefox clicks a button on Space's keyup, + // and that click must not pass for assistive tech's lone click + const keyUpAt = useRef(0); + const start = () => { + if (timer.current !== undefined || opened) return; + since.current = Date.now(); + setHolding(true); + setTapped(false); + timer.current = window.setTimeout(() => { + setOpened(true); + timer.current = window.setTimeout(onDone, OPEN_MS); + }, HOLD_MS); + }; + const stop = () => { + if (opened || timer.current === undefined) return; + clearTimeout(timer.current); + timer.current = undefined; + setHolding(false); + if (Date.now() - since.current < 500) { + setTapped(true); + clearTimeout(tapTimer.current); + tapTimer.current = window.setTimeout(() => setTapped(false), 2000); + } + }; + useEffect( + () => () => { clearTimeout(timer.current); - timer.current = undefined; - setHolding(false); - if (Date.now() - since.current < 500) { - setTapped(true); - clearTimeout(tapTimer.current); - tapTimer.current = window.setTimeout(() => setTapped(false), 2000); - } - }; - useEffect( - () => () => { - clearTimeout(timer.current); - clearTimeout(tapTimer.current); - }, - [] - ); - const isPress = (key: string) => key === 'Enter' || key === ' '; - // the ring fills while held, and snaps back when let go - const sweep = holding ? `${HOLD_MS}ms linear` : '150ms ease-out'; - return ( - - ); + clearTimeout(tapTimer.current); + }, + [] + ); + const isPress = (key: string) => key === "Enter" || key === " "; + // the ring fills while held, and snaps back when let go + const sweep = holding ? `${HOLD_MS}ms linear` : "150ms ease-out"; + return ( + + ); } const pullId = (p: DerivedPull) => `${p.data.repo}#${p.data.number}`; /** The PRs that just left `pulls` (merged or closed), kept for LEAVE_MS so * their rows can slide out instead of vanishing. */ -function useLeaving(pulls: DerivedPull[], openIds: ReadonlySet): DerivedPull[] { - const last = useRef(new Map()); - const [leaving, setLeaving] = useState([]); - const timers = useRef([]); - useEffect(() => { - const now = new Map(pulls.map(p => [pullId(p), p])); - // gone from the list and closed on the board; one that only stopped - // counting (the rule switched, or it went draft) just leaves - const gone = [...last.current] - .filter(([id]) => !now.has(id) && !openIds.has(id)) - .map(([, p]) => p); - last.current = now; - if (!gone.length) return; - setLeaving(l => [...l, ...gone]); - timers.current.push( - window.setTimeout(() => setLeaving(l => l.filter(p => !gone.includes(p))), LEAVE_MS) - ); - // keyed on pulls alone: openIds changes with every board update, and - // it's only read to tell a close from a rule switch - }, [pulls]); - useEffect(() => () => timers.current.forEach(clearTimeout), []); - // one that came back (reopened) is just a row again - return leaving.filter(p => !pulls.some(q => pullId(q) === pullId(p))); +function useLeaving( + pulls: DerivedPull[], + openIds: ReadonlySet +): DerivedPull[] { + const last = useRef(new Map()); + const [leaving, setLeaving] = useState([]); + const timers = useRef([]); + useEffect(() => { + const now = new Map(pulls.map((p) => [pullId(p), p])); + // gone from the list and closed on the board; one that only stopped + // counting (the rule switched, or it went draft) just leaves + const gone = [...last.current] + .filter(([id]) => !now.has(id) && !openIds.has(id)) + .map(([, p]) => p); + last.current = now; + if (!gone.length) return; + setLeaving((l) => [...l, ...gone]); + timers.current.push( + window.setTimeout( + () => setLeaving((l) => l.filter((p) => !gone.includes(p))), + LEAVE_MS + ) + ); + // keyed on pulls alone: openIds changes with every board update, and + // it's only read to tell a close from a rule switch + }, [pulls]); + useEffect(() => () => timers.current.forEach(clearTimeout), []); + // one that came back (reopened) is just a row again + return leaving.filter((p) => !pulls.some((q) => pullId(q) === pullId(p))); } /** Slack-ready text chasing the reviews you asked for: each title, its * link, and who was asked. Only PRs with a request: a self-reviewed PR waits * on nobody, so there's no one to nudge about it. */ function nudgeText(rows: { pull: DerivedPull }[]): string { - return [ - 'A few of my PRs are waiting on a review I asked for:', - ...rows.map( - ({ pull: p }) => - `• ${p.data.title} ${githubUrl(p.data.repo, p.data.number)} (${askedOf(p).join(', ')})` - ), - ].join('\n'); + return [ + "A few of my PRs are waiting on a review I asked for:", + ...rows.map( + ({ pull: p }) => + `• ${p.data.title} ${githubUrl(p.data.repo, p.data.number)} (${askedOf( + p + ).join(", ")})` + ), + ].join("\n"); } /** Copies nudgeText, and says whether it worked. */ function CopyNudge({ rows }: { rows: { pull: DerivedPull }[] }) { - const [copied, setCopied] = useState<'copied' | 'failed' | null>(null); - const done = (result: 'copied' | 'failed') => { - setCopied(result); - window.setTimeout(() => setCopied(null), 3000); - }; - return ( - { - if (!navigator.clipboard) return done('failed'); - navigator.clipboard.writeText(nudgeText(rows)).then( - () => done('copied'), - () => done('failed') - ); - }} - > - {copied === 'copied' ? 'Copied' : copied === 'failed' ? 'Couldn’t copy' : 'Copy a nudge'} - - ); + const [copied, setCopied] = useState<"copied" | "failed" | null>(null); + const done = (result: "copied" | "failed") => { + setCopied(result); + window.setTimeout(() => setCopied(null), 3000); + }; + return ( + { + if (!navigator.clipboard) return done("failed"); + navigator.clipboard.writeText(nudgeText(rows)).then( + () => done("copied"), + () => done("failed") + ); + }} + > + {copied === "copied" + ? "Copied" + : copied === "failed" + ? "Couldn’t copy" + : "Copy a nudge"} + + ); } /** Your PRs, the way My work groups them, with the ready ones on top: the @@ -699,255 +721,238 @@ function CopyNudge({ rows }: { rows: { pull: DerivedPull }[] }) { * word on what to do, oldest first. Someone else's PRs (the preview's * fallback) list flat, since their groups would be a reviewer's. */ function FinishList({ - pulls, - me, - maxDays, - openIds, - closed, - onToggle, + pulls, + me, + maxDays, + openIds, }: { - pulls: DerivedPull[]; - me: string; - maxDays: number; - openIds: ReadonlySet; - /** the groups folded shut */ - closed: ReadonlySet; - onToggle: (k: GroupKey) => void; + pulls: DerivedPull[]; + me: string; + maxDays: number; + openIds: ReadonlySet; }) { - const leaving = useLeaving(pulls, openIds); - const gone = new Set(leaving.map(pullId)); - const all = [...pulls, ...leaving]; - const mine = all.every(p => p.data.user.login === me); - const groups = mine - ? finishGroups(all, me) - : { ready: [], move: [], waiting: all.map(pull => ({ pull })) }; - const live = (rows: { pull: DerivedPull }[]) => rows.filter(r => !gone.has(pullId(r.pull))); - // the PRs waiting on people you asked, wherever they sit (a request gone - // quiet past 4 hours is Your move); the copy button rides on the first - // section holding one - const asked = (rows: { pull: DerivedPull }[]) => live(rows).filter(r => askedOf(r.pull).length); - const nudgeRows = asked([...groups.move, ...groups.waiting]); - const sections = [ - { - key: 'ready' as const, - title: STAGE_WORDS.ready, - sub: 'Merge these first', - rows: groups.ready, - ready: true, - }, - { - key: 'move' as const, - title: 'Your move', - sub: 'Waiting on you', - rows: groups.move, - }, - { - key: 'waiting' as const, - title: 'Waiting on others', - sub: 'Nudge whoever you asked, or close what you don’t need', - rows: groups.waiting, - }, - ].filter(g => g.rows.length); - const nudgeOn = sections.find(g => !g.ready && asked(g.rows).length)?.title; - return ( -
    - {sections.map(g => { - const open = !mine || !closed.has(g.key); - return ( -
    - {mine && ( - onToggle(g.key)} - className="hit -ml-0.5 inline-flex items-center gap-1 rounded border-0 bg-transparent p-0 text-left font-semibold text-inherit" - > - - {g.title} - - } - sub={g.sub} - count={live(g.rows).length} - headerExtra={ - g.title === nudgeOn ? : undefined - } - /> - )} - -
    - ); - })} -
    - ); + const leaving = useLeaving(pulls, openIds); + const gone = new Set(leaving.map(pullId)); + const all = [...pulls, ...leaving]; + const mine = all.every((p) => p.data.user.login === me); + const groups = mine + ? finishGroups(all, me) + : { ready: [], move: [], waiting: all.map((pull) => ({ pull })) }; + const live = (rows: { pull: DerivedPull }[]) => + rows.filter((r) => !gone.has(pullId(r.pull))); + // the PRs waiting on people you asked, wherever they sit (a request gone + // quiet past 4 hours is Your move); the copy button rides on the first + // section holding one + const asked = (rows: { pull: DerivedPull }[]) => + live(rows).filter((r) => askedOf(r.pull).length); + const nudgeRows = asked([...groups.move, ...groups.waiting]); + const sections = [ + { + key: "ready" as const, + title: STAGE_WORDS.ready, + sub: "Merge these first", + rows: groups.ready, + ready: true, + }, + { + key: "move" as const, + title: "Your move", + sub: "Do the next step on each", + rows: groups.move, + }, + { + key: "waiting" as const, + title: "Waiting on others", + sub: "Nudge whoever you asked, or close what you don’t need", + rows: groups.waiting, + }, + ].filter((g) => g.rows.length); + const nudgeOn = sections.find((g) => !g.ready && asked(g.rows).length)?.title; + return ( +
    + {sections.map((g) => ( +
    + {mine && ( + : undefined + } + /> + )} +
      + {g.rows.map(({ pull }) => ( + + ))} +
    +
    + ))} +
    + ); } /** One PR: the title to GitHub and its age, then the move and what it's * waiting on. */ function FinishRow({ - pull: p, - me, - maxDays, - ready, - mine, - leaving, + pull: p, + me, + maxDays, + ready, + mine, + leaving, }: { - pull: DerivedPull; - me: string; - maxDays: number; - ready: boolean; - mine: boolean; - /** merged or closed: it slides out */ - leaving?: boolean; + pull: DerivedPull; + me: string; + maxDays: number; + ready: boolean; + mine: boolean; + /** merged or closed: it slides out */ + leaving?: boolean; }) { - const word = mine && !ready ? rowWord(p, me) : null; - const context = mine ? rowNote(p, me).context : null; - // a do row names the move, then the detail; a wait row's detail already - // says who it waits on ("waiting on bob to re-stamp"), so the word only - // stands in when there's no detail - const move = word?.kind === 'do' ? word.word : null; - const detail = context ?? (word?.kind === 'wait' ? word.word : null); - const hint = mine && !ready ? askForInputHint(p) : null; - const old = p.ageDays > maxDays; - return ( -
  • - - - {p.data.title} ↗ - - - {p.ageDays ? `${p.ageDays} days` : 'today'} - - - - - {move && ( - - {move} - - )} - {!ready && ( - <> - - {/* the glyph is aria-hidden; say what it shows */} - {p.ci === 'failing' && CI failing} - {p.ci === 'pending' && CI running} - - )} - {detail && {detail}} - - {hint && {hint}} -
    • - ); + const word = mine && !ready ? rowWord(p, me) : null; + const context = mine ? rowNote(p, me).context : null; + // a do row names the move, then the detail; a wait row's detail already + // says who it waits on ("waiting on bob to re-stamp"), so the word only + // stands in when there's no detail + const move = word?.kind === "do" ? word.word : null; + const detail = context ?? (word?.kind === "wait" ? word.word : null); + const hint = mine && !ready ? askForInputHint(p) : null; + const old = p.ageDays > maxDays; + return ( +
  • + + + {p.data.title} ↗ + + {/* quiet even past the limit: the stat above already says how many + are, and amber on every row would stop meaning "look here" */} + + {p.ageDays ? `${p.ageDays} days` : "today"} + + + + + {move && ( + + {move} + + )} + {!ready && ( + <> + + {/* the glyph is aria-hidden; say what it shows */} + {p.ci === "failing" && CI failing} + {p.ci === "pending" && CI running} + + )} + {detail && {detail}} + + {hint && {hint}} +
    • + ); } /** A robot cheering at a checkered finish flag, drawn in the board's own * line weights and color tokens so it themes with everything else. */ function FinishMark() { - return ( - - {/* the finish line on the ground */} - - {/* the flag: a pole and a checkered cloth */} - - - - {/* an upright antenna with a lit tip */} - - - {/* head, and a happy face on its screen */} - - - - {/* body, one arm up at the flag, one down */} - - - - - ); + return ( + + {/* the finish line on the ground */} + + {/* the flag: a pole and a checkered cloth */} + + + + {/* an upright antenna with a lit tip */} + + + {/* head, and a happy face on its screen */} + + + + {/* body, one arm up at the flag, one down */} + + + + + ); } diff --git a/frontend-v2/src/components/Lane.tsx b/frontend-v2/src/components/Lane.tsx index 3aedf2d5..eed26c64 100644 --- a/frontend-v2/src/components/Lane.tsx +++ b/frontend-v2/src/components/Lane.tsx @@ -112,9 +112,7 @@ export function GroupHeader({ headerExtra, level = 2, }: { - /** a string, or a node when the title is itself a control (Finish - * line's groups fold on their title) */ - title: ReactNode; + title: string; /** plain string for most lanes; a ReactNode when the sub-line itself is * the door to more detail (Review's queue wraps its ranking one-liner in * a popover — existing text becomes interactive, zero new chrome) */ diff --git a/frontend-v2/src/components/Legend.tsx b/frontend-v2/src/components/Legend.tsx index c1942a0f..924507b9 100644 --- a/frontend-v2/src/components/Legend.tsx +++ b/frontend-v2/src/components/Legend.tsx @@ -1,17 +1,17 @@ -import type { ReactNode } from 'react'; -import { CircleHelp, Flag, Heart, Star } from 'lucide-react'; -import { HeaderIconButton } from './bits'; -import { Icon } from './Icon'; -import { Popover } from './Popover'; -import { eyebrowText } from './WordGroups'; +import type { ReactNode } from "react"; +import { CircleHelp, Flag, Heart, Star } from "lucide-react"; +import { HeaderIconButton } from "./bits"; +import { Icon } from "./Icon"; +import { Popover } from "./Popover"; +import { eyebrowText } from "./WordGroups"; /** a keycap, sized to the legend's small type */ function Kbd({ children }: { children: ReactNode }) { - return ( - - {children} - - ); + return ( + + {children} + + ); } /** @@ -20,24 +20,24 @@ function Kbd({ children }: { children: ReactNode }) { * line inside the column instead of overflowing into the definition. */ function Item({ term, def }: { term: ReactNode; def: ReactNode }) { - return ( -
    - - {term} - - {def} -
    - ); + return ( +
    + + {term} + + {def} +
    + ); } /** a labeled band of the key; every band after the first draws a hairline */ function Group({ title, children }: { title: string; children: ReactNode }) { - return ( -
    -
    {title}
    - {children} -
    - ); + return ( +
    +
    {title}
    + {children} +
    + ); } /** @@ -50,187 +50,211 @@ function Group({ title, children }: { title: string; children: ReactNode }) { * tokens, and the keyboard. One screen, no folds, no scrolling essays. */ export function Legend() { - return ( - } - > - - Reading the board - -

    - Everything explains itself where it sits: hover any mark, flag, header, or count. These - are the conventions that don’t announce themselves. -

    + return ( + ( + + )} + > + + Reading the board + +

    + Everything explains itself where it sits: hover any mark, flag, header, + or count. These are the conventions that don’t announce themselves. +

    - - stamp} - def="a CR or QA sign-off: a GitHub approval, or a “CR 👍” / “QA 👍” comment. A push undoes it until it’s re-confirmed" - /> - - - - - - } - def="one circle per required stamp, in order: stands, stale review, still needed. A dotted underline marks yours; click them for who signed. While a PR is a draft, blocked, or red, the re-ask waits: the author moves first" - /> - - CI - - - - } - def="CI is one circle, the machine’s mark: the red ring with a center dot, a check failed; the ring sweeping shut is a run in progress, the filled share done. On row hover a green ring closes the loop: passed. No news is good news" - /> - + + stamp} + def="a CR or QA sign-off: a GitHub approval, or a “CR 👍” / “QA 👍” comment. A push undoes it until it’s re-confirmed" + /> + + + + + + } + def="one circle per required stamp, in order: stands, stale review, still needed. A dotted underline marks yours; click them for who signed. While a PR is a draft, blocked, or red, the re-ask waits: the author moves first" + /> + + CI + + + + } + def="CI is one circle, the machine’s mark: the red ring with a center dot, a check failed; the ring sweeping shut is a run in progress, the filled share done. On row hover a green ring closes the loop: passed. No news is good news" + /> + - - - - - - } - def="a circle avatar is a person; a square tile is a bot or app" - /> - - - - - } - def="a corner heart marks someone on your team; the star seated on your own avatar’s rim is you" - /> - + + + + + + } + def="a circle avatar is a person; a square tile is a bot or app" + /> + + + + + } + def="a corner heart marks someone on your team; the star seated on your own avatar’s rim is you" + /> + - - - Re-stamp · 3 - - } - def="brand is your move: a brand header names an action you owe, a gray one says why a card waits. Hover the word for its meaning" - /> - - conflicts - stacked - - } - def="amber flag: act on it. Gray flag: a plain fact" - /> - + + + Re-stamp · 3 + + } + def="brand is your move: a brand header names an action you owe, a gray one says why a card waits. Hover the word for its meaning" + /> + + conflicts + stacked + + } + def="amber flag: act on it. Gray flag: a plain fact" + /> + - - self-review} - def="a developer reviews and tests their own PR unless they ask for a review. Nobody else owes it anything" - /> - review requested} - def="the author asked people (or a team) for a review on GitHub. It leads their Waiting on you; answer within hours" - /> - I’ll review} - def="hover a row for its verbs. It adds you as a reviewer on the PR, so GitHub and the board both show you’re on it; the row moves to Waiting on you until you submit, drop it, or are removed" - /> - + + self-review} + def="a developer reviews and tests their own PR unless they ask for a review. Nobody else owes it anything" + /> + review requested} + def="the author asked people (or a team) for a review on GitHub. It leads their Waiting on you; answer within hours" + /> + I’ll review + } + def="hover a row for its verbs. It adds you as a reviewer on the PR, so GitHub and the board both show you’re on it; the row moves to Waiting on you until you submit, drop it, or are removed" + /> + - - - - - 2 to finish - - - } - def="shows in the header when your own open PRs pass your limits (by default more than 7 open, or one open over 14 days): how many to merge or close. Click it for the list, ready ones first. Only you see it; nobody else is told" - /> - Settings} - def="under Finish line, or the cog in the list: Pop up opens the list by itself when you go over (off by default), Header count only waits for a click, Off hides it. Esc or ✕ closes the list" - /> - + + + + + 2 to finish + + + } + def="shows in the header when your own open PRs pass your limits (by default more than 7 open, or one open over 14 days): how many to merge or close. Click it for the list, ready ones first. Only you see it; nobody else is told" + /> + Settings} + def="under Finish line, or the cog in the list: Pop up opens the list by itself when you go over (off by default), Header count only waits for a click, Off hides it. Esc or ✕ closes it, unless you’ve chosen to hold to close" + /> + - - weight:xs,s} - def="review-effort classes; a row shows its class as a letter beside CR, detailed in the CR popover" - /> - has:action} - def="cards where the next move is yours" - /> - is:restamp} - def="a push undid your stamp and the PR is reviewable again" - /> - is:asked} - def="someone asked you for a review" - /> - is:blocked} - def="under a dev or deploy block, whoever holds it" - /> - is:draft} - def="still a draft" - /> - is:mine} - def="opened by you" - /> - + + + weight:xs,s + + } + def="review-effort classes; a row shows its class as a letter beside CR, detailed in the CR popover" + /> + has:action + } + def="cards where the next move is yours" + /> + is:restamp + } + def="a push undid your stamp and the PR is reviewable again" + /> + is:asked + } + def="someone asked you for a review" + /> + is:blocked + } + def="under a dev or deploy block, whoever holds it" + /> + is:draft + } + def="still a draft" + /> + is:mine + } + def="opened by you" + /> + - - /} - def="focus the filter; on Projects, the view’s own find box" - /> - - j - k - - } - def="walk down / up the rows" - /> - ↵} - def="open the selected PR or project; on Decide, make the call" - /> - c} def="copy the selected branch name" /> - a} def="on a project’s page, add an issue" /> - u} def="on a project’s page, post an update" /> - -     - ); + + /} + def="focus the filter; on Projects, the view’s own find box" + /> + + j + k + + } + def="walk down / up the rows" + /> + ↵} + def="open the selected PR or project; on Decide, make the call" + /> + c} def="copy the selected branch name" /> + a} def="on a project’s page, add an issue" /> + u} def="on a project’s page, post an update" /> + +     + ); } diff --git a/frontend-v2/src/components/Settings.tsx b/frontend-v2/src/components/Settings.tsx index 0104260c..1365d077 100644 --- a/frontend-v2/src/components/Settings.tsx +++ b/frontend-v2/src/components/Settings.tsx @@ -1,568 +1,603 @@ -import { useEffect, useRef, useState } from 'react'; -import { createPortal } from 'react-dom'; -import { Settings as SettingsIcon, Trash2, X } from 'lucide-react'; -import { clearStoredPrefs } from '../storage'; -import { refreshAll, refreshAllText, usePulldasher } from '../store'; +import { useEffect, useRef, useState } from "react"; +import { createPortal } from "react-dom"; +import { Settings as SettingsIcon, Trash2, X } from "lucide-react"; +import { clearStoredPrefs } from "../storage"; +import { refreshAll, refreshAllText, usePulldasher } from "../store"; import { - addCodeRegion, - removeCodeRegion, - setLaneCapForLens, - type Settings as SettingsShape, - setSettings, - useSettings, -} from '../settings'; -import { LENS_LABELS, type Lens } from '../lens'; -import { HeaderIconButton, QuietButton, Segmented } from './bits'; -import { Icon } from './Icon'; -import { Explainer, Field, Group, NumberField } from './SettingsBits'; -import { useArmedConfirm } from './useArmedConfirm'; -import { commitKeyHandler } from './useCommitOnEnter'; + addCodeRegion, + removeCodeRegion, + setLaneCapForLens, + type Settings as SettingsShape, + setSettings, + useSettings, +} from "../settings"; +import { LENS_LABELS, type Lens } from "../lens"; +import { HeaderIconButton, QuietButton, Segmented } from "./bits"; +import { Icon } from "./Icon"; +import { Explainer, Field, Group, NumberField } from "./SettingsBits"; +import { useArmedConfirm } from "./useArmedConfirm"; +import { commitKeyHandler } from "./useCommitOnEnter"; const LENS_OPTIONS: [string, string][] = Object.entries(LENS_LABELS); // the Projects tab exists only on a server set up for projects -const LENS_OPTIONS_NO_PROJECTS = LENS_OPTIONS.filter(([id]) => id !== 'projects'); +const LENS_OPTIONS_NO_PROJECTS = LENS_OPTIONS.filter( + ([id]) => id !== "projects" +); // Stats has no lanes, so it's left out of the per-lens lane-length list // (unlike LENS_OPTIONS above, which covers every tab including Stats) — the // membership stays its own explicit list, but each label still comes from // the shared map so it can't drift from LENS_OPTIONS' wording. -const LANE_CAP_LENSES: Lens[] = ['review', 'mine', 'team', 'classic', 'ci']; -const LANE_CAP_LENS_OPTIONS: [string, string][] = LANE_CAP_LENSES.map(id => [id, LENS_LABELS[id]]); +const LANE_CAP_LENSES: Lens[] = ["review", "mine", "team", "classic", "ci"]; +const LANE_CAP_LENS_OPTIONS: [string, string][] = LANE_CAP_LENSES.map((id) => [ + id, + LENS_LABELS[id], +]); const LANE_CAP_OPTIONS: [string, string][] = [ - ['default', 'Default'], - ['10', '10'], - ['25', '25'], - ['50', '50'], - ['0', 'No cap'], + ["default", "Default"], + ["10", "10"], + ["25", "25"], + ["50", "50"], + ["0", "No cap"], ]; /** One lens' lane-length override row, inside the "Different length per view" * disclosure. "Default" clears the override so the lens falls back to the * global Lane length above. */ -function LaneCapByLensRow({ lensId, label }: { lensId: string; label: string }) { - const cap = useSettings().laneCapByLens[lensId]; - return ( - - setLaneCapForLens(lensId, v === 'default' ? null : Number(v))} - /> - - ); +function LaneCapByLensRow({ + lensId, + label, +}: { + lensId: string; + label: string; +}) { + const cap = useSettings().laneCapByLens[lensId]; + return ( + + + setLaneCapForLens(lensId, v === "default" ? null : Number(v)) + } + /> + + ); } /** Free-text editor for the code regions that put matching self-reviews into * Review's Could use your input. Arbitrary strings (not a known set), so it's a plain input * plus removable chips, not a candidate picker. */ function CodeRegionsGroup() { - const regions = useSettings().codeRegions; - const [draft, setDraft] = useState(''); - const add = () => { - addCodeRegion(draft); - setDraft(''); - }; - return ( - - - Areas you own or care about. Someone’s self-reviewed PR whose title, description, - labels, branch, or repo contains one shows under Could use your input on Review. - -
    - setDraft(e.target.value)} - onKeyDown={commitKeyHandler({ onCommit: add, onCancel: () => setDraft('') })} - placeholder="e.g. Growthbook, Shopify, Diagrams" - aria-label="add a code region" - className="min-w-0 flex-1 rounded-md border border-line bg-surface px-2 py-1 text-sm outline-none focus-visible:border-brand" - /> - - Add - -
    - {regions.length > 0 && ( -
      - {regions.map(r => ( -
    • - - {r} - - -
      • - ))} -
    - )} -     - ); + const regions = useSettings().codeRegions; + const [draft, setDraft] = useState(""); + const add = () => { + addCodeRegion(draft); + setDraft(""); + }; + return ( + + + Areas you own or care about. Someone’s self-reviewed PR whose title, + description, labels, branch, or repo contains one shows under Could use + your input on Review. + +
    + setDraft(e.target.value)} + onKeyDown={commitKeyHandler({ + onCommit: add, + onCancel: () => setDraft(""), + })} + placeholder="e.g. Growthbook, Shopify, Diagrams" + aria-label="add a code region" + className="min-w-0 flex-1 rounded-md border border-line bg-surface px-2 py-1 text-sm outline-none focus-visible:border-brand" + /> + + Add + +
    + {regions.length > 0 && ( +
      + {regions.map((r) => ( +
    • + + {r} + + +
      • + ))} +
    + )} +     + ); } export function Settings({ - onGoToTeam, + onGoToTeam, }: { - /** switch the board to the Team lens and close this panel — the "Edited - * on the board" pointer line's action. The team picker and repo manager - * embeds are gone (Team view and the header's Repos filter own those - * now), so this is the only cross-surface wiring this panel needs. */ - onGoToTeam: () => void; + /** switch the board to the Team lens and close this panel — the "Edited + * on the board" pointer line's action. The team picker and repo manager + * embeds are gone (Team view and the header's Repos filter own those + * now), so this is the only cross-surface wiring this panel needs. */ + onGoToTeam: () => void; }) { - const [open, setOpen] = useState(false); - const s = useSettings(); - const { refreshProgress, projectLabelPrefix } = usePulldasher(); - const panelRef = useRef(null); - const triggerRef = useRef(null); - const { armed: armReset, run: runReset } = useArmedConfirm(); + const [open, setOpen] = useState(false); + const s = useSettings(); + const { refreshProgress, projectLabelPrefix } = usePulldasher(); + const panelRef = useRef(null); + const triggerRef = useRef(null); + const { armed: armReset, run: runReset } = useArmedConfirm(); - // which section to scroll to and focus once open: a Group's id - const [section, setSection] = useState(null); + // which section to scroll to and focus once open: a Group's id + const [section, setSection] = useState(null); - // open on demand — the code-regions tip's "Set them up" action dispatches - // this so a reader can jump straight here from the board; a CustomEvent - // whose detail names a Group's id opens at that group (Finish line's cog) - useEffect(() => { - const openIt = (e: Event) => { - setSection( - typeof (e as CustomEvent).detail === 'string' ? (e as CustomEvent).detail : null - ); - setOpen(true); - }; - window.addEventListener('pd2:open-settings', openIt); - return () => window.removeEventListener('pd2:open-settings', openIt); - }, []); + // open on demand — the code-regions tip's "Set them up" action dispatches + // this so a reader can jump straight here from the board; a CustomEvent + // whose detail names a Group's id opens at that group (Finish line's cog) + useEffect(() => { + const openIt = (e: Event) => { + setSection( + typeof (e as CustomEvent).detail === "string" + ? (e as CustomEvent).detail + : null + ); + setOpen(true); + }; + window.addEventListener("pd2:open-settings", openIt); + return () => window.removeEventListener("pd2:open-settings", openIt); + }, []); - useEffect(() => { - if (!open) return; - const onKey = (e: KeyboardEvent) => { - // a dialog above this one (Finish line) handled it - if (e.defaultPrevented) return; - if (e.key === 'Escape') setOpen(false); - // aria-modal promises a focus trap; without this, Tab walks out of - // the dialog into the live board behind the scrim - if (e.key === 'Tab' && panelRef.current) { - const focusables = panelRef.current.querySelectorAll( - 'button, input, select, textarea, a[href], [tabindex]:not([tabindex="-1"])' - ); - if (!focusables.length) return; - const first = focusables[0]; - const last = focusables[focusables.length - 1]; - const active = document.activeElement; - if (e.shiftKey && (active === first || active === panelRef.current)) { - e.preventDefault(); - last.focus(); - } else if (!e.shiftKey && active === last) { - e.preventDefault(); - first.focus(); - } - } - }; - document.addEventListener('keydown', onKey); - // a modal locks the page behind it: the scrim already blocks clicks, - // this blocks scroll - const prevOverflow = document.body.style.overflow; - document.body.style.overflow = 'hidden'; - // focus the panel so Escape and tabbing land inside it, or the first - // control of the section it was opened at - panelRef.current?.focus(); - if (section) { - const target = panelRef.current?.querySelector(`#${section}`); - target?.scrollIntoView({ block: 'start' }); - target?.querySelector('button, input')?.focus({ preventScroll: true }); - setSection(null); + useEffect(() => { + if (!open) return; + const onKey = (e: KeyboardEvent) => { + // a dialog above this one (Finish line) handled it + if (e.defaultPrevented) return; + if (e.key === "Escape") setOpen(false); + // aria-modal promises a focus trap; without this, Tab walks out of + // the dialog into the live board behind the scrim + if (e.key === "Tab" && panelRef.current) { + const focusables = panelRef.current.querySelectorAll( + 'button, input, select, textarea, a[href], [tabindex]:not([tabindex="-1"])' + ); + if (!focusables.length) return; + const first = focusables[0]; + const last = focusables[focusables.length - 1]; + const active = document.activeElement; + if (e.shiftKey && (active === first || active === panelRef.current)) { + e.preventDefault(); + last.focus(); + } else if (!e.shiftKey && active === last) { + e.preventDefault(); + first.focus(); + } } - return () => { - document.removeEventListener('keydown', onKey); - document.body.style.overflow = prevOverflow; - }; - // section is read once, when it opens, so it isn't a dependency - }, [open]); + }; + document.addEventListener("keydown", onKey); + // a modal locks the page behind it: the scrim already blocks clicks, + // this blocks scroll + const prevOverflow = document.body.style.overflow; + document.body.style.overflow = "hidden"; + // focus the panel so Escape and tabbing land inside it, or the first + // control of the section it was opened at + panelRef.current?.focus(); + if (section) { + const target = panelRef.current?.querySelector( + `#${section}` + ); + target?.scrollIntoView({ block: "start" }); + target + ?.querySelector("button, input") + ?.focus({ preventScroll: true }); + setSection(null); + } + return () => { + document.removeEventListener("keydown", onKey); + document.body.style.overflow = prevOverflow; + }; + // section is read once, when it opens, so it isn't a dependency + }, [open]); - // return focus to the cog when the panel closes — but only after a real - // open->close transition, not on initial mount (open starts false, and - // stealing focus to the gear on page load with no user interaction) - const didOpenRef = useRef(false); - useEffect(() => { - if (open) didOpenRef.current = true; - else if (didOpenRef.current) triggerRef.current?.focus?.(); - }, [open]); + // return focus to the cog when the panel closes — but only after a real + // open->close transition, not on initial mount (open starts false, and + // stealing focus to the gear on page load with no user interaction) + const didOpenRef = useRef(false); + useEffect(() => { + if (open) didOpenRef.current = true; + else if (didOpenRef.current) triggerRef.current?.focus?.(); + }, [open]); - const set = (patch: Partial) => setSettings(patch); + const set = (patch: Partial) => setSettings(patch); - return ( - <> - setOpen(o => !o)} - /> - {open && - createPortal( -
    -
    setOpen(false)} - aria-hidden - /> -
    - {/* z-10: children further down the panel (avatars, the + return ( + <> + setOpen((o) => !o)} + /> + {open && + createPortal( +
    +
    setOpen(false)} + aria-hidden + /> +
    + {/* z-10: children further down the panel (avatars, the code-regions Add button — anything position:relative via .hit/.pressable) paint above a sticky header with no z-index of its own once they scroll under it. The overlay above is already z-[100], so z-10 in here only has to beat panel content. */} -
    - Settings - -
    +
    + Settings + +
    - - - set({ theme })} - /> - - - set({ density })} - /> - - - set({ defaultLens })} - /> - - + + + set({ theme })} + /> + + + set({ density })} + /> + + + set({ defaultLens })} + /> + + - - - set({ laneCap: Number(v) })} - /> - - - - Classic can show everything while Review stays short. - -
    - {LANE_CAP_LENS_OPTIONS.map(([id, label]) => ( - - ))} -
    -     - - set({ repoQueueCap: Number(v) })} - /> - - - set({ ageWarnDays })} - /> - - - set({ ageDisplay })} - /> - - - set({ claimWarnMins: Number(v) })} - /> - - - set({ openPrsNewTab: v === 'new' })} - /> - - - set({ hoverDelayMs: Number(v) })} - /> - -     + + + set({ laneCap: Number(v) })} + /> + + + + Classic can show everything while Review stays short. + +
    + {LANE_CAP_LENS_OPTIONS.map(([id, label]) => ( + + ))} +
    +     + + set({ repoQueueCap: Number(v) })} + /> + + + set({ ageWarnDays })} + /> + + + set({ ageDisplay })} + /> + + + set({ claimWarnMins: Number(v) })} + /> + + + set({ openPrsNewTab: v === "new" })} + /> + + + set({ hoverDelayMs: Number(v) })} + /> + +     - - - A count in the header of your own open PRs past your limits, and a list - of them to merge or close. Only you see it. - - - set({ finishLineMode })} - /> - - - set({ finishLineMaxOpen })} - /> - - - set({ finishLineMaxDays })} - /> - - - set({ finishLineCountDrafts: v === 'on' })} - /> - - - set({ finishLineHoldToClose: v === 'on' })} - /> - - - { - // close Settings first so the list isn't stacked on it - setOpen(false); - window.dispatchEvent(new Event('pd2:preview-finish-line')); - }} - > - Preview - - - + + + A count in the header of your own open PRs past your limits, + and a list of them to merge or close. Only you see it. + + + set({ finishLineMode })} + /> + + + set({ finishLineMaxOpen })} + /> + + + set({ finishLineMaxDays })} + /> + + + set({ finishLineCountDrafts: v === "on" })} + /> + + + set({ finishLineHoldToClose: v === "on" })} + /> + + + { + // close Settings first so the list isn't stacked on it + setOpen(false); + window.dispatchEvent( + new Event("pd2:preview-finish-line") + ); + }} + > + Preview + + + - + - -
    - - Teams: build your rosters on the Team view. - - { - onGoToTeam(); - setOpen(false); - }} - > - Open Team view - -
    - - Repos: hide and reveal from the Repos filter in the header. - -     + +
    + + Teams: build your rosters on the Team view. + + { + onGoToTeam(); + setOpen(false); + }} + > + Open Team view + +
    + + Repos: hide and reveal from the Repos filter in the header. + +     -
    - - - Check the board against GitHub and re-fetch the PRs that differ, - including ones it missed or still shows open. The board updates as - each one comes back. - -
    - refreshAll()} - > - Refresh all - - {/* role=status stays mounted so the announcement fires +
    + + + Check the board against GitHub and re-fetch the PRs that + differ, including ones it missed or still shows open. The + board updates as each one comes back. + +
    + refreshAll()} + > + Refresh all + + {/* role=status stays mounted so the announcement fires when the text lands — a screen reader hears the confirmation, not just sighted users. It takes the room the button leaves and wraps in it, so a long line never squeezes the button onto two lines */} - - {refreshProgress ? refreshAllText(refreshProgress) : ''} - -
    + + {refreshProgress ? refreshAllText(refreshProgress) : ""} + +
    - - Reset every preference on this browser (theme, filters, hidden repos - and people, last-seen marker) back to defaults. This can’t be undone. - -
    - -
    -     -
    + + Reset every preference on this browser (theme, filters, + hidden repos and people, last-seen marker) back to defaults. + This can’t be undone. + +
    +
    -
    , - document.body - )} - - ); + +
    + + , + document.body + )} + + ); } diff --git a/frontend-v2/src/model/finishLine.test.ts b/frontend-v2/src/model/finishLine.test.ts index 43f4a901..e4008aca 100644 --- a/frontend-v2/src/model/finishLine.test.ts +++ b/frontend-v2/src/model/finishLine.test.ts @@ -179,11 +179,11 @@ describe('finishStats', () => { it('shows each number only when it is above zero', () => { expect(texts({ count: 9, over: ['a', 'b'], ready: 3 })).toEqual([ - '9 open PRs · aim for 7', + '9 open PRs, aim for 7', '2 PRs older than 14 days', '3 PRs ready to merge', ]); - expect(texts({ count: 9, over: [], ready: 0 })).toEqual(['9 open PRs · aim for 7']); + expect(texts({ count: 9, over: [], ready: 0 })).toEqual(['9 open PRs, aim for 7']); expect(texts({ count: 3, over: ['a'], ready: 1 })).toEqual([ '1 PR older than 14 days', '1 PR ready to merge', diff --git a/frontend-v2/src/model/finishLine.ts b/frontend-v2/src/model/finishLine.ts index bb79c05a..0d9fde96 100644 --- a/frontend-v2/src/model/finishLine.ts +++ b/frontend-v2/src/model/finishLine.ts @@ -1,7 +1,7 @@ -import type { DerivedPull } from '../../../shared/model/status'; -import { isBotLogin } from '../../../shared/model/visibility'; -import type { PullData } from '../../../shared/types'; -import { rowWord } from './actions'; +import type { DerivedPull } from "../../../shared/model/status"; +import { isBotLogin } from "../../../shared/model/visibility"; +import type { PullData } from "../../../shared/types"; +import { rowWord } from "./actions"; /** Over the finish line's limit with more than this many open PRs of your own. */ export const FINISH_MAX_OPEN = 7; @@ -9,48 +9,51 @@ export const FINISH_MAX_OPEN = 7; export const FINISH_MAX_DAYS = 14; /** localStorage key holding the FinishRecord of the last time it popped up. */ -export const FINISH_LINE_KEY = 'pd2.finishLine'; +export const FINISH_LINE_KEY = "pd2.finishLine"; /** After it pops up, it stays away at least this long. */ export const FINISH_COOLDOWN_MS = 4 * 60 * 60 * 1000; /** What it showed you last time, so it only comes back when that got worse. */ export interface FinishRecord { - /** ms when it showed */ - at: number; - /** how many counted open PRs you had then */ - count: number; - /** repo#number of each PR past the age limit then */ - over: string[]; + /** ms when it showed */ + at: number; + /** how many counted open PRs you had then */ + count: number; + /** repo#number of each PR past the age limit then */ + over: string[]; } /** The fields the rule reads off a derived pull. */ export interface FinishPull { - data: Pick; - ageDays: number; - /** who holds a deploy block on it; a held PR is done, so it doesn't count */ - deployBlockedBy: string[]; - status: DerivedPull['status']; - /** parked (Cryogenic Storage): kept open on purpose, never "ready" */ - cryo: boolean; + data: Pick< + PullData, + "repo" | "number" | "title" | "draft" | "state" | "user" + >; + ageDays: number; + /** who holds a deploy block on it; a held PR is done, so it doesn't count */ + deployBlockedBy: string[]; + status: DerivedPull["status"]; + /** parked (Cryogenic Storage): kept open on purpose, never "ready" */ + cryo: boolean; } /** Can merge now, the quickest way to the finish line. A parked PR can be * signed off and green, but My work files it under waiting, so this does too. */ -export const isFinishReady = (p: FinishPull) => p.status === 'ready' && !p.cryo; +export const isFinishReady = (p: FinishPull) => p.status === "ready" && !p.cryo; export interface FinishLineCase

    { - /** the PRs that put you there, oldest first */ - pulls: P[]; - /** how many of `pulls` can merge now */ - ready: number; - /** how many open PRs counted */ - count: number; - /** repo#number of each counted PR past the age limit */ - over: string[]; - /** how many PRs you still have to merge or close to finish: enough to - * get under the count limit, and every one past the age limit (closing an - * old one counts toward both) */ - toGo: number; + /** the PRs that put you there, oldest first */ + pulls: P[]; + /** how many of `pulls` can merge now */ + ready: number; + /** how many open PRs counted */ + count: number; + /** repo#number of each counted PR past the age limit */ + over: string[]; + /** how many PRs you still have to merge or close to finish: enough to + * get under the count limit, and every one past the age limit (closing an + * old one counts toward both) */ + toGo: number; } const idOf = (p: FinishPull) => `${p.data.repo}#${p.data.number}`; @@ -63,35 +66,35 @@ const oldestFirst =

    (a: P, b: P) => b.ageDays - a.ageDays; * under both. */ export function finishLineCase

    ( - pulls: P[], - me: string, - extraBots: ReadonlySet, - limits: { maxOpen: number; maxDays: number; countDrafts?: boolean } + pulls: P[], + me: string, + extraBots: ReadonlySet, + limits: { maxOpen: number; maxDays: number; countDrafts?: boolean } ): FinishLineCase

    | null { - const mine = pulls - .filter( - p => - p.data.user.login === me && - p.data.state === 'open' && - (limits.countDrafts || !p.data.draft) && - !p.deployBlockedBy.length && - !isBotLogin(p.data.user.login, extraBots) - ) - .sort(oldestFirst); - const old = mine.filter(p => p.ageDays > limits.maxDays); - const counts = { - count: mine.length, - over: old.map(idOf), - toGo: Math.max(mine.length - limits.maxOpen, old.length), - }; - if (mine.length <= limits.maxOpen && !old.length) return null; - const listed = mine.length > limits.maxOpen ? mine : old; - const ready = listed.filter(isFinishReady).length; - return { - pulls: listed, - ready, - ...counts, - }; + const mine = pulls + .filter( + (p) => + p.data.user.login === me && + p.data.state === "open" && + (limits.countDrafts || !p.data.draft) && + !p.deployBlockedBy.length && + !isBotLogin(p.data.user.login, extraBots) + ) + .sort(oldestFirst); + const old = mine.filter((p) => p.ageDays > limits.maxDays); + const counts = { + count: mine.length, + over: old.map(idOf), + toGo: Math.max(mine.length - limits.maxOpen, old.length), + }; + if (mine.length <= limits.maxOpen && !old.length) return null; + const listed = mine.length > limits.maxOpen ? mine : old; + const ready = listed.filter(isFinishReady).length; + return { + pulls: listed, + ready, + ...counts, + }; } /** @@ -100,35 +103,42 @@ export function finishLineCase

    ( * on others. Each group runs oldest first; the row's word says the rest. */ export function finishGroups(pulls: DerivedPull[], me: string) { - const worded = pulls - .map(pull => ({ pull, word: rowWord(pull, me) })) - .sort((a, b) => b.pull.ageDays - a.pull.ageDays); - const rest = worded.filter(w => !isFinishReady(w.pull)); - return { - ready: worded.filter(w => isFinishReady(w.pull)), - move: rest.filter(w => w.word.kind === 'do'), - waiting: rest.filter(w => w.word.kind === 'wait'), - }; + const worded = pulls + .map((pull) => ({ pull, word: rowWord(pull, me) })) + .sort((a, b) => b.pull.ageDays - a.pull.ageDays); + const rest = worded.filter((w) => !isFinishReady(w.pull)); + return { + ready: worded.filter((w) => isFinishReady(w.pull)), + move: rest.filter((w) => w.word.kind === "do"), + waiting: rest.filter((w) => w.word.kind === "wait"), + }; } /** The record to store when `found` shows at `now`. */ -export const finishRecord = (found: FinishLineCase, now: number): FinishRecord => ({ - at: now, - count: found.count, - over: found.over, +export const finishRecord = ( + found: FinishLineCase, + now: number +): FinishRecord => ({ + at: now, + count: found.count, + over: found.over, }); /** A stored record, or null when it's missing or unreadable. */ export function parseFinishRecord(raw: string | null): FinishRecord | null { - try { - const r = JSON.parse(raw ?? 'null') as Partial | null; - if (typeof r?.at !== 'number' || typeof r.count !== 'number' || !Array.isArray(r.over)) { - return null; - } - return { at: r.at, count: r.count, over: r.over }; - } catch { + try { + const r = JSON.parse(raw ?? "null") as Partial | null; + if ( + typeof r?.at !== "number" || + typeof r.count !== "number" || + !Array.isArray(r.over) + ) { return null; - } + } + return { at: r.at, count: r.count, over: r.over }; + } catch { + return null; + } } /** @@ -137,27 +147,29 @@ export function parseFinishRecord(raw: string | null): FinishRecord | null { * (more open PRs, net, or a PR newly past the age limit). */ export function finishDue( - found: FinishLineCase | null, - last: FinishRecord | null, - now: number + found: FinishLineCase | null, + last: FinishRecord | null, + now: number ): boolean { - if (!found) return false; - if (!last) return true; - if (now - last.at < FINISH_COOLDOWN_MS) return false; - return found.count > last.count || found.over.some(id => !last.over.includes(id)); + if (!found) return false; + if (!last) return true; + if (now - last.at < FINISH_COOLDOWN_MS) return false; + return ( + found.count > last.count || found.over.some((id) => !last.over.includes(id)) + ); } /** One number at the top of the list you can act on, and which PRs it * points at, so clicking it can open their group. */ export interface FinishStat { - key: 'open' | 'old' | 'ready'; - text: string; - /** the PRs this number counts */ - counts: (p: FinishPull) => boolean; + key: "open" | "old" | "ready"; + text: string; + /** the PRs this number counts */ + counts: (p: FinishPull) => boolean; } /** "1 PR", "2 PRs" */ -export const prCount = (n: number) => (n === 1 ? '1 PR' : `${n} PRs`); +export const prCount = (n: number) => (n === 1 ? "1 PR" : `${n} PRs`); /** * The numbers you can act on, each only when above zero: how many open @@ -165,27 +177,27 @@ export const prCount = (n: number) => (n === 1 ? '1 PR' : `${n} PRs`); * Waiting on others is left out: there's nothing to do about it here. */ export function finishStats( - found: Pick, 'count' | 'over' | 'ready'>, - limits: { maxOpen: number; maxDays: number } + found: Pick, "count" | "over" | "ready">, + limits: { maxOpen: number; maxDays: number } ): FinishStat[] { - const stats: FinishStat[] = []; - if (found.count > limits.maxOpen) - stats.push({ - key: 'open', - text: `${found.count} open PRs · aim for ${limits.maxOpen}`, - counts: () => true, - }); - if (found.over.length) - stats.push({ - key: 'old', - text: `${prCount(found.over.length)} older than ${limits.maxDays} days`, - counts: p => p.ageDays > limits.maxDays, - }); - if (found.ready) - stats.push({ - key: 'ready', - text: `${prCount(found.ready)} ready to merge`, - counts: isFinishReady, - }); - return stats; + const stats: FinishStat[] = []; + if (found.count > limits.maxOpen) + stats.push({ + key: "open", + text: `${found.count} open PRs, aim for ${limits.maxOpen}`, + counts: () => true, + }); + if (found.over.length) + stats.push({ + key: "old", + text: `${prCount(found.over.length)} older than ${limits.maxDays} days`, + counts: (p) => p.ageDays > limits.maxDays, + }); + if (found.ready) + stats.push({ + key: "ready", + text: `${prCount(found.ready)} ready to merge`, + counts: isFinishReady, + }); + return stats; } From 54d84300d57993062e3482db996d81e9452f7e3b Mon Sep 17 00:00:00 2001 From: Jarred Stelfox Date: Mon, 12 Oct 2026 13:35:45 -0700 Subject: [PATCH 6/6] Keep Finish line's focus in and its files in style The dialog now opens with focus on itself, and the trap only knew the first and last controls, so Shift+Tab from the dialog walked out onto the board behind it. Tab from the dialog now goes in at either end. The review fix commit had reformatted five files to prettier's defaults (an editor hook from another repo), turning ~100 real Settings lines into ~1000; they're back in the house style (3-space, single quotes, 100 columns), with app.tsx too. The stats are spaced instead of dotted, so a wrap at phone width never leaves a dot hanging, and an old PR's age is set apart by weight now that it isn't amber. Co-Authored-By: Claude Opus 5.5 --- frontend-v2/src/app.tsx | 7 +- .../src/components/FinishLine.test.tsx | 1092 ++++++----- frontend-v2/src/components/FinishLine.tsx | 1640 ++++++++--------- frontend-v2/src/components/Legend.tsx | 426 ++--- frontend-v2/src/components/Settings.tsx | 1070 ++++++----- frontend-v2/src/model/finishLine.ts | 240 ++- 6 files changed, 2150 insertions(+), 2325 deletions(-) diff --git a/frontend-v2/src/app.tsx b/frontend-v2/src/app.tsx index 5d6d9b36..4869b95f 100644 --- a/frontend-v2/src/app.tsx +++ b/frontend-v2/src/app.tsx @@ -1142,7 +1142,12 @@ export function App() { > v1 board - + { - n += 1; - const at = new Date(Date.now() - ageDays * 86400_000 - 60_000).toISOString(); - const data: PullData = { - repo: "iFixit/ifixit", - number: n, - state: "open", - title: `PR ${n}`, - body: "", - draft: false, - created_at: at, - updated_at: at, - closed_at: null, - merged_at: null, - mergeable: true, - difficulty: null, - additions: 10, - deletions: 5, - changed_files: 2, - milestone: { title: null, due_on: null }, - head: { ref: "branch", sha: "abc", repo: { owner: { login: "iFixit" } } }, - base: { ref: "master" }, - user: { login: "me" }, - status: { - cr_req: ready ? 0 : 1, - qa_req: ready ? 0 : 1, - allCR: [], - allQA: [], - dev_block: [], - deploy_block: [], - commit_statuses: [], - }, - labels: [], - participants: [], - requested_reviewers: asked, - review_requests: asked.map((login) => ({ - login, - at: Date.now() / 1000 - 3600, - self: false, - })), - input_hints: hints, - }; - return derive(data, undefined); + n += 1; + const at = new Date(Date.now() - ageDays * 86400_000 - 60_000).toISOString(); + const data: PullData = { + repo: 'iFixit/ifixit', + number: n, + state: 'open', + title: `PR ${n}`, + body: '', + draft: false, + created_at: at, + updated_at: at, + closed_at: null, + merged_at: null, + mergeable: true, + difficulty: null, + additions: 10, + deletions: 5, + changed_files: 2, + milestone: { title: null, due_on: null }, + head: { ref: 'branch', sha: 'abc', repo: { owner: { login: 'iFixit' } } }, + base: { ref: 'master' }, + user: { login: 'me' }, + status: { + cr_req: ready ? 0 : 1, + qa_req: ready ? 0 : 1, + allCR: [], + allQA: [], + dev_block: [], + deploy_block: [], + commit_statuses: [], + }, + labels: [], + participants: [], + requested_reviewers: asked, + review_requests: asked.map(login => ({ + login, + at: Date.now() / 1000 - 3600, + self: false, + })), + input_hints: hints, + }; + return derive(data, undefined); }; const many = (count: number) => Array.from({ length: count }, () => pull()); let root: Root | null = null; -const host = document.createElement("div"); +const host = document.createElement('div'); document.body.append(host); // most of these watch it pop up, and close it the old way, by holding -beforeEach(() => - setSettings({ finishLineMode: "auto", finishLineHoldToClose: true }) -); +beforeEach(() => setSettings({ finishLineMode: 'auto', finishLineHoldToClose: true })); afterEach(() => { - act(() => root?.unmount()); - root = null; - localStorage.clear(); - vi.useRealTimers(); + act(() => root?.unmount()); + root = null; + localStorage.clear(); + vi.useRealTimers(); }); function render(pulls: DerivedPull[]) { - const r = (root ??= createRoot(host)); - act(() => - r.render( - - ) - ); + const r = (root ??= createRoot(host)); + act(() => r.render()); } /** the stats row's buttons, by their words */ const stats = () => - [...document.querySelectorAll("#finish-stats button")].map( - (b) => b.textContent - ); + [...document.querySelectorAll('#finish-stats button')].map( + b => b.textContent + ); const stat = (text: string) => - [ - ...document.querySelectorAll("#finish-stats button"), - ].find((b) => b.textContent === text); -const listed = () => - document.querySelectorAll('[role="dialog"] section li').length; + [...document.querySelectorAll('#finish-stats button')].find( + b => b.textContent === text + ); +const listed = () => document.querySelectorAll('[role="dialog"] section li').length; const dialog = () => document.querySelector('[role="dialog"]'); -const countdown = () => document.getElementById("finish-title")?.textContent; +const countdown = () => document.getElementById('finish-title')?.textContent; const badge = () => - document.querySelector( - '[aria-label^="Finish"][aria-label$="open PRs"]' - ); + document.querySelector('[aria-label^="Finish"][aria-label$="open PRs"]'); const esc = () => - act(() => { - document.body.dispatchEvent( - new KeyboardEvent("keydown", { - key: "Escape", - bubbles: true, - cancelable: true, - }) - ); - }); -const headers = () => - [...document.querySelectorAll('[role="dialog"] h3')].map( - (h) => h.textContent - ); - -describe("FinishLine while it is up", () => { - it("follows new PRs and merges live", () => { - const first = many(8); - render(first); - expect(stats()).toEqual(["8 open PRs, aim for 7"]); - expect(listed()).toBe(8); - - // a new PR arrives over the socket - render([...first, pull()]); - expect(stats()).toEqual(["9 open PRs, aim for 7"]); - expect(listed()).toBe(9); - - expect(countdown()).toBe("Finish 2 PRs"); - - // merges bring you under the limits: it says so, then lets you go - vi.useFakeTimers(); - render(first.slice(0, 3)); - expect(stats()).toEqual([]); - expect(countdown()).toBe("All finished"); - expect(dialog()?.textContent).toContain( - "None of your open PRs are past your limits." - ); - expect(listed()).toBe(0); - act(() => vi.advanceTimersByTime(2500)); - expect(dialog()).toBeNull(); - }); - - it("lifts ready PRs to the top and counts them in the stats", () => { - const ready = pull(2, { ready: true }); - render([...many(7), ready]); - // the rest are mine to stamp under self-review - expect(headers()).toEqual(["Ready to merge", "Your move"]); - expect( - document.querySelector('[role="dialog"] li a')?.textContent - ).toContain(ready.data.title); - expect(countdown()).toBe("Finish 1 PR"); - expect(stats()).toEqual(["8 open PRs, aim for 7", "1 PR ready to merge"]); - expect(dialog()?.textContent).toContain("Only you see this."); - }); - - it("only shows the badge until you turn the popup on", () => { - setSettings({ finishLineMode: DEFAULT_SETTINGS.finishLineMode }); - render(many(9)); - expect(dialog()).toBeNull(); - expect(badge()).not.toBeNull(); - }); - - it("only shows the badge when the popup is off", () => { - setSettings({ finishLineMode: "manual" }); - render(many(9)); - expect(document.querySelector('[role="dialog"]')).toBeNull(); - expect(badge()?.getAttribute("aria-label")).toBe( - "Finish 2 of your open PRs" - ); - expect(badge()?.textContent).toBe("2 to finish"); - act(() => badge()?.click()); - expect(countdown()).toBe("Finish 2 PRs"); - }); - - it("stays away, badge and all, when it is off", () => { - setSettings({ finishLineMode: "off" }); - render(many(9)); - expect(document.querySelector('[role="dialog"]')).toBeNull(); - expect(badge()).toBeNull(); - }); - - it("with the hold setting, closes only when the button is held down", () => { - vi.useFakeTimers(); - render(many(8)); - const hold = [ - ...document.querySelectorAll('[role="dialog"] button'), - ].find((b) => b.textContent?.includes("Hold to close")); - // it has focus from the start, so the keyboard can hold it right away - expect(document.activeElement).toBe(hold); - const key = (type: string, k = "Enter") => + act(() => { + document.body.dispatchEvent( + new KeyboardEvent('keydown', { + key: 'Escape', + bubbles: true, + cancelable: true, + }) + ); + }); +const headers = () => [...document.querySelectorAll('[role="dialog"] h3')].map(h => h.textContent); + +describe('FinishLine while it is up', () => { + it('follows new PRs and merges live', () => { + const first = many(8); + render(first); + expect(stats()).toEqual(['8 open PRs, aim for 7']); + expect(listed()).toBe(8); + + // a new PR arrives over the socket + render([...first, pull()]); + expect(stats()).toEqual(['9 open PRs, aim for 7']); + expect(listed()).toBe(9); + + expect(countdown()).toBe('Finish 2 PRs'); + + // merges bring you under the limits: it says so, then lets you go + vi.useFakeTimers(); + render(first.slice(0, 3)); + expect(stats()).toEqual([]); + expect(countdown()).toBe('All finished'); + expect(dialog()?.textContent).toContain('None of your open PRs are past your limits.'); + expect(listed()).toBe(0); + act(() => vi.advanceTimersByTime(2500)); + expect(dialog()).toBeNull(); + }); + + it('lifts ready PRs to the top and counts them in the stats', () => { + const ready = pull(2, { ready: true }); + render([...many(7), ready]); + // the rest are mine to stamp under self-review + expect(headers()).toEqual(['Ready to merge', 'Your move']); + expect(document.querySelector('[role="dialog"] li a')?.textContent).toContain( + ready.data.title + ); + expect(countdown()).toBe('Finish 1 PR'); + expect(stats()).toEqual(['8 open PRs, aim for 7', '1 PR ready to merge']); + expect(dialog()?.textContent).toContain('Only you see this.'); + }); + + it('only shows the badge until you turn the popup on', () => { + setSettings({ finishLineMode: DEFAULT_SETTINGS.finishLineMode }); + render(many(9)); + expect(dialog()).toBeNull(); + expect(badge()).not.toBeNull(); + }); + + it('only shows the badge when the popup is off', () => { + setSettings({ finishLineMode: 'manual' }); + render(many(9)); + expect(document.querySelector('[role="dialog"]')).toBeNull(); + expect(badge()?.getAttribute('aria-label')).toBe('Finish 2 of your open PRs'); + expect(badge()?.textContent).toBe('2 to finish'); + act(() => badge()?.click()); + expect(countdown()).toBe('Finish 2 PRs'); + }); + + it('stays away, badge and all, when it is off', () => { + setSettings({ finishLineMode: 'off' }); + render(many(9)); + expect(document.querySelector('[role="dialog"]')).toBeNull(); + expect(badge()).toBeNull(); + }); + + it('with the hold setting, closes only when the button is held down', () => { + vi.useFakeTimers(); + render(many(8)); + const hold = [...document.querySelectorAll('[role="dialog"] button')].find( + b => b.textContent?.includes('Hold to close') + ); + // it has focus from the start, so the keyboard can hold it right away + expect(document.activeElement).toBe(hold); + const key = (type: string, k = 'Enter') => + act(() => hold?.dispatchEvent(new KeyboardEvent(type, { key: k, bubbles: true }))); + // a tap of Enter does nothing but say to hold it; neither does Esc + key('keydown'); + act(() => vi.advanceTimersByTime(200)); + key('keyup'); + expect(hold?.textContent).toBe('Hold it down'); + act(() => vi.advanceTimersByTime(3000)); + act(() => document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' }))); + expect(dialog()).not.toBeNull(); + // holding Space the full two seconds turns the key, then it opens + key('keydown', ' '); + act(() => vi.advanceTimersByTime(2000)); + act(() => vi.advanceTimersByTime(700)); + expect(dialog()).toBeNull(); + }); + + it('slides a merged PR out of the list as the countdown ticks', () => { + vi.useFakeTimers(); + const nine = many(9); + render(nine); + expect(countdown()).toBe('Finish 2 PRs'); + render(nine.slice(1)); + expect(countdown()).toBe('Finish 1 PR'); + // still there for the slide, then gone + expect(document.querySelectorAll('.finish-row-leave')).toHaveLength(1); + expect(listed()).toBe(9); + act(() => vi.advanceTimersByTime(450)); + expect(listed()).toBe(8); + }); + + it('copies a nudge listing only the PRs waiting on people I asked, naming them', async () => { + const writeText = vi.fn(() => Promise.resolve()); + Object.assign(navigator, { clipboard: { writeText } }); + const first = pull(3, { asked: ['bob'] }); + const second = pull(2, { asked: ['cy', 'dee'] }); + render([first, second, ...many(6)]); + expect(headers()).toEqual(['Your move', 'Waiting on others']); + const copy = [...document.querySelectorAll('[role="dialog"] button')].find( + b => b.textContent === 'Copy a nudge' + ); + await act(async () => copy?.click()); + const text = (writeText.mock.calls[0] as unknown as [string])[0]; + expect(text.split('\n')).toHaveLength(3); + expect(text).toContain( + `• ${first.data.title} https://github.com/iFixit/ifixit/pull/${first.data.number} (bob)` + ); + expect(text).toContain(`(cy, dee)`); + expect(copy?.textContent).toBe('Copied'); + }); + + it('offers no nudge when I asked nobody: a self-review waits on no one', () => { + render(many(8)); + const copy = [...document.querySelectorAll('[role="dialog"] button')].find( + b => b.textContent === 'Copy a nudge' + ); + expect(copy).toBeUndefined(); + }); + + it('hints when a self-reviewed PR touches what usually deserves team input', () => { + render([pull(1, { hints: ['ci', 'migrations'] }), ...many(7)]); + expect(dialog()?.textContent).toContain( + 'touches CI and migrations · worth asking for review?' + ); + }); + + it('lets a screen reader or voice control close the hold with a lone click', () => { + render(many(8)); + const hold = [...document.querySelectorAll('[role="dialog"] button')].find( + b => b.textContent?.includes('Hold to close') + ); + // a mouse click carries detail 1 and does nothing + act(() => hold?.dispatchEvent(new MouseEvent('click', { bubbles: true, detail: 1 }))); + expect(dialog()).not.toBeNull(); + // assistive tech sends a click with no pointer or key behind it + act(() => hold?.dispatchEvent(new MouseEvent('click', { bubbles: true, detail: 0 }))); + expect(dialog()).toBeNull(); + }); + + it('with the hold setting, keeps Esc from closing it or reaching anything behind it', () => { + render(many(8)); + let reached = false; + const behind = (e: KeyboardEvent) => { + if (!e.defaultPrevented) reached = true; + }; + document.addEventListener('keydown', behind); + act(() => { + document.body.dispatchEvent( + new KeyboardEvent('keydown', { + key: 'Escape', + bubbles: true, + cancelable: true, + }) + ); + }); + document.removeEventListener('keydown', behind); + expect(reached).toBe(false); + expect(dialog()).not.toBeNull(); + }); + + it('zeroes the last showing once you are under the limits, but keeps its time', () => { + vi.useFakeTimers(); + const nine = many(9); + render(nine); + const shownAt = JSON.parse(localStorage.getItem('pd2.finishLine') ?? 'null').at; + render(nine.slice(0, 3)); + expect(JSON.parse(localStorage.getItem('pd2.finishLine') ?? 'null')).toEqual({ + at: shownAt, + count: 0, + over: [], + }); + }); + + it('going back over within 4 hours waits out the rest of them', () => { + const nine = many(9); + render(nine); act(() => - hold?.dispatchEvent(new KeyboardEvent(type, { key: k, bubbles: true })) + document + .querySelector('[data-hold]') + ?.dispatchEvent(new MouseEvent('click', { bubbles: true, detail: 0 })) ); - // a tap of Enter does nothing but say to hold it; neither does Esc - key("keydown"); - act(() => vi.advanceTimersByTime(200)); - key("keyup"); - expect(hold?.textContent).toBe("Hold it down"); - act(() => vi.advanceTimersByTime(3000)); - act(() => - document.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape" })) - ); - expect(dialog()).not.toBeNull(); - // holding Space the full two seconds turns the key, then it opens - key("keydown", " "); - act(() => vi.advanceTimersByTime(2000)); - act(() => vi.advanceTimersByTime(700)); - expect(dialog()).toBeNull(); - }); - - it("slides a merged PR out of the list as the countdown ticks", () => { - vi.useFakeTimers(); - const nine = many(9); - render(nine); - expect(countdown()).toBe("Finish 2 PRs"); - render(nine.slice(1)); - expect(countdown()).toBe("Finish 1 PR"); - // still there for the slide, then gone - expect(document.querySelectorAll(".finish-row-leave")).toHaveLength(1); - expect(listed()).toBe(9); - act(() => vi.advanceTimersByTime(450)); - expect(listed()).toBe(8); - }); - - it("copies a nudge listing only the PRs waiting on people I asked, naming them", async () => { - const writeText = vi.fn(() => Promise.resolve()); - Object.assign(navigator, { clipboard: { writeText } }); - const first = pull(3, { asked: ["bob"] }); - const second = pull(2, { asked: ["cy", "dee"] }); - render([first, second, ...many(6)]); - expect(headers()).toEqual(["Your move", "Waiting on others"]); - const copy = [ - ...document.querySelectorAll('[role="dialog"] button'), - ].find((b) => b.textContent === "Copy a nudge"); - await act(async () => copy?.click()); - const text = (writeText.mock.calls[0] as unknown as [string])[0]; - expect(text.split("\n")).toHaveLength(3); - expect(text).toContain( - `• ${first.data.title} https://github.com/iFixit/ifixit/pull/${first.data.number} (bob)` - ); - expect(text).toContain(`(cy, dee)`); - expect(copy?.textContent).toBe("Copied"); - }); - - it("offers no nudge when I asked nobody: a self-review waits on no one", () => { - render(many(8)); - const copy = [...document.querySelectorAll('[role="dialog"] button')].find( - (b) => b.textContent === "Copy a nudge" - ); - expect(copy).toBeUndefined(); - }); - - it("hints when a self-reviewed PR touches what usually deserves team input", () => { - render([pull(1, { hints: ["ci", "migrations"] }), ...many(7)]); - expect(dialog()?.textContent).toContain( - "touches CI and migrations · worth asking for review?" - ); - }); - - it("lets a screen reader or voice control close the hold with a lone click", () => { - render(many(8)); - const hold = [ - ...document.querySelectorAll('[role="dialog"] button'), - ].find((b) => b.textContent?.includes("Hold to close")); - // a mouse click carries detail 1 and does nothing - act(() => - hold?.dispatchEvent(new MouseEvent("click", { bubbles: true, detail: 1 })) - ); - expect(dialog()).not.toBeNull(); - // assistive tech sends a click with no pointer or key behind it - act(() => - hold?.dispatchEvent(new MouseEvent("click", { bubbles: true, detail: 0 })) - ); - expect(dialog()).toBeNull(); - }); - - it("with the hold setting, keeps Esc from closing it or reaching anything behind it", () => { - render(many(8)); - let reached = false; - const behind = (e: KeyboardEvent) => { - if (!e.defaultPrevented) reached = true; - }; - document.addEventListener("keydown", behind); - act(() => { - document.body.dispatchEvent( - new KeyboardEvent("keydown", { - key: "Escape", - bubbles: true, - cancelable: true, - }) + expect(dialog()).toBeNull(); + render(nine.slice(0, 3)); // under: the record zeroes + render([...nine, pull()]); // over again, minutes later + expect(dialog()).toBeNull(); + }); + + it('board hotkeys stand down while it is open', async () => { + const { modalOpen } = await import('../hooks'); + render(many(8)); + expect(modalOpen()).toBe(true); + }); + + it('opens the preview every time Settings asks, or the URL says #finish=1', () => { + render(many(2)); + expect(dialog()).toBeNull(); + act(() => window.dispatchEvent(new Event('pd2:preview-finish-line'))); + expect(dialog()).not.toBeNull(); + act(() => + document + .querySelector('[data-hold]') + ?.dispatchEvent(new MouseEvent('click', { bubbles: true, detail: 0 })) ); - }); - document.removeEventListener("keydown", behind); - expect(reached).toBe(false); - expect(dialog()).not.toBeNull(); - }); - - it("zeroes the last showing once you are under the limits, but keeps its time", () => { - vi.useFakeTimers(); - const nine = many(9); - render(nine); - const shownAt = JSON.parse( - localStorage.getItem("pd2.finishLine") ?? "null" - ).at; - render(nine.slice(0, 3)); - expect( - JSON.parse(localStorage.getItem("pd2.finishLine") ?? "null") - ).toEqual({ - at: shownAt, - count: 0, - over: [], - }); - }); - - it("going back over within 4 hours waits out the rest of them", () => { - const nine = many(9); - render(nine); - act(() => - document - .querySelector("[data-hold]") - ?.dispatchEvent(new MouseEvent("click", { bubbles: true, detail: 0 })) - ); - expect(dialog()).toBeNull(); - render(nine.slice(0, 3)); // under: the record zeroes - render([...nine, pull()]); // over again, minutes later - expect(dialog()).toBeNull(); - }); - - it("board hotkeys stand down while it is open", async () => { - const { modalOpen } = await import("../hooks"); - render(many(8)); - expect(modalOpen()).toBe(true); - }); - - it("opens the preview every time Settings asks, or the URL says #finish=1", () => { - render(many(2)); - expect(dialog()).toBeNull(); - act(() => window.dispatchEvent(new Event("pd2:preview-finish-line"))); - expect(dialog()).not.toBeNull(); - act(() => - document - .querySelector("[data-hold]") - ?.dispatchEvent(new MouseEvent("click", { bubbles: true, detail: 0 })) - ); - expect(dialog()).toBeNull(); - act(() => { - location.hash = "finish=1"; - window.dispatchEvent(new HashChangeEvent("hashchange")); - }); - expect(dialog()).not.toBeNull(); - location.hash = ""; - }); - - it("a quick Space tap is not a lone click, even where the browser clicks on keyup", () => { - render(many(8)); - const hold = [ - ...document.querySelectorAll('[role="dialog"] button'), - ].find((b) => b.textContent?.includes("Hold to close")); - act(() => { - hold?.dispatchEvent( - new KeyboardEvent("keydown", { key: " ", bubbles: true }) + expect(dialog()).toBeNull(); + act(() => { + location.hash = 'finish=1'; + window.dispatchEvent(new HashChangeEvent('hashchange')); + }); + expect(dialog()).not.toBeNull(); + location.hash = ''; + }); + + it('a quick Space tap is not a lone click, even where the browser clicks on keyup', () => { + render(many(8)); + const hold = [...document.querySelectorAll('[role="dialog"] button')].find( + b => b.textContent?.includes('Hold to close') ); - hold?.dispatchEvent( - new KeyboardEvent("keyup", { key: " ", bubbles: true }) + act(() => { + hold?.dispatchEvent(new KeyboardEvent('keydown', { key: ' ', bubbles: true })); + hold?.dispatchEvent(new KeyboardEvent('keyup', { key: ' ', bubbles: true })); + // Firefox's keyup click, with no pointer behind it + hold?.dispatchEvent(new MouseEvent('click', { bubbles: true, detail: 0 })); + }); + expect(dialog()).not.toBeNull(); + }); + + it('keeps the last showing while it doesn’t know who you are', () => { + localStorage.setItem( + 'pd2.finishLine', + JSON.stringify({ at: Date.now(), count: 9, over: [] }) + ); + const r = (root ??= createRoot(host)); + act(() => r.render()); + expect(localStorage.getItem('pd2.finishLine')).not.toBeNull(); + expect(dialog()).toBeNull(); + }); + + it('stays down when another window showed it a moment ago', () => { + // another window wrote the record after this one loaded + const r = (root ??= createRoot(host)); + act(() => + r.render() ); - // Firefox's keyup click, with no pointer behind it - hold?.dispatchEvent( - new MouseEvent("click", { bubbles: true, detail: 0 }) + localStorage.setItem( + 'pd2.finishLine', + JSON.stringify({ at: Date.now(), count: 8, over: [] }) ); - }); - expect(dialog()).not.toBeNull(); - }); - - it("keeps the last showing while it doesn’t know who you are", () => { - localStorage.setItem( - "pd2.finishLine", - JSON.stringify({ at: Date.now(), count: 9, over: [] }) - ); - const r = (root ??= createRoot(host)); - act(() => - r.render( - - ) - ); - expect(localStorage.getItem("pd2.finishLine")).not.toBeNull(); - expect(dialog()).toBeNull(); - }); - - it("stays down when another window showed it a moment ago", () => { - // another window wrote the record after this one loaded - const r = (root ??= createRoot(host)); - act(() => - r.render( - - ) - ); - localStorage.setItem( - "pd2.finishLine", - JSON.stringify({ at: Date.now(), count: 8, over: [] }) - ); - act(() => - r.render( - - ) - ); - expect(dialog()).toBeNull(); - }); - - it("waits while another dialog is open", () => { - const other = document.createElement("div"); - other.setAttribute("aria-modal", "true"); - document.body.append(other); - render(many(9)); - expect(dialog()).toBeNull(); - other.remove(); - }); - - it("a drop ends a drag even when the dragged row left the page", () => { - const row = document.createElement("div"); - document.body.append(row); - row.dispatchEvent(new Event("dragstart", { bubbles: true })); - row.remove(); // moved to another lane: its dragend never reaches the page - document.body.dispatchEvent(new Event("drop", { bubbles: true })); - render(many(9)); - expect(dialog()).not.toBeNull(); - }); - - it("closes on Esc or the close button, and stays away until things get worse", () => { - setSettings({ finishLineHoldToClose: false }); - const nine = many(9); - render(nine); - // focus starts on the dialog itself, so a stray Enter can't close it; - // there's no hold button - expect(document.activeElement).toBe(dialog()); - expect( - document.querySelector("[data-close]")?.getAttribute("aria-label") - ).toBe("Close"); - expect(document.querySelector("[data-hold]")).toBeNull(); - esc(); - expect(dialog()).toBeNull(); - render([...nine, pull()]); // worse, minutes later: within the 4 hours - expect(dialog()).toBeNull(); - act(() => badge()?.click()); - expect(dialog()).not.toBeNull(); - act(() => - document.querySelector("[data-close]")?.click() - ); - expect(dialog()).toBeNull(); - }); - - it("the cog closes it and opens Settings at its own settings", () => { - setSettings({ finishLineHoldToClose: false }); - const asked: unknown[] = []; - const listen = (e: Event) => asked.push((e as CustomEvent).detail); - window.addEventListener("pd2:open-settings", listen); - render(many(9)); - act(() => - document - .querySelector('[aria-label="Finish line settings"]') - ?.click() - ); - window.removeEventListener("pd2:open-settings", listen); - expect(asked).toEqual(["finish-line"]); - expect(dialog()).toBeNull(); - }); + act(() => r.render()); + expect(dialog()).toBeNull(); + }); + + it('waits while another dialog is open', () => { + const other = document.createElement('div'); + other.setAttribute('aria-modal', 'true'); + document.body.append(other); + render(many(9)); + expect(dialog()).toBeNull(); + other.remove(); + }); + + it('a drop ends a drag even when the dragged row left the page', () => { + const row = document.createElement('div'); + document.body.append(row); + row.dispatchEvent(new Event('dragstart', { bubbles: true })); + row.remove(); // moved to another lane: its dragend never reaches the page + document.body.dispatchEvent(new Event('drop', { bubbles: true })); + render(many(9)); + expect(dialog()).not.toBeNull(); + }); + + it('closes on Esc or the close button, and stays away until things get worse', () => { + setSettings({ finishLineHoldToClose: false }); + const nine = many(9); + render(nine); + // focus starts on the dialog itself, so a stray Enter can't close it; + // there's no hold button + expect(document.activeElement).toBe(dialog()); + expect(document.querySelector('[data-close]')?.getAttribute('aria-label')).toBe('Close'); + expect(document.querySelector('[data-hold]')).toBeNull(); + esc(); + expect(dialog()).toBeNull(); + render([...nine, pull()]); // worse, minutes later: within the 4 hours + expect(dialog()).toBeNull(); + act(() => badge()?.click()); + expect(dialog()).not.toBeNull(); + act(() => document.querySelector('[data-close]')?.click()); + expect(dialog()).toBeNull(); + }); + + it('the cog closes it and opens Settings at its own settings', () => { + setSettings({ finishLineHoldToClose: false }); + const asked: unknown[] = []; + const listen = (e: Event) => asked.push((e as CustomEvent).detail); + window.addEventListener('pd2:open-settings', listen); + render(many(9)); + act(() => + document.querySelector('[aria-label="Finish line settings"]')?.click() + ); + window.removeEventListener('pd2:open-settings', listen); + expect(asked).toEqual(['finish-line']); + expect(dialog()).toBeNull(); + }); }); -describe("FinishLine stats", () => { - beforeEach(() => setSettings({ finishLineHoldToClose: false })); - - it("shows only the numbers above zero, old PRs alone when the count is fine", () => { - render([pull(20), pull(30), pull(1)]); - expect(countdown()).toBe("Finish 2 PRs"); - expect(stats()).toEqual(["2 PRs older than 14 days"]); - // the list is the old ones only - expect(listed()).toBe(2); - }); - - it("leaves waiting on others out of the stats", () => { - render([pull(2, { asked: ["bob"] }), ...many(7)]); - expect(headers()).toEqual(["Your move", "Waiting on others"]); - expect(stats()).toEqual(["8 open PRs, aim for 7"]); - }); - - it("jumps to and focuses the first PR a stat counts", () => { - const scrolled = vi.fn(); - Element.prototype.scrollIntoView = scrolled; - const ready = pull(2, { ready: true }); - const old = pull(20, { asked: ["bob"] }); - render([ready, old, ...many(7)]); - expect(stats()).toEqual([ - "9 open PRs, aim for 7", - "1 PR older than 14 days", - "1 PR ready to merge", - ]); - const row = (p: DerivedPull) => - document.querySelector( - `[role="dialog"] li[data-pull="${p.data.repo}#${p.data.number}"]` - ); - // the old PR waits on bob, at the bottom of the list - act(() => stat("1 PR older than 14 days")?.click()); - expect(scrolled.mock.contexts.at(-1)).toBe(row(old)); - expect(document.activeElement).toBe(row(old)?.querySelector("a")); - act(() => stat("1 PR ready to merge")?.click()); - expect(document.activeElement).toBe(row(ready)?.querySelector("a")); - // the open count counts them all: the first row in the list - act(() => (document.activeElement as HTMLElement).blur()); - act(() => stat("9 open PRs, aim for 7")?.click()); - expect(document.activeElement).toBe( - document.querySelector('[role="dialog"] li a') - ); - }); +describe('FinishLine stats', () => { + beforeEach(() => setSettings({ finishLineHoldToClose: false })); + + it('shows only the numbers above zero, old PRs alone when the count is fine', () => { + render([pull(20), pull(30), pull(1)]); + expect(countdown()).toBe('Finish 2 PRs'); + expect(stats()).toEqual(['2 PRs older than 14 days']); + // the list is the old ones only + expect(listed()).toBe(2); + }); + + it('leaves waiting on others out of the stats', () => { + render([pull(2, { asked: ['bob'] }), ...many(7)]); + expect(headers()).toEqual(['Your move', 'Waiting on others']); + expect(stats()).toEqual(['8 open PRs, aim for 7']); + }); + + it('jumps to and focuses the first PR a stat counts', () => { + const scrolled = vi.fn(); + Element.prototype.scrollIntoView = scrolled; + const ready = pull(2, { ready: true }); + const old = pull(20, { asked: ['bob'] }); + render([ready, old, ...many(7)]); + expect(stats()).toEqual([ + '9 open PRs, aim for 7', + '1 PR older than 14 days', + '1 PR ready to merge', + ]); + const row = (p: DerivedPull) => + document.querySelector(`[role="dialog"] li[data-pull="${p.data.repo}#${p.data.number}"]`); + // the old PR waits on bob, at the bottom of the list + act(() => stat('1 PR older than 14 days')?.click()); + expect(scrolled.mock.contexts.at(-1)).toBe(row(old)); + expect(document.activeElement).toBe(row(old)?.querySelector('a')); + act(() => stat('1 PR ready to merge')?.click()); + expect(document.activeElement).toBe(row(ready)?.querySelector('a')); + // the open count counts them all: the first row in the list + act(() => (document.activeElement as HTMLElement).blur()); + act(() => stat('9 open PRs, aim for 7')?.click()); + expect(document.activeElement).toBe(document.querySelector('[role="dialog"] li a')); + }); }); -describe("FinishLine focus and preview", () => { - beforeEach(() => setSettings({ finishLineHoldToClose: false })); - - const tab = (shiftKey = false) => { - const e = new KeyboardEvent("keydown", { - key: "Tab", - shiftKey, - bubbles: true, - cancelable: true, - }); - act(() => { - document.activeElement?.dispatchEvent(e); - }); - return e; - }; - - it("wraps Tab past a row sliding out, so focus never leaves the list", () => { - const nine = many(9); - render(nine); - // the first PR merges: its row slides out at the end of the list - render(nine.slice(1)); - const leaving = document.querySelector(".finish-row-leave"); - const links = [ - ...document.querySelectorAll('[role="dialog"] a[href]'), - ]; - expect(links.at(-1)?.closest("li")).toBe(leaving); - const lastLive = links.at(-2); - act(() => lastLive?.focus()); - expect(tab().defaultPrevented).toBe(true); - const first = document.querySelector('[role="dialog"] button'); - expect(document.activeElement).toBe(first); - // and back the other way - expect(tab(true).defaultPrevented).toBe(true); - expect(document.activeElement).toBe(lastLive); - }); - - it("skips anything hidden when wrapping", () => { - render(many(9)); - const links = [ - ...document.querySelectorAll('[role="dialog"] a[href]'), - ]; - links.at(-1)?.closest("li")?.setAttribute("hidden", ""); - act(() => links.at(-2)?.focus()); - expect(tab().defaultPrevented).toBe(true); - expect(document.activeElement).toBe( - document.querySelector('[role="dialog"] button') - ); - }); - - it("previews your real numbers even with it off", () => { - setSettings({ finishLineMode: "off" }); - render([pull(20), ...many(8)]); - expect(badge()).toBeNull(); - act(() => window.dispatchEvent(new Event("pd2:preview-finish-line"))); - expect(countdown()).toBe("Finish 2 PRs"); - expect(stats()).toEqual([ - "9 open PRs, aim for 7", - "1 PR older than 14 days", - ]); - }); - - it("previews a sample only when you are under the limits", () => { - setSettings({ finishLineMode: "off" }); - render(many(2)); - act(() => window.dispatchEvent(new Event("pd2:preview-finish-line"))); - expect(countdown()).toBe("Nothing to finish"); - expect(stats()).toEqual([]); - expect(listed()).toBe(2); - // nothing to describe it with, so it points at nothing - expect(dialog()?.hasAttribute("aria-describedby")).toBe(false); - }); +describe('FinishLine focus and preview', () => { + beforeEach(() => setSettings({ finishLineHoldToClose: false })); + + const tab = (shiftKey = false) => { + const e = new KeyboardEvent('keydown', { + key: 'Tab', + shiftKey, + bubbles: true, + cancelable: true, + }); + act(() => { + document.activeElement?.dispatchEvent(e); + }); + return e; + }; + + it('wraps Tab past a row sliding out, so focus never leaves the list', () => { + const nine = many(9); + render(nine); + // the first PR merges: its row slides out at the end of the list + render(nine.slice(1)); + const leaving = document.querySelector('.finish-row-leave'); + const links = [...document.querySelectorAll('[role="dialog"] a[href]')]; + expect(links.at(-1)?.closest('li')).toBe(leaving); + const lastLive = links.at(-2); + act(() => lastLive?.focus()); + expect(tab().defaultPrevented).toBe(true); + const first = document.querySelector('[role="dialog"] button'); + expect(document.activeElement).toBe(first); + // and back the other way + expect(tab(true).defaultPrevented).toBe(true); + expect(document.activeElement).toBe(lastLive); + }); + + it('sends Tab from the dialog itself to either end', () => { + render(many(9)); + const root = dialog() as HTMLElement; + const reach = [...root.querySelectorAll('a[href], button')]; + act(() => root.focus()); + expect(tab(true).defaultPrevented).toBe(true); + expect(document.activeElement).toBe(reach.at(-1)); + act(() => root.focus()); + expect(tab().defaultPrevented).toBe(true); + expect(document.activeElement).toBe(reach[0]); + }); + + it('skips anything hidden when wrapping', () => { + render(many(9)); + const links = [...document.querySelectorAll('[role="dialog"] a[href]')]; + links.at(-1)?.closest('li')?.setAttribute('hidden', ''); + act(() => links.at(-2)?.focus()); + expect(tab().defaultPrevented).toBe(true); + expect(document.activeElement).toBe(document.querySelector('[role="dialog"] button')); + }); + + it('previews your real numbers even with it off', () => { + setSettings({ finishLineMode: 'off' }); + render([pull(20), ...many(8)]); + expect(badge()).toBeNull(); + act(() => window.dispatchEvent(new Event('pd2:preview-finish-line'))); + expect(countdown()).toBe('Finish 2 PRs'); + expect(stats()).toEqual(['9 open PRs, aim for 7', '1 PR older than 14 days']); + }); + + it('previews a sample only when you are under the limits', () => { + setSettings({ finishLineMode: 'off' }); + render(many(2)); + act(() => window.dispatchEvent(new Event('pd2:preview-finish-line'))); + expect(countdown()).toBe('Nothing to finish'); + expect(stats()).toEqual([]); + expect(listed()).toBe(2); + // nothing to describe it with, so it points at nothing + expect(dialog()?.hasAttribute('aria-describedby')).toBe(false); + }); }); -describe("Settings, opened from the cog", () => { - it("scrolls to and focuses the Finish line settings", async () => { - const { Settings } = await import("./Settings"); - const scrolled = vi.fn(); - Element.prototype.scrollIntoView = scrolled; - const r = (root ??= createRoot(host)); - act(() => r.render( undefined} />)); - act(() => { - window.dispatchEvent( - new CustomEvent("pd2:open-settings", { detail: "finish-line" }) - ); - }); - const section = document.getElementById("finish-line"); - expect(scrolled.mock.contexts[0]).toBe(section); - expect(section?.contains(document.activeElement)).toBe(true); - expect(document.activeElement?.textContent).toBe("Pop up"); - }); +describe('Settings, opened from the cog', () => { + it('scrolls to and focuses the Finish line settings', async () => { + const { Settings } = await import('./Settings'); + const scrolled = vi.fn(); + Element.prototype.scrollIntoView = scrolled; + const r = (root ??= createRoot(host)); + act(() => r.render( undefined} />)); + act(() => { + window.dispatchEvent(new CustomEvent('pd2:open-settings', { detail: 'finish-line' })); + }); + const section = document.getElementById('finish-line'); + expect(scrolled.mock.contexts[0]).toBe(section); + expect(section?.contains(document.activeElement)).toBe(true); + expect(document.activeElement?.textContent).toBe('Pop up'); + }); }); diff --git a/frontend-v2/src/components/FinishLine.tsx b/frontend-v2/src/components/FinishLine.tsx index cb571a49..14b7a55b 100644 --- a/frontend-v2/src/components/FinishLine.tsx +++ b/frontend-v2/src/components/FinishLine.tsx @@ -1,29 +1,29 @@ -import { useEffect, useMemo, useRef, useState } from "react"; -import { createPortal } from "react-dom"; -import { Flag, Settings as SettingsIcon, X } from "lucide-react"; -import type { DerivedPull } from "../../../shared/model/status"; -import { githubUrl } from "../../../shared/format"; -import { askedOf, askForInputHint, rowNote, rowWord } from "../model/actions"; -import { STAGE_WORDS } from "../model/stage"; +import { useEffect, useMemo, useRef, useState } from 'react'; +import { createPortal } from 'react-dom'; +import { Flag, Settings as SettingsIcon, X } from 'lucide-react'; +import type { DerivedPull } from '../../../shared/model/status'; +import { githubUrl } from '../../../shared/format'; +import { askedOf, askForInputHint, rowNote, rowWord } from '../model/actions'; +import { STAGE_WORDS } from '../model/stage'; import { - finishLineCase, - finishDue, - finishGroups, - finishRecord, - finishStats, - parseFinishRecord, - prCount, - FINISH_LINE_KEY, - type FinishLineCase, - type FinishStat, -} from "../model/finishLine"; -import { useSettings } from "../settings"; -import { readStorage, writeStorage } from "../storage"; -import { FactLink, QuietButton, RepoRef } from "./bits"; -import { Icon } from "./Icon"; -import { modalOpen } from "../hooks"; -import { GroupHeader } from "./Lane"; -import { CiGlyph } from "./pips"; + finishLineCase, + finishDue, + finishGroups, + finishRecord, + finishStats, + parseFinishRecord, + prCount, + FINISH_LINE_KEY, + type FinishLineCase, + type FinishStat, +} from '../model/finishLine'; +import { useSettings } from '../settings'; +import { readStorage, writeStorage } from '../storage'; +import { FactLink, QuietButton, RepoRef } from './bits'; +import { Icon } from './Icon'; +import { modalOpen } from '../hooks'; +import { GroupHeader } from './Lane'; +import { CiGlyph } from './pips'; /** * Finish line: when your own open PRs pass a limit (model/finishLine.ts), a @@ -41,217 +41,207 @@ import { CiGlyph } from "./pips"; let pressing = false; let dragging = false; const pointerBusy = () => pressing || dragging; -if (typeof document !== "undefined") { - // the main button only: a right-click's context menu swallows its - // pointerup and would leave this stuck on - // a new press also means no drag is running, whatever became of the last - document.addEventListener( - "pointerdown", - (e) => { - pressing = e.button === 0; - dragging = false; - }, - true - ); - // a touch that turns into a scroll, or a press that turns into a native - // drag, ends with pointercancel; the drag is tracked on its own - for (const end of ["pointerup", "pointercancel", "contextmenu"] as const) - document.addEventListener(end, () => (pressing = false), true); - document.addEventListener("dragstart", () => (dragging = true), true); - // dragend goes to the dragged node, which may have left the page by then - // (a Roadmap plan moved to another lane), so the drop counts as the end too - for (const end of ["dragend", "drop"] as const) - document.addEventListener(end, () => (dragging = false), true); - window.addEventListener("blur", () => (pressing = dragging = false)); +if (typeof document !== 'undefined') { + // the main button only: a right-click's context menu swallows its + // pointerup and would leave this stuck on + // a new press also means no drag is running, whatever became of the last + document.addEventListener( + 'pointerdown', + e => { + pressing = e.button === 0; + dragging = false; + }, + true + ); + // a touch that turns into a scroll, or a press that turns into a native + // drag, ends with pointercancel; the drag is tracked on its own + for (const end of ['pointerup', 'pointercancel', 'contextmenu'] as const) + document.addEventListener(end, () => (pressing = false), true); + document.addEventListener('dragstart', () => (dragging = true), true); + // dragend goes to the dragged node, which may have left the page by then + // (a Roadmap plan moved to another lane), so the drop counts as the end too + for (const end of ['dragend', 'drop'] as const) + document.addEventListener(end, () => (dragging = false), true); + window.addEventListener('blur', () => (pressing = dragging = false)); } -const hasPreviewFlag = () => - new URLSearchParams(location.hash.slice(1)).get("finish") === "1"; +const hasPreviewFlag = () => new URLSearchParams(location.hash.slice(1)).get('finish') === '1'; /** Renders the header badge in place, and the cell over the page. */ export function FinishLine({ - pulls, - me, - extraBots, - initialized, + pulls, + me, + extraBots, + initialized, }: { - pulls: DerivedPull[]; - me: string; - extraBots: ReadonlySet; - initialized: boolean; + pulls: DerivedPull[]; + me: string; + extraBots: ReadonlySet; + initialized: boolean; }) { - const settings = useSettings(); - // read on the first render: the app rewrites the hash right after and - // drops the unknown param - const [preview, setPreview] = useState(hasPreviewFlag); - // which list is open, not a copy of it: the list below is read live, so a - // merge or a new PR shows up while it's open - const [shown, setShown] = useState<"real" | "preview" | null>(null); - // the last showing, kept here too so blocked storage still holds it for - // this page load; another tab's showing arrives by the storage event - const record = useRef(parseFinishRecord(readStorage(FINISH_LINE_KEY))); - - // where you stand whatever the mode, for the preview: with it off, a - // preview still shows your real numbers - const real = useMemo( - () => - // no case until we know who you are: an empty `me` (a failed whoami) - // would read as "under the limits" and wipe the last showing - initialized && me - ? finishLineCase(pulls, me, extraBots, { - maxOpen: settings.finishLineMaxOpen, - maxDays: settings.finishLineMaxDays, - countDrafts: settings.finishLineCountDrafts, - }) - : null, - [initialized, pulls, me, extraBots, settings] - ); - const found = settings.finishLineMode === "off" ? null : real; - - // under the limits again: zero what the last showing counted, so going - // over later is worse than it (a fresh offense), but keep when it showed, - // so it still comes back at most every 4 hours - useEffect(() => { - const last = record.current; - if ( - !initialized || - !me || - found || - settings.finishLineMode === "off" || - !last - ) - return; - if (!last.count && !last.over.length) return; - record.current = { at: last.at, count: 0, over: [] }; - writeStorage(FINISH_LINE_KEY, JSON.stringify(record.current)); - }, [initialized, me, found, settings.finishLineMode]); - - // the latest case, for closing: the button's hold started seconds ago, and - // what you saw last is what the next showing compares against - const latest = useRef(found); - latest.current = found; - - // open PRs on the whole board, so a row leaving the list only slides - // out as merged when it really closed (not when the rule switched) - const openIds = useMemo( - () => new Set(pulls.filter((p) => p.data.state === "open").map(pullId)), - [pulls] - ); - - const remember = (c: FinishLineCase) => { - record.current = finishRecord(c, Date.now()); - writeStorage(FINISH_LINE_KEY, JSON.stringify(record.current)); - }; - const show = (c: FinishLineCase) => { - remember(c); - setShown("real"); - }; - - useEffect(() => { - const onHash = () => { - if (hasPreviewFlag()) setPreview(true); - }; - // Settings' "Preview it": an event, since setting the same hash - // twice fires nothing - const onPreview = () => setPreview(true); - window.addEventListener("pd2:preview-finish-line", onPreview); - const onStorage = (e: StorageEvent) => { - if (e.key === FINISH_LINE_KEY) - record.current = parseFinishRecord(e.newValue); - }; - window.addEventListener("hashchange", onHash); - window.addEventListener("storage", onStorage); - return () => { - window.removeEventListener("hashchange", onHash); - window.removeEventListener("pd2:preview-finish-line", onPreview); - window.removeEventListener("storage", onStorage); - }; - }, []); - - // the preview never writes the record, so it can't stand in for a real one - useEffect(() => { - if (!initialized || !preview) return; - setPreview(false); - setShown("preview"); - }, [initialized, preview]); - - // checked on load, on every data change, and when the tab comes back; a - // hidden tab or a focused field waits for the next one, so it never lands - // mid-typing or out of sight - useEffect(() => { - if (!found || shown || settings.finishLineMode !== "auto") return; - const check = () => { - if (document.visibilityState === "hidden") return; - // another dialog is up (Settings), or a drag is under way: it - // would land on top of it, and swallow the Esc that cancels a drag - if (modalOpen() || pointerBusy()) return; - if (document.activeElement?.matches("input, textarea, select")) return; - // read the record fresh: a second visible window may have shown it - // a moment ago, before its storage event reached this one - record.current = - parseFinishRecord(readStorage(FINISH_LINE_KEY)) ?? record.current; - if (!finishDue(found, record.current, Date.now())) return; - show(found); - }; - check(); - document.addEventListener("visibilitychange", check); - window.addEventListener("focus", check); - return () => { - document.removeEventListener("visibilitychange", check); - window.removeEventListener("focus", check); - }; - }, [found, shown, settings.finishLineMode]); - - // you got under the limits while it was up: it says so, then lets you go - const freed = shown === "real" && !found; - useEffect(() => { - if (!freed) return; - const t = window.setTimeout(() => setShown(null), FREED_MS); - return () => clearTimeout(t); - }, [freed]); - - return ( - <> - {found && ( - // the action, said as a number and a verb: "2 to finish" - - )} - {shown && - createPortal( - { - // what you saw last is what the next showing compares - // against, so PRs opened while it was up don't re-drop it - if (shown === "real" && latest.current) remember(latest.current); - setShown(null); - }} - />, - document.body - )} - - ); + const settings = useSettings(); + // read on the first render: the app rewrites the hash right after and + // drops the unknown param + const [preview, setPreview] = useState(hasPreviewFlag); + // which list is open, not a copy of it: the list below is read live, so a + // merge or a new PR shows up while it's open + const [shown, setShown] = useState<'real' | 'preview' | null>(null); + // the last showing, kept here too so blocked storage still holds it for + // this page load; another tab's showing arrives by the storage event + const record = useRef(parseFinishRecord(readStorage(FINISH_LINE_KEY))); + + // where you stand whatever the mode, for the preview: with it off, a + // preview still shows your real numbers + const real = useMemo( + () => + // no case until we know who you are: an empty `me` (a failed whoami) + // would read as "under the limits" and wipe the last showing + initialized && me + ? finishLineCase(pulls, me, extraBots, { + maxOpen: settings.finishLineMaxOpen, + maxDays: settings.finishLineMaxDays, + countDrafts: settings.finishLineCountDrafts, + }) + : null, + [initialized, pulls, me, extraBots, settings] + ); + const found = settings.finishLineMode === 'off' ? null : real; + + // under the limits again: zero what the last showing counted, so going + // over later is worse than it (a fresh offense), but keep when it showed, + // so it still comes back at most every 4 hours + useEffect(() => { + const last = record.current; + if (!initialized || !me || found || settings.finishLineMode === 'off' || !last) return; + if (!last.count && !last.over.length) return; + record.current = { at: last.at, count: 0, over: [] }; + writeStorage(FINISH_LINE_KEY, JSON.stringify(record.current)); + }, [initialized, me, found, settings.finishLineMode]); + + // the latest case, for closing: the button's hold started seconds ago, and + // what you saw last is what the next showing compares against + const latest = useRef(found); + latest.current = found; + + // open PRs on the whole board, so a row leaving the list only slides + // out as merged when it really closed (not when the rule switched) + const openIds = useMemo( + () => new Set(pulls.filter(p => p.data.state === 'open').map(pullId)), + [pulls] + ); + + const remember = (c: FinishLineCase) => { + record.current = finishRecord(c, Date.now()); + writeStorage(FINISH_LINE_KEY, JSON.stringify(record.current)); + }; + const show = (c: FinishLineCase) => { + remember(c); + setShown('real'); + }; + + useEffect(() => { + const onHash = () => { + if (hasPreviewFlag()) setPreview(true); + }; + // Settings' "Preview it": an event, since setting the same hash + // twice fires nothing + const onPreview = () => setPreview(true); + window.addEventListener('pd2:preview-finish-line', onPreview); + const onStorage = (e: StorageEvent) => { + if (e.key === FINISH_LINE_KEY) record.current = parseFinishRecord(e.newValue); + }; + window.addEventListener('hashchange', onHash); + window.addEventListener('storage', onStorage); + return () => { + window.removeEventListener('hashchange', onHash); + window.removeEventListener('pd2:preview-finish-line', onPreview); + window.removeEventListener('storage', onStorage); + }; + }, []); + + // the preview never writes the record, so it can't stand in for a real one + useEffect(() => { + if (!initialized || !preview) return; + setPreview(false); + setShown('preview'); + }, [initialized, preview]); + + // checked on load, on every data change, and when the tab comes back; a + // hidden tab or a focused field waits for the next one, so it never lands + // mid-typing or out of sight + useEffect(() => { + if (!found || shown || settings.finishLineMode !== 'auto') return; + const check = () => { + if (document.visibilityState === 'hidden') return; + // another dialog is up (Settings), or a drag is under way: it + // would land on top of it, and swallow the Esc that cancels a drag + if (modalOpen() || pointerBusy()) return; + if (document.activeElement?.matches('input, textarea, select')) return; + // read the record fresh: a second visible window may have shown it + // a moment ago, before its storage event reached this one + record.current = parseFinishRecord(readStorage(FINISH_LINE_KEY)) ?? record.current; + if (!finishDue(found, record.current, Date.now())) return; + show(found); + }; + check(); + document.addEventListener('visibilitychange', check); + window.addEventListener('focus', check); + return () => { + document.removeEventListener('visibilitychange', check); + window.removeEventListener('focus', check); + }; + }, [found, shown, settings.finishLineMode]); + + // you got under the limits while it was up: it says so, then lets you go + const freed = shown === 'real' && !found; + useEffect(() => { + if (!freed) return; + const t = window.setTimeout(() => setShown(null), FREED_MS); + return () => clearTimeout(t); + }, [freed]); + + return ( + <> + {found && ( + // the action, said as a number and a verb: "2 to finish" + + )} + {shown && + createPortal( + { + // what you saw last is what the next showing compares + // against, so PRs opened while it was up don't re-drop it + if (shown === 'real' && latest.current) remember(latest.current); + setShown(null); + }} + />, + document.body + )} + + ); } /** How long "All finished" shows before it closes on its own. */ @@ -262,38 +252,38 @@ const finishLabel = (n: number) => `Finish ${n} of your open PRs`; /** Merged or closed your way under the limits while it was up. */ const FREE: FinishLineCase = { - pulls: [], - ready: 0, - count: 0, - over: [], - toGo: 0, + pulls: [], + ready: 0, + count: 0, + over: [], + toGo: 0, }; /** The preview when you're under the limits: your PRs as it counts them (any * open one trips a limit of 0), or the board's oldest when you have none, * so there's something to look at. Nothing to finish: toGo is 0. */ function sampleCase( - pulls: DerivedPull[], - me: string, - extraBots: ReadonlySet, - settings: { finishLineMaxDays: number; finishLineCountDrafts: boolean } + pulls: DerivedPull[], + me: string, + extraBots: ReadonlySet, + settings: { finishLineMaxDays: number; finishLineCountDrafts: boolean } ): FinishLineCase { - const mine = finishLineCase(pulls, me, extraBots, { - maxOpen: 0, - maxDays: settings.finishLineMaxDays, - countDrafts: settings.finishLineCountDrafts, - }); - if (mine) return { ...mine, toGo: 0 }; - return { - pulls: pulls - .filter((p) => p.data.state === "open") - .sort((a, b) => b.ageDays - a.ageDays) - .slice(0, 6), - ready: 0, - count: 0, - over: [], - toGo: 0, - }; + const mine = finishLineCase(pulls, me, extraBots, { + maxOpen: 0, + maxDays: settings.finishLineMaxDays, + countDrafts: settings.finishLineCountDrafts, + }); + if (mine) return { ...mine, toGo: 0 }; + return { + pulls: pulls + .filter(p => p.data.state === 'open') + .sort((a, b) => b.ageDays - a.ageDays) + .slice(0, 6), + ready: 0, + count: 0, + over: [], + toGo: 0, + }; } /** How long "Hold to close" has to be held down. */ @@ -304,7 +294,7 @@ const OPEN_MS = 700; const LEAVE_MS = 450; const iconButton = - "pressable inline-flex h-8 w-8 items-center justify-center rounded-lg text-ink-3 hover:bg-muted hover:text-ink"; + 'pressable inline-flex h-8 w-8 items-center justify-center rounded-lg text-ink-3 hover:bg-muted hover:text-ink'; /** The modal over the board: a centered card on a desktop, a sheet up from * the bottom on a phone. Its top is the action and its numbers ("Finish 2 @@ -314,406 +304,380 @@ const iconButton = * for a couple of seconds, by pointer, finger, Enter or Space. The cog opens * Settings at this feature's own settings. */ function FinishModal({ - found, - me, - maxOpen, - maxDays, - hold, - openIds, - onDone, + found, + me, + maxOpen, + maxDays, + hold, + openIds, + onDone, }: { - found: FinishLineCase; - me: string; - maxOpen: number; - maxDays: number; - hold: boolean; - openIds: ReadonlySet; - onDone: () => void; + found: FinishLineCase; + me: string; + maxOpen: number; + maxDays: number; + hold: boolean; + openIds: ReadonlySet; + onDone: () => void; }) { - const rootRef = useRef(null); - const free = found === FREE; - const stats = finishStats(found, { maxOpen, maxDays }); - // a stat jumps to the first PR it counts, in the list's order - const goTo = (stat: FinishStat) => { - const ids = new Set(found.pulls.filter(stat.counts).map(pullId)); - const row = [ - ...(rootRef.current?.querySelectorAll("li[data-pull]") ?? - []), - ].find((li) => ids.has(li.dataset.pull ?? "")); - row?.scrollIntoView?.({ block: "nearest" }); - row?.querySelector("a[href]")?.focus({ preventScroll: true }); - }; - // the key handler is set up once; it reads these fresh - const latest = useRef({ hold, onDone }); - latest.current = { hold, onDone }; - - // "You're at the finish line" unmounts the focused button: keep focus in - // the dialog - useEffect(() => { - if (free) rootRef.current?.focus({ preventScroll: true }); - }, [free]); - - useEffect(() => { - const root = rootRef.current; - // focus goes back where it was (the header flag, say) when it closes - const before = document.activeElement as HTMLElement | null; - // focus starts on the hold button, so holding Enter or Space works - // right away; with no hold, on the dialog itself, so a stray Enter - // can't close it before it's read - const start = root?.querySelector("[data-hold]"); - (start ?? root)?.focus({ preventScroll: true }); - const onKey = (e: KeyboardEvent) => { - // Esc closes it (unless you asked to hold), and must not reach a - // dialog behind this one (Settings) either: listening in the - // capture phase runs first - if (e.key === "Escape") { - e.preventDefault(); - e.stopPropagation(); - if (!latest.current.hold) latest.current.onDone(); - } - if (e.key !== "Tab" || !root) return; - // only what Tab can reach: not a row sliding out, nor anything - // hidden or not rendered, or the wrap below never sees the real last - const focusables = [ - ...root.querySelectorAll("a[href], button"), - ].filter( - (e) => - !e.closest('[hidden], [aria-hidden="true"]') && - (e.checkVisibility?.() ?? true) + const rootRef = useRef(null); + const free = found === FREE; + const stats = finishStats(found, { maxOpen, maxDays }); + // a stat jumps to the first PR it counts, in the list's order + const goTo = (stat: FinishStat) => { + const ids = new Set(found.pulls.filter(stat.counts).map(pullId)); + const row = [...(rootRef.current?.querySelectorAll('li[data-pull]') ?? [])].find( + li => ids.has(li.dataset.pull ?? '') ); - if (!focusables.length) return; - const first = focusables[0]; - const last = focusables[focusables.length - 1]; - if (e.shiftKey && document.activeElement === first) { - e.preventDefault(); - last.focus(); - } else if (!e.shiftKey && document.activeElement === last) { - e.preventDefault(); - first.focus(); - } else if (!root.contains(document.activeElement)) { - e.preventDefault(); - first.focus(); - } - }; - document.addEventListener("keydown", onKey, true); - const prevOverflow = document.body.style.overflow; - document.body.style.overflow = "hidden"; - return () => { - document.removeEventListener("keydown", onKey, true); - document.body.style.overflow = prevOverflow; - before?.focus?.({ preventScroll: true }); - }; - }, []); - - return ( -

    -
    -
    - {free && } -
    - {/* the action with its real number, ticking down live as + row?.scrollIntoView?.({ block: 'nearest' }); + row?.querySelector('a[href]')?.focus({ preventScroll: true }); + }; + // the key handler is set up once; it reads these fresh + const latest = useRef({ hold, onDone }); + latest.current = { hold, onDone }; + + // "You're at the finish line" unmounts the focused button: keep focus in + // the dialog + useEffect(() => { + if (free) rootRef.current?.focus({ preventScroll: true }); + }, [free]); + + useEffect(() => { + const root = rootRef.current; + // focus goes back where it was (the header flag, say) when it closes + const before = document.activeElement as HTMLElement | null; + // focus starts on the hold button, so holding Enter or Space works + // right away; with no hold, on the dialog itself, so a stray Enter + // can't close it before it's read + const start = root?.querySelector('[data-hold]'); + (start ?? root)?.focus({ preventScroll: true }); + const onKey = (e: KeyboardEvent) => { + // Esc closes it (unless you asked to hold), and must not reach a + // dialog behind this one (Settings) either: listening in the + // capture phase runs first + if (e.key === 'Escape') { + e.preventDefault(); + e.stopPropagation(); + if (!latest.current.hold) latest.current.onDone(); + } + if (e.key !== 'Tab' || !root) return; + // only what Tab can reach: not a row sliding out, nor anything + // hidden or not rendered, or the wrap below never sees the real last + const focusables = [...root.querySelectorAll('a[href], button')].filter( + el => !el.closest('[hidden], [aria-hidden="true"]') && (el.checkVisibility?.() ?? true) + ); + if (!focusables.length) return; + const first = focusables[0]; + const last = focusables[focusables.length - 1]; + // from the dialog itself (where it opens), Tab goes in at either end + if (document.activeElement === root) { + e.preventDefault(); + (e.shiftKey ? last : first).focus(); + } else if (e.shiftKey && document.activeElement === first) { + e.preventDefault(); + last.focus(); + } else if (!e.shiftKey && document.activeElement === last) { + e.preventDefault(); + first.focus(); + } else if (!root.contains(document.activeElement)) { + e.preventDefault(); + first.focus(); + } + }; + document.addEventListener('keydown', onKey, true); + const prevOverflow = document.body.style.overflow; + document.body.style.overflow = 'hidden'; + return () => { + document.removeEventListener('keydown', onKey, true); + document.body.style.overflow = prevOverflow; + before?.focus?.({ preventScroll: true }); + }; + }, []); + + return ( +
    +
    +
    + {free && } +
    + {/* the action with its real number, ticking down live as you merge or close */} -

    - {free - ? "All finished" - : found.toGo - ? `Finish ${prCount(found.toGo)}` - : "Nothing to finish"} -

    - {free && ( -

    - None of your open PRs are past your limits. -

    - )} - {/* why, in numbers you can act on: each one jumps to its PRs */} - {!free && stats.length > 0 && ( -

    - {stats.map((stat, i) => ( - - {i > 0 && · } - goTo(stat)}>{stat.text} - - ))} -

    - )} -
    -
    - - {!hold && ( - - )} -
    -
    - {!free && ( - <> - {/* the board's own sticky group headers, pinned to the top +

    + {free + ? 'All finished' + : found.toGo + ? `Finish ${prCount(found.toGo)}` + : 'Nothing to finish'} +

    + {free && ( +

    + None of your open PRs are past your limits. +

    + )} + {/* why, in numbers you can act on: each one jumps to its PRs */} + {!free && stats.length > 0 && ( +

    + {/* spaced, not dotted, so a wrap never leaves a dot hanging */} + {stats.map(stat => ( + goTo(stat)}> + {stat.text} + + ))} +

    + )} +
    +
    + + {!hold && ( + + )} +
    +
    + {!free && ( + <> + {/* the board's own sticky group headers, pinned to the top of this list on the card's color instead of under the app header on the canvas */} -
    - -
    -

    Only you see this.

    - {hold && } - - )} -
    -
    - ); +
    + +
    +

    Only you see this.

    + {hold && } + + )} +
    + + ); } /** The hold button: hold it down for HOLD_MS, by pointer, finger, Enter or * Space. The ring fills as you hold; letting go early springs it back, and a * quick click says to hold it. At the end it closes. */ function HoldToClose({ onDone }: { onDone: () => void }) { - const [holding, setHolding] = useState(false); - const [opened, setOpened] = useState(false); - // a click too short to be a hold: the label teaches the gesture - const [tapped, setTapped] = useState(false); - const timer = useRef(undefined); - const tapTimer = useRef(undefined); - const since = useRef(0); - // when a key was last let go: Firefox clicks a button on Space's keyup, - // and that click must not pass for assistive tech's lone click - const keyUpAt = useRef(0); - const start = () => { - if (timer.current !== undefined || opened) return; - since.current = Date.now(); - setHolding(true); - setTapped(false); - timer.current = window.setTimeout(() => { - setOpened(true); - timer.current = window.setTimeout(onDone, OPEN_MS); - }, HOLD_MS); - }; - const stop = () => { - if (opened || timer.current === undefined) return; - clearTimeout(timer.current); - timer.current = undefined; - setHolding(false); - if (Date.now() - since.current < 500) { - setTapped(true); - clearTimeout(tapTimer.current); - tapTimer.current = window.setTimeout(() => setTapped(false), 2000); - } - }; - useEffect( - () => () => { + const [holding, setHolding] = useState(false); + const [opened, setOpened] = useState(false); + // a click too short to be a hold: the label teaches the gesture + const [tapped, setTapped] = useState(false); + const timer = useRef(undefined); + const tapTimer = useRef(undefined); + const since = useRef(0); + // when a key was last let go: Firefox clicks a button on Space's keyup, + // and that click must not pass for assistive tech's lone click + const keyUpAt = useRef(0); + const start = () => { + if (timer.current !== undefined || opened) return; + since.current = Date.now(); + setHolding(true); + setTapped(false); + timer.current = window.setTimeout(() => { + setOpened(true); + timer.current = window.setTimeout(onDone, OPEN_MS); + }, HOLD_MS); + }; + const stop = () => { + if (opened || timer.current === undefined) return; clearTimeout(timer.current); - clearTimeout(tapTimer.current); - }, - [] - ); - const isPress = (key: string) => key === "Enter" || key === " "; - // the ring fills while held, and snaps back when let go - const sweep = holding ? `${HOLD_MS}ms linear` : "150ms ease-out"; - return ( - - ); + timer.current = undefined; + setHolding(false); + if (Date.now() - since.current < 500) { + setTapped(true); + clearTimeout(tapTimer.current); + tapTimer.current = window.setTimeout(() => setTapped(false), 2000); + } + }; + useEffect( + () => () => { + clearTimeout(timer.current); + clearTimeout(tapTimer.current); + }, + [] + ); + const isPress = (key: string) => key === 'Enter' || key === ' '; + // the ring fills while held, and snaps back when let go + const sweep = holding ? `${HOLD_MS}ms linear` : '150ms ease-out'; + return ( + + ); } const pullId = (p: DerivedPull) => `${p.data.repo}#${p.data.number}`; /** The PRs that just left `pulls` (merged or closed), kept for LEAVE_MS so * their rows can slide out instead of vanishing. */ -function useLeaving( - pulls: DerivedPull[], - openIds: ReadonlySet -): DerivedPull[] { - const last = useRef(new Map()); - const [leaving, setLeaving] = useState([]); - const timers = useRef([]); - useEffect(() => { - const now = new Map(pulls.map((p) => [pullId(p), p])); - // gone from the list and closed on the board; one that only stopped - // counting (the rule switched, or it went draft) just leaves - const gone = [...last.current] - .filter(([id]) => !now.has(id) && !openIds.has(id)) - .map(([, p]) => p); - last.current = now; - if (!gone.length) return; - setLeaving((l) => [...l, ...gone]); - timers.current.push( - window.setTimeout( - () => setLeaving((l) => l.filter((p) => !gone.includes(p))), - LEAVE_MS - ) - ); - // keyed on pulls alone: openIds changes with every board update, and - // it's only read to tell a close from a rule switch - }, [pulls]); - useEffect(() => () => timers.current.forEach(clearTimeout), []); - // one that came back (reopened) is just a row again - return leaving.filter((p) => !pulls.some((q) => pullId(q) === pullId(p))); +function useLeaving(pulls: DerivedPull[], openIds: ReadonlySet): DerivedPull[] { + const last = useRef(new Map()); + const [leaving, setLeaving] = useState([]); + const timers = useRef([]); + useEffect(() => { + const now = new Map(pulls.map(p => [pullId(p), p])); + // gone from the list and closed on the board; one that only stopped + // counting (the rule switched, or it went draft) just leaves + const gone = [...last.current] + .filter(([id]) => !now.has(id) && !openIds.has(id)) + .map(([, p]) => p); + last.current = now; + if (!gone.length) return; + setLeaving(l => [...l, ...gone]); + timers.current.push( + window.setTimeout(() => setLeaving(l => l.filter(p => !gone.includes(p))), LEAVE_MS) + ); + // keyed on pulls alone: openIds changes with every board update, and + // it's only read to tell a close from a rule switch + }, [pulls]); + useEffect(() => () => timers.current.forEach(clearTimeout), []); + // one that came back (reopened) is just a row again + return leaving.filter(p => !pulls.some(q => pullId(q) === pullId(p))); } /** Slack-ready text chasing the reviews you asked for: each title, its * link, and who was asked. Only PRs with a request: a self-reviewed PR waits * on nobody, so there's no one to nudge about it. */ function nudgeText(rows: { pull: DerivedPull }[]): string { - return [ - "A few of my PRs are waiting on a review I asked for:", - ...rows.map( - ({ pull: p }) => - `• ${p.data.title} ${githubUrl(p.data.repo, p.data.number)} (${askedOf( - p - ).join(", ")})` - ), - ].join("\n"); + return [ + 'A few of my PRs are waiting on a review I asked for:', + ...rows.map( + ({ pull: p }) => + `• ${p.data.title} ${githubUrl(p.data.repo, p.data.number)} (${askedOf(p).join(', ')})` + ), + ].join('\n'); } /** Copies nudgeText, and says whether it worked. */ function CopyNudge({ rows }: { rows: { pull: DerivedPull }[] }) { - const [copied, setCopied] = useState<"copied" | "failed" | null>(null); - const done = (result: "copied" | "failed") => { - setCopied(result); - window.setTimeout(() => setCopied(null), 3000); - }; - return ( - { - if (!navigator.clipboard) return done("failed"); - navigator.clipboard.writeText(nudgeText(rows)).then( - () => done("copied"), - () => done("failed") - ); - }} - > - {copied === "copied" - ? "Copied" - : copied === "failed" - ? "Couldn’t copy" - : "Copy a nudge"} - - ); + const [copied, setCopied] = useState<'copied' | 'failed' | null>(null); + const done = (result: 'copied' | 'failed') => { + setCopied(result); + window.setTimeout(() => setCopied(null), 3000); + }; + return ( + { + if (!navigator.clipboard) return done('failed'); + navigator.clipboard.writeText(nudgeText(rows)).then( + () => done('copied'), + () => done('failed') + ); + }} + > + {copied === 'copied' ? 'Copied' : copied === 'failed' ? 'Couldn’t copy' : 'Copy a nudge'} + + ); } /** Your PRs, the way My work groups them, with the ready ones on top: the @@ -721,238 +685,222 @@ function CopyNudge({ rows }: { rows: { pull: DerivedPull }[] }) { * word on what to do, oldest first. Someone else's PRs (the preview's * fallback) list flat, since their groups would be a reviewer's. */ function FinishList({ - pulls, - me, - maxDays, - openIds, + pulls, + me, + maxDays, + openIds, }: { - pulls: DerivedPull[]; - me: string; - maxDays: number; - openIds: ReadonlySet; + pulls: DerivedPull[]; + me: string; + maxDays: number; + openIds: ReadonlySet; }) { - const leaving = useLeaving(pulls, openIds); - const gone = new Set(leaving.map(pullId)); - const all = [...pulls, ...leaving]; - const mine = all.every((p) => p.data.user.login === me); - const groups = mine - ? finishGroups(all, me) - : { ready: [], move: [], waiting: all.map((pull) => ({ pull })) }; - const live = (rows: { pull: DerivedPull }[]) => - rows.filter((r) => !gone.has(pullId(r.pull))); - // the PRs waiting on people you asked, wherever they sit (a request gone - // quiet past 4 hours is Your move); the copy button rides on the first - // section holding one - const asked = (rows: { pull: DerivedPull }[]) => - live(rows).filter((r) => askedOf(r.pull).length); - const nudgeRows = asked([...groups.move, ...groups.waiting]); - const sections = [ - { - key: "ready" as const, - title: STAGE_WORDS.ready, - sub: "Merge these first", - rows: groups.ready, - ready: true, - }, - { - key: "move" as const, - title: "Your move", - sub: "Do the next step on each", - rows: groups.move, - }, - { - key: "waiting" as const, - title: "Waiting on others", - sub: "Nudge whoever you asked, or close what you don’t need", - rows: groups.waiting, - }, - ].filter((g) => g.rows.length); - const nudgeOn = sections.find((g) => !g.ready && asked(g.rows).length)?.title; - return ( -
    - {sections.map((g) => ( -
    - {mine && ( - : undefined - } - /> - )} -
      - {g.rows.map(({ pull }) => ( - - ))} -
    -
    - ))} -
    - ); + const leaving = useLeaving(pulls, openIds); + const gone = new Set(leaving.map(pullId)); + const all = [...pulls, ...leaving]; + const mine = all.every(p => p.data.user.login === me); + const groups = mine + ? finishGroups(all, me) + : { ready: [], move: [], waiting: all.map(pull => ({ pull })) }; + const live = (rows: { pull: DerivedPull }[]) => rows.filter(r => !gone.has(pullId(r.pull))); + // the PRs waiting on people you asked, wherever they sit (a request gone + // quiet past 4 hours is Your move); the copy button rides on the first + // section holding one + const asked = (rows: { pull: DerivedPull }[]) => live(rows).filter(r => askedOf(r.pull).length); + const nudgeRows = asked([...groups.move, ...groups.waiting]); + const sections = [ + { + key: 'ready' as const, + title: STAGE_WORDS.ready, + sub: 'Merge these first', + rows: groups.ready, + ready: true, + }, + { + key: 'move' as const, + title: 'Your move', + sub: 'Do the next step on each', + rows: groups.move, + }, + { + key: 'waiting' as const, + title: 'Waiting on others', + sub: 'Nudge whoever you asked, or close what you don’t need', + rows: groups.waiting, + }, + ].filter(g => g.rows.length); + const nudgeOn = sections.find(g => !g.ready && asked(g.rows).length)?.title; + return ( +
    + {sections.map(g => ( +
    + {mine && ( + : undefined} + /> + )} +
      + {g.rows.map(({ pull }) => ( + + ))} +
    +
    + ))} +
    + ); } /** One PR: the title to GitHub and its age, then the move and what it's * waiting on. */ function FinishRow({ - pull: p, - me, - maxDays, - ready, - mine, - leaving, + pull: p, + me, + maxDays, + ready, + mine, + leaving, }: { - pull: DerivedPull; - me: string; - maxDays: number; - ready: boolean; - mine: boolean; - /** merged or closed: it slides out */ - leaving?: boolean; + pull: DerivedPull; + me: string; + maxDays: number; + ready: boolean; + mine: boolean; + /** merged or closed: it slides out */ + leaving?: boolean; }) { - const word = mine && !ready ? rowWord(p, me) : null; - const context = mine ? rowNote(p, me).context : null; - // a do row names the move, then the detail; a wait row's detail already - // says who it waits on ("waiting on bob to re-stamp"), so the word only - // stands in when there's no detail - const move = word?.kind === "do" ? word.word : null; - const detail = context ?? (word?.kind === "wait" ? word.word : null); - const hint = mine && !ready ? askForInputHint(p) : null; - const old = p.ageDays > maxDays; - return ( -
  • - - - {p.data.title} ↗ - - {/* quiet even past the limit: the stat above already says how many - are, and amber on every row would stop meaning "look here" */} - - {p.ageDays ? `${p.ageDays} days` : "today"} - - - - - {move && ( - - {move} - - )} - {!ready && ( - <> - - {/* the glyph is aria-hidden; say what it shows */} - {p.ci === "failing" && CI failing} - {p.ci === "pending" && CI running} - - )} - {detail && {detail}} - - {hint && {hint}} -
    • - ); + const word = mine && !ready ? rowWord(p, me) : null; + const context = mine ? rowNote(p, me).context : null; + // a do row names the move, then the detail; a wait row's detail already + // says who it waits on ("waiting on bob to re-stamp"), so the word only + // stands in when there's no detail + const move = word?.kind === 'do' ? word.word : null; + const detail = context ?? (word?.kind === 'wait' ? word.word : null); + const hint = mine && !ready ? askForInputHint(p) : null; + const old = p.ageDays > maxDays; + return ( +
  • + + + {p.data.title} ↗ + + {/* no amber past the limit: the stat above already says how many + are, and amber on every row would stop meaning "look here"; + weight alone sets the old ones apart */} + + {p.ageDays ? `${p.ageDays} days` : 'today'} + + + + + {move && ( + + {move} + + )} + {!ready && ( + <> + + {/* the glyph is aria-hidden; say what it shows */} + {p.ci === 'failing' && CI failing} + {p.ci === 'pending' && CI running} + + )} + {detail && {detail}} + + {hint && {hint}} +
    • + ); } /** A robot cheering at a checkered finish flag, drawn in the board's own * line weights and color tokens so it themes with everything else. */ function FinishMark() { - return ( - - {/* the finish line on the ground */} - - {/* the flag: a pole and a checkered cloth */} - - - - {/* an upright antenna with a lit tip */} - - - {/* head, and a happy face on its screen */} - - - - {/* body, one arm up at the flag, one down */} - - - - - ); + return ( + + {/* the finish line on the ground */} + + {/* the flag: a pole and a checkered cloth */} + + + + {/* an upright antenna with a lit tip */} + + + {/* head, and a happy face on its screen */} + + + + {/* body, one arm up at the flag, one down */} + + + + + ); } diff --git a/frontend-v2/src/components/Legend.tsx b/frontend-v2/src/components/Legend.tsx index 924507b9..77f36e8e 100644 --- a/frontend-v2/src/components/Legend.tsx +++ b/frontend-v2/src/components/Legend.tsx @@ -1,17 +1,17 @@ -import type { ReactNode } from "react"; -import { CircleHelp, Flag, Heart, Star } from "lucide-react"; -import { HeaderIconButton } from "./bits"; -import { Icon } from "./Icon"; -import { Popover } from "./Popover"; -import { eyebrowText } from "./WordGroups"; +import type { ReactNode } from 'react'; +import { CircleHelp, Flag, Heart, Star } from 'lucide-react'; +import { HeaderIconButton } from './bits'; +import { Icon } from './Icon'; +import { Popover } from './Popover'; +import { eyebrowText } from './WordGroups'; /** a keycap, sized to the legend's small type */ function Kbd({ children }: { children: ReactNode }) { - return ( - - {children} - - ); + return ( + + {children} + + ); } /** @@ -20,24 +20,24 @@ function Kbd({ children }: { children: ReactNode }) { * line inside the column instead of overflowing into the definition. */ function Item({ term, def }: { term: ReactNode; def: ReactNode }) { - return ( -
    - - {term} - - {def} -
    - ); + return ( +
    + + {term} + + {def} +
    + ); } /** a labeled band of the key; every band after the first draws a hairline */ function Group({ title, children }: { title: string; children: ReactNode }) { - return ( -
    -
    {title}
    - {children} -
    - ); + return ( +
    +
    {title}
    + {children} +
    + ); } /** @@ -50,211 +50,187 @@ function Group({ title, children }: { title: string; children: ReactNode }) { * tokens, and the keyboard. One screen, no folds, no scrolling essays. */ export function Legend() { - return ( - ( - - )} - > - - Reading the board - -

    - Everything explains itself where it sits: hover any mark, flag, header, - or count. These are the conventions that don’t announce themselves. -

    + return ( + } + > + + Reading the board + +

    + Everything explains itself where it sits: hover any mark, flag, header, or count. These + are the conventions that don’t announce themselves. +

    - - stamp} - def="a CR or QA sign-off: a GitHub approval, or a “CR 👍” / “QA 👍” comment. A push undoes it until it’s re-confirmed" - /> - - - - - - } - def="one circle per required stamp, in order: stands, stale review, still needed. A dotted underline marks yours; click them for who signed. While a PR is a draft, blocked, or red, the re-ask waits: the author moves first" - /> - - CI - - - - } - def="CI is one circle, the machine’s mark: the red ring with a center dot, a check failed; the ring sweeping shut is a run in progress, the filled share done. On row hover a green ring closes the loop: passed. No news is good news" - /> - + + stamp} + def="a CR or QA sign-off: a GitHub approval, or a “CR 👍” / “QA 👍” comment. A push undoes it until it’s re-confirmed" + /> + + + + + + } + def="one circle per required stamp, in order: stands, stale review, still needed. A dotted underline marks yours; click them for who signed. While a PR is a draft, blocked, or red, the re-ask waits: the author moves first" + /> + + CI + + + + } + def="CI is one circle, the machine’s mark: the red ring with a center dot, a check failed; the ring sweeping shut is a run in progress, the filled share done. On row hover a green ring closes the loop: passed. No news is good news" + /> + - - - - - - } - def="a circle avatar is a person; a square tile is a bot or app" - /> - - - - - } - def="a corner heart marks someone on your team; the star seated on your own avatar’s rim is you" - /> - + + + + + + } + def="a circle avatar is a person; a square tile is a bot or app" + /> + + + + + } + def="a corner heart marks someone on your team; the star seated on your own avatar’s rim is you" + /> + - - - Re-stamp · 3 - - } - def="brand is your move: a brand header names an action you owe, a gray one says why a card waits. Hover the word for its meaning" - /> - - conflicts - stacked - - } - def="amber flag: act on it. Gray flag: a plain fact" - /> - + + + Re-stamp · 3 + + } + def="brand is your move: a brand header names an action you owe, a gray one says why a card waits. Hover the word for its meaning" + /> + + conflicts + stacked + + } + def="amber flag: act on it. Gray flag: a plain fact" + /> + - - self-review} - def="a developer reviews and tests their own PR unless they ask for a review. Nobody else owes it anything" - /> - review requested} - def="the author asked people (or a team) for a review on GitHub. It leads their Waiting on you; answer within hours" - /> - I’ll review - } - def="hover a row for its verbs. It adds you as a reviewer on the PR, so GitHub and the board both show you’re on it; the row moves to Waiting on you until you submit, drop it, or are removed" - /> - + + self-review} + def="a developer reviews and tests their own PR unless they ask for a review. Nobody else owes it anything" + /> + review requested} + def="the author asked people (or a team) for a review on GitHub. It leads their Waiting on you; answer within hours" + /> + I’ll review} + def="hover a row for its verbs. It adds you as a reviewer on the PR, so GitHub and the board both show you’re on it; the row moves to Waiting on you until you submit, drop it, or are removed" + /> + - - - - - 2 to finish - - - } - def="shows in the header when your own open PRs pass your limits (by default more than 7 open, or one open over 14 days): how many to merge or close. Click it for the list, ready ones first. Only you see it; nobody else is told" - /> - Settings} - def="under Finish line, or the cog in the list: Pop up opens the list by itself when you go over (off by default), Header count only waits for a click, Off hides it. Esc or ✕ closes it, unless you’ve chosen to hold to close" - /> - + + + + + 2 to finish + + + } + def="shows in the header when your own open PRs pass your limits (by default more than 7 open, or one open over 14 days): how many to merge or close. Click it for the list, ready ones first. Only you see it; nobody else is told" + /> + Settings} + def="under Finish line, or the cog in the list: Pop up opens the list by itself when you go over (off by default), Header count only waits for a click, Off hides it. Esc or ✕ closes it, unless you’ve chosen to hold to close" + /> + - - - weight:xs,s - - } - def="review-effort classes; a row shows its class as a letter beside CR, detailed in the CR popover" - /> - has:action - } - def="cards where the next move is yours" - /> - is:restamp - } - def="a push undid your stamp and the PR is reviewable again" - /> - is:asked - } - def="someone asked you for a review" - /> - is:blocked - } - def="under a dev or deploy block, whoever holds it" - /> - is:draft - } - def="still a draft" - /> - is:mine - } - def="opened by you" - /> - + + weight:xs,s} + def="review-effort classes; a row shows its class as a letter beside CR, detailed in the CR popover" + /> + has:action} + def="cards where the next move is yours" + /> + is:restamp} + def="a push undid your stamp and the PR is reviewable again" + /> + is:asked} + def="someone asked you for a review" + /> + is:blocked} + def="under a dev or deploy block, whoever holds it" + /> + is:draft} + def="still a draft" + /> + is:mine} + def="opened by you" + /> + - - /} - def="focus the filter; on Projects, the view’s own find box" - /> - - j - k - - } - def="walk down / up the rows" - /> - ↵} - def="open the selected PR or project; on Decide, make the call" - /> - c} def="copy the selected branch name" /> - a} def="on a project’s page, add an issue" /> - u} def="on a project’s page, post an update" /> - -     - ); + + /} + def="focus the filter; on Projects, the view’s own find box" + /> + + j + k + + } + def="walk down / up the rows" + /> + ↵} + def="open the selected PR or project; on Decide, make the call" + /> + c} def="copy the selected branch name" /> + a} def="on a project’s page, add an issue" /> + u} def="on a project’s page, post an update" /> + +     + ); } diff --git a/frontend-v2/src/components/Settings.tsx b/frontend-v2/src/components/Settings.tsx index 1365d077..23193859 100644 --- a/frontend-v2/src/components/Settings.tsx +++ b/frontend-v2/src/components/Settings.tsx @@ -1,603 +1,571 @@ -import { useEffect, useRef, useState } from "react"; -import { createPortal } from "react-dom"; -import { Settings as SettingsIcon, Trash2, X } from "lucide-react"; -import { clearStoredPrefs } from "../storage"; -import { refreshAll, refreshAllText, usePulldasher } from "../store"; +import { useEffect, useRef, useState } from 'react'; +import { createPortal } from 'react-dom'; +import { Settings as SettingsIcon, Trash2, X } from 'lucide-react'; +import { clearStoredPrefs } from '../storage'; +import { refreshAll, refreshAllText, usePulldasher } from '../store'; import { - addCodeRegion, - removeCodeRegion, - setLaneCapForLens, - type Settings as SettingsShape, - setSettings, - useSettings, -} from "../settings"; -import { LENS_LABELS, type Lens } from "../lens"; -import { HeaderIconButton, QuietButton, Segmented } from "./bits"; -import { Icon } from "./Icon"; -import { Explainer, Field, Group, NumberField } from "./SettingsBits"; -import { useArmedConfirm } from "./useArmedConfirm"; -import { commitKeyHandler } from "./useCommitOnEnter"; + addCodeRegion, + removeCodeRegion, + setLaneCapForLens, + type Settings as SettingsShape, + setSettings, + useSettings, +} from '../settings'; +import { LENS_LABELS, type Lens } from '../lens'; +import { HeaderIconButton, QuietButton, Segmented } from './bits'; +import { Icon } from './Icon'; +import { Explainer, Field, Group, NumberField } from './SettingsBits'; +import { useArmedConfirm } from './useArmedConfirm'; +import { commitKeyHandler } from './useCommitOnEnter'; const LENS_OPTIONS: [string, string][] = Object.entries(LENS_LABELS); // the Projects tab exists only on a server set up for projects -const LENS_OPTIONS_NO_PROJECTS = LENS_OPTIONS.filter( - ([id]) => id !== "projects" -); +const LENS_OPTIONS_NO_PROJECTS = LENS_OPTIONS.filter(([id]) => id !== 'projects'); // Stats has no lanes, so it's left out of the per-lens lane-length list // (unlike LENS_OPTIONS above, which covers every tab including Stats) — the // membership stays its own explicit list, but each label still comes from // the shared map so it can't drift from LENS_OPTIONS' wording. -const LANE_CAP_LENSES: Lens[] = ["review", "mine", "team", "classic", "ci"]; -const LANE_CAP_LENS_OPTIONS: [string, string][] = LANE_CAP_LENSES.map((id) => [ - id, - LENS_LABELS[id], -]); +const LANE_CAP_LENSES: Lens[] = ['review', 'mine', 'team', 'classic', 'ci']; +const LANE_CAP_LENS_OPTIONS: [string, string][] = LANE_CAP_LENSES.map(id => [id, LENS_LABELS[id]]); const LANE_CAP_OPTIONS: [string, string][] = [ - ["default", "Default"], - ["10", "10"], - ["25", "25"], - ["50", "50"], - ["0", "No cap"], + ['default', 'Default'], + ['10', '10'], + ['25', '25'], + ['50', '50'], + ['0', 'No cap'], ]; /** One lens' lane-length override row, inside the "Different length per view" * disclosure. "Default" clears the override so the lens falls back to the * global Lane length above. */ -function LaneCapByLensRow({ - lensId, - label, -}: { - lensId: string; - label: string; -}) { - const cap = useSettings().laneCapByLens[lensId]; - return ( - - - setLaneCapForLens(lensId, v === "default" ? null : Number(v)) - } - /> - - ); +function LaneCapByLensRow({ lensId, label }: { lensId: string; label: string }) { + const cap = useSettings().laneCapByLens[lensId]; + return ( + + setLaneCapForLens(lensId, v === 'default' ? null : Number(v))} + /> + + ); } /** Free-text editor for the code regions that put matching self-reviews into * Review's Could use your input. Arbitrary strings (not a known set), so it's a plain input * plus removable chips, not a candidate picker. */ function CodeRegionsGroup() { - const regions = useSettings().codeRegions; - const [draft, setDraft] = useState(""); - const add = () => { - addCodeRegion(draft); - setDraft(""); - }; - return ( - - - Areas you own or care about. Someone’s self-reviewed PR whose title, - description, labels, branch, or repo contains one shows under Could use - your input on Review. - -
    - setDraft(e.target.value)} - onKeyDown={commitKeyHandler({ - onCommit: add, - onCancel: () => setDraft(""), - })} - placeholder="e.g. Growthbook, Shopify, Diagrams" - aria-label="add a code region" - className="min-w-0 flex-1 rounded-md border border-line bg-surface px-2 py-1 text-sm outline-none focus-visible:border-brand" - /> - - Add - -
    - {regions.length > 0 && ( -
      - {regions.map((r) => ( -
    • - - {r} - - -
      • - ))} -
    - )} -     - ); + const regions = useSettings().codeRegions; + const [draft, setDraft] = useState(''); + const add = () => { + addCodeRegion(draft); + setDraft(''); + }; + return ( + + + Areas you own or care about. Someone’s self-reviewed PR whose title, description, + labels, branch, or repo contains one shows under Could use your input on Review. + +
    + setDraft(e.target.value)} + onKeyDown={commitKeyHandler({ + onCommit: add, + onCancel: () => setDraft(''), + })} + placeholder="e.g. Growthbook, Shopify, Diagrams" + aria-label="add a code region" + className="min-w-0 flex-1 rounded-md border border-line bg-surface px-2 py-1 text-sm outline-none focus-visible:border-brand" + /> + + Add + +
    + {regions.length > 0 && ( +
      + {regions.map(r => ( +
    • + + {r} + + +
      • + ))} +
    + )} +     + ); } export function Settings({ - onGoToTeam, + onGoToTeam, }: { - /** switch the board to the Team lens and close this panel — the "Edited - * on the board" pointer line's action. The team picker and repo manager - * embeds are gone (Team view and the header's Repos filter own those - * now), so this is the only cross-surface wiring this panel needs. */ - onGoToTeam: () => void; + /** switch the board to the Team lens and close this panel — the "Edited + * on the board" pointer line's action. The team picker and repo manager + * embeds are gone (Team view and the header's Repos filter own those + * now), so this is the only cross-surface wiring this panel needs. */ + onGoToTeam: () => void; }) { - const [open, setOpen] = useState(false); - const s = useSettings(); - const { refreshProgress, projectLabelPrefix } = usePulldasher(); - const panelRef = useRef(null); - const triggerRef = useRef(null); - const { armed: armReset, run: runReset } = useArmedConfirm(); + const [open, setOpen] = useState(false); + const s = useSettings(); + const { refreshProgress, projectLabelPrefix } = usePulldasher(); + const panelRef = useRef(null); + const triggerRef = useRef(null); + const { armed: armReset, run: runReset } = useArmedConfirm(); - // which section to scroll to and focus once open: a Group's id - const [section, setSection] = useState(null); + // which section to scroll to and focus once open: a Group's id + const [section, setSection] = useState(null); - // open on demand — the code-regions tip's "Set them up" action dispatches - // this so a reader can jump straight here from the board; a CustomEvent - // whose detail names a Group's id opens at that group (Finish line's cog) - useEffect(() => { - const openIt = (e: Event) => { - setSection( - typeof (e as CustomEvent).detail === "string" - ? (e as CustomEvent).detail - : null - ); - setOpen(true); - }; - window.addEventListener("pd2:open-settings", openIt); - return () => window.removeEventListener("pd2:open-settings", openIt); - }, []); + // open on demand — the code-regions tip's "Set them up" action dispatches + // this so a reader can jump straight here from the board; a CustomEvent + // whose detail names a Group's id opens at that group (Finish line's cog) + useEffect(() => { + const openIt = (e: Event) => { + setSection( + typeof (e as CustomEvent).detail === 'string' ? (e as CustomEvent).detail : null + ); + setOpen(true); + }; + window.addEventListener('pd2:open-settings', openIt); + return () => window.removeEventListener('pd2:open-settings', openIt); + }, []); - useEffect(() => { - if (!open) return; - const onKey = (e: KeyboardEvent) => { - // a dialog above this one (Finish line) handled it - if (e.defaultPrevented) return; - if (e.key === "Escape") setOpen(false); - // aria-modal promises a focus trap; without this, Tab walks out of - // the dialog into the live board behind the scrim - if (e.key === "Tab" && panelRef.current) { - const focusables = panelRef.current.querySelectorAll( - 'button, input, select, textarea, a[href], [tabindex]:not([tabindex="-1"])' - ); - if (!focusables.length) return; - const first = focusables[0]; - const last = focusables[focusables.length - 1]; - const active = document.activeElement; - if (e.shiftKey && (active === first || active === panelRef.current)) { - e.preventDefault(); - last.focus(); - } else if (!e.shiftKey && active === last) { - e.preventDefault(); - first.focus(); - } + useEffect(() => { + if (!open) return; + const onKey = (e: KeyboardEvent) => { + // a dialog above this one (Finish line) handled it + if (e.defaultPrevented) return; + if (e.key === 'Escape') setOpen(false); + // aria-modal promises a focus trap; without this, Tab walks out of + // the dialog into the live board behind the scrim + if (e.key === 'Tab' && panelRef.current) { + const focusables = panelRef.current.querySelectorAll( + 'button, input, select, textarea, a[href], [tabindex]:not([tabindex="-1"])' + ); + if (!focusables.length) return; + const first = focusables[0]; + const last = focusables[focusables.length - 1]; + const active = document.activeElement; + if (e.shiftKey && (active === first || active === panelRef.current)) { + e.preventDefault(); + last.focus(); + } else if (!e.shiftKey && active === last) { + e.preventDefault(); + first.focus(); + } + } + }; + document.addEventListener('keydown', onKey); + // a modal locks the page behind it: the scrim already blocks clicks, + // this blocks scroll + const prevOverflow = document.body.style.overflow; + document.body.style.overflow = 'hidden'; + // focus the panel so Escape and tabbing land inside it, or the first + // control of the section it was opened at + panelRef.current?.focus(); + if (section) { + const target = panelRef.current?.querySelector(`#${section}`); + target?.scrollIntoView({ block: 'start' }); + target?.querySelector('button, input')?.focus({ preventScroll: true }); + setSection(null); } - }; - document.addEventListener("keydown", onKey); - // a modal locks the page behind it: the scrim already blocks clicks, - // this blocks scroll - const prevOverflow = document.body.style.overflow; - document.body.style.overflow = "hidden"; - // focus the panel so Escape and tabbing land inside it, or the first - // control of the section it was opened at - panelRef.current?.focus(); - if (section) { - const target = panelRef.current?.querySelector( - `#${section}` - ); - target?.scrollIntoView({ block: "start" }); - target - ?.querySelector("button, input") - ?.focus({ preventScroll: true }); - setSection(null); - } - return () => { - document.removeEventListener("keydown", onKey); - document.body.style.overflow = prevOverflow; - }; - // section is read once, when it opens, so it isn't a dependency - }, [open]); + return () => { + document.removeEventListener('keydown', onKey); + document.body.style.overflow = prevOverflow; + }; + // section is read once, when it opens, so it isn't a dependency + }, [open]); - // return focus to the cog when the panel closes — but only after a real - // open->close transition, not on initial mount (open starts false, and - // stealing focus to the gear on page load with no user interaction) - const didOpenRef = useRef(false); - useEffect(() => { - if (open) didOpenRef.current = true; - else if (didOpenRef.current) triggerRef.current?.focus?.(); - }, [open]); + // return focus to the cog when the panel closes — but only after a real + // open->close transition, not on initial mount (open starts false, and + // stealing focus to the gear on page load with no user interaction) + const didOpenRef = useRef(false); + useEffect(() => { + if (open) didOpenRef.current = true; + else if (didOpenRef.current) triggerRef.current?.focus?.(); + }, [open]); - const set = (patch: Partial) => setSettings(patch); + const set = (patch: Partial) => setSettings(patch); - return ( - <> - setOpen((o) => !o)} - /> - {open && - createPortal( -
    -
    setOpen(false)} - aria-hidden - /> -
    - {/* z-10: children further down the panel (avatars, the + return ( + <> + setOpen(o => !o)} + /> + {open && + createPortal( +
    +
    setOpen(false)} + aria-hidden + /> +
    + {/* z-10: children further down the panel (avatars, the code-regions Add button — anything position:relative via .hit/.pressable) paint above a sticky header with no z-index of its own once they scroll under it. The overlay above is already z-[100], so z-10 in here only has to beat panel content. */} -
    - Settings - -
    +
    + Settings + +
    - - - set({ theme })} - /> - - - set({ density })} - /> - - - set({ defaultLens })} - /> - - + + + set({ theme })} + /> + + + set({ density })} + /> + + + set({ defaultLens })} + /> + + - - - set({ laneCap: Number(v) })} - /> - - - - Classic can show everything while Review stays short. - -
    - {LANE_CAP_LENS_OPTIONS.map(([id, label]) => ( - - ))} -
    -     - - set({ repoQueueCap: Number(v) })} - /> - - - set({ ageWarnDays })} - /> - - - set({ ageDisplay })} - /> - - - set({ claimWarnMins: Number(v) })} - /> - - - set({ openPrsNewTab: v === "new" })} - /> - - - set({ hoverDelayMs: Number(v) })} - /> - -     + + + set({ laneCap: Number(v) })} + /> + + + + Classic can show everything while Review stays short. + +
    + {LANE_CAP_LENS_OPTIONS.map(([id, label]) => ( + + ))} +
    +     + + set({ repoQueueCap: Number(v) })} + /> + + + set({ ageWarnDays })} + /> + + + set({ ageDisplay })} + /> + + + set({ claimWarnMins: Number(v) })} + /> + + + set({ openPrsNewTab: v === 'new' })} + /> + + + set({ hoverDelayMs: Number(v) })} + /> + +     - - - A count in the header of your own open PRs past your limits, - and a list of them to merge or close. Only you see it. - - - set({ finishLineMode })} - /> - - - set({ finishLineMaxOpen })} - /> - - - set({ finishLineMaxDays })} - /> - - - set({ finishLineCountDrafts: v === "on" })} - /> - - - set({ finishLineHoldToClose: v === "on" })} - /> - - - { - // close Settings first so the list isn't stacked on it - setOpen(false); - window.dispatchEvent( - new Event("pd2:preview-finish-line") - ); - }} - > - Preview - - - + + + A count in the header of your own open PRs past your limits, and a list + of them to merge or close. Only you see it. + + + set({ finishLineMode })} + /> + + + set({ finishLineMaxOpen })} + /> + + + set({ finishLineMaxDays })} + /> + + + set({ finishLineCountDrafts: v === 'on' })} + /> + + + set({ finishLineHoldToClose: v === 'on' })} + /> + + + { + // close Settings first so the list isn't stacked on it + setOpen(false); + window.dispatchEvent(new Event('pd2:preview-finish-line')); + }} + > + Preview + + + - + - -
    - - Teams: build your rosters on the Team view. - - { - onGoToTeam(); - setOpen(false); - }} - > - Open Team view - -
    - - Repos: hide and reveal from the Repos filter in the header. - -     + +
    + + Teams: build your rosters on the Team view. + + { + onGoToTeam(); + setOpen(false); + }} + > + Open Team view + +
    + + Repos: hide and reveal from the Repos filter in the header. + +     -
    - - - Check the board against GitHub and re-fetch the PRs that - differ, including ones it missed or still shows open. The - board updates as each one comes back. - -
    - refreshAll()} - > - Refresh all - - {/* role=status stays mounted so the announcement fires +
    + + + Check the board against GitHub and re-fetch the PRs that differ, + including ones it missed or still shows open. The board updates as + each one comes back. + +
    + refreshAll()} + > + Refresh all + + {/* role=status stays mounted so the announcement fires when the text lands — a screen reader hears the confirmation, not just sighted users. It takes the room the button leaves and wraps in it, so a long line never squeezes the button onto two lines */} - - {refreshProgress ? refreshAllText(refreshProgress) : ""} - -
    + + {refreshProgress ? refreshAllText(refreshProgress) : ''} + +
    - - Reset every preference on this browser (theme, filters, - hidden repos and people, last-seen marker) back to defaults. - This can’t be undone. - -
    - + + Reset every preference on this browser (theme, filters, hidden repos + and people, last-seen marker) back to defaults. This can’t be undone. + +
    + +
    + +
    - - -
    -
    , - document.body - )} - - ); +
    , + document.body + )} + + ); } diff --git a/frontend-v2/src/model/finishLine.ts b/frontend-v2/src/model/finishLine.ts index 0d9fde96..1c3aec17 100644 --- a/frontend-v2/src/model/finishLine.ts +++ b/frontend-v2/src/model/finishLine.ts @@ -1,7 +1,7 @@ -import type { DerivedPull } from "../../../shared/model/status"; -import { isBotLogin } from "../../../shared/model/visibility"; -import type { PullData } from "../../../shared/types"; -import { rowWord } from "./actions"; +import type { DerivedPull } from '../../../shared/model/status'; +import { isBotLogin } from '../../../shared/model/visibility'; +import type { PullData } from '../../../shared/types'; +import { rowWord } from './actions'; /** Over the finish line's limit with more than this many open PRs of your own. */ export const FINISH_MAX_OPEN = 7; @@ -9,51 +9,48 @@ export const FINISH_MAX_OPEN = 7; export const FINISH_MAX_DAYS = 14; /** localStorage key holding the FinishRecord of the last time it popped up. */ -export const FINISH_LINE_KEY = "pd2.finishLine"; +export const FINISH_LINE_KEY = 'pd2.finishLine'; /** After it pops up, it stays away at least this long. */ export const FINISH_COOLDOWN_MS = 4 * 60 * 60 * 1000; /** What it showed you last time, so it only comes back when that got worse. */ export interface FinishRecord { - /** ms when it showed */ - at: number; - /** how many counted open PRs you had then */ - count: number; - /** repo#number of each PR past the age limit then */ - over: string[]; + /** ms when it showed */ + at: number; + /** how many counted open PRs you had then */ + count: number; + /** repo#number of each PR past the age limit then */ + over: string[]; } /** The fields the rule reads off a derived pull. */ export interface FinishPull { - data: Pick< - PullData, - "repo" | "number" | "title" | "draft" | "state" | "user" - >; - ageDays: number; - /** who holds a deploy block on it; a held PR is done, so it doesn't count */ - deployBlockedBy: string[]; - status: DerivedPull["status"]; - /** parked (Cryogenic Storage): kept open on purpose, never "ready" */ - cryo: boolean; + data: Pick; + ageDays: number; + /** who holds a deploy block on it; a held PR is done, so it doesn't count */ + deployBlockedBy: string[]; + status: DerivedPull['status']; + /** parked (Cryogenic Storage): kept open on purpose, never "ready" */ + cryo: boolean; } /** Can merge now, the quickest way to the finish line. A parked PR can be * signed off and green, but My work files it under waiting, so this does too. */ -export const isFinishReady = (p: FinishPull) => p.status === "ready" && !p.cryo; +export const isFinishReady = (p: FinishPull) => p.status === 'ready' && !p.cryo; export interface FinishLineCase

    { - /** the PRs that put you there, oldest first */ - pulls: P[]; - /** how many of `pulls` can merge now */ - ready: number; - /** how many open PRs counted */ - count: number; - /** repo#number of each counted PR past the age limit */ - over: string[]; - /** how many PRs you still have to merge or close to finish: enough to - * get under the count limit, and every one past the age limit (closing an - * old one counts toward both) */ - toGo: number; + /** the PRs that put you there, oldest first */ + pulls: P[]; + /** how many of `pulls` can merge now */ + ready: number; + /** how many open PRs counted */ + count: number; + /** repo#number of each counted PR past the age limit */ + over: string[]; + /** how many PRs you still have to merge or close to finish: enough to + * get under the count limit, and every one past the age limit (closing an + * old one counts toward both) */ + toGo: number; } const idOf = (p: FinishPull) => `${p.data.repo}#${p.data.number}`; @@ -66,35 +63,35 @@ const oldestFirst =

    (a: P, b: P) => b.ageDays - a.ageDays; * under both. */ export function finishLineCase

    ( - pulls: P[], - me: string, - extraBots: ReadonlySet, - limits: { maxOpen: number; maxDays: number; countDrafts?: boolean } + pulls: P[], + me: string, + extraBots: ReadonlySet, + limits: { maxOpen: number; maxDays: number; countDrafts?: boolean } ): FinishLineCase

    | null { - const mine = pulls - .filter( - (p) => - p.data.user.login === me && - p.data.state === "open" && - (limits.countDrafts || !p.data.draft) && - !p.deployBlockedBy.length && - !isBotLogin(p.data.user.login, extraBots) - ) - .sort(oldestFirst); - const old = mine.filter((p) => p.ageDays > limits.maxDays); - const counts = { - count: mine.length, - over: old.map(idOf), - toGo: Math.max(mine.length - limits.maxOpen, old.length), - }; - if (mine.length <= limits.maxOpen && !old.length) return null; - const listed = mine.length > limits.maxOpen ? mine : old; - const ready = listed.filter(isFinishReady).length; - return { - pulls: listed, - ready, - ...counts, - }; + const mine = pulls + .filter( + p => + p.data.user.login === me && + p.data.state === 'open' && + (limits.countDrafts || !p.data.draft) && + !p.deployBlockedBy.length && + !isBotLogin(p.data.user.login, extraBots) + ) + .sort(oldestFirst); + const old = mine.filter(p => p.ageDays > limits.maxDays); + const counts = { + count: mine.length, + over: old.map(idOf), + toGo: Math.max(mine.length - limits.maxOpen, old.length), + }; + if (mine.length <= limits.maxOpen && !old.length) return null; + const listed = mine.length > limits.maxOpen ? mine : old; + const ready = listed.filter(isFinishReady).length; + return { + pulls: listed, + ready, + ...counts, + }; } /** @@ -103,42 +100,35 @@ export function finishLineCase

    ( * on others. Each group runs oldest first; the row's word says the rest. */ export function finishGroups(pulls: DerivedPull[], me: string) { - const worded = pulls - .map((pull) => ({ pull, word: rowWord(pull, me) })) - .sort((a, b) => b.pull.ageDays - a.pull.ageDays); - const rest = worded.filter((w) => !isFinishReady(w.pull)); - return { - ready: worded.filter((w) => isFinishReady(w.pull)), - move: rest.filter((w) => w.word.kind === "do"), - waiting: rest.filter((w) => w.word.kind === "wait"), - }; + const worded = pulls + .map(pull => ({ pull, word: rowWord(pull, me) })) + .sort((a, b) => b.pull.ageDays - a.pull.ageDays); + const rest = worded.filter(w => !isFinishReady(w.pull)); + return { + ready: worded.filter(w => isFinishReady(w.pull)), + move: rest.filter(w => w.word.kind === 'do'), + waiting: rest.filter(w => w.word.kind === 'wait'), + }; } /** The record to store when `found` shows at `now`. */ -export const finishRecord = ( - found: FinishLineCase, - now: number -): FinishRecord => ({ - at: now, - count: found.count, - over: found.over, +export const finishRecord = (found: FinishLineCase, now: number): FinishRecord => ({ + at: now, + count: found.count, + over: found.over, }); /** A stored record, or null when it's missing or unreadable. */ export function parseFinishRecord(raw: string | null): FinishRecord | null { - try { - const r = JSON.parse(raw ?? "null") as Partial | null; - if ( - typeof r?.at !== "number" || - typeof r.count !== "number" || - !Array.isArray(r.over) - ) { + try { + const r = JSON.parse(raw ?? 'null') as Partial | null; + if (typeof r?.at !== 'number' || typeof r.count !== 'number' || !Array.isArray(r.over)) { + return null; + } + return { at: r.at, count: r.count, over: r.over }; + } catch { return null; - } - return { at: r.at, count: r.count, over: r.over }; - } catch { - return null; - } + } } /** @@ -147,29 +137,27 @@ export function parseFinishRecord(raw: string | null): FinishRecord | null { * (more open PRs, net, or a PR newly past the age limit). */ export function finishDue( - found: FinishLineCase | null, - last: FinishRecord | null, - now: number + found: FinishLineCase | null, + last: FinishRecord | null, + now: number ): boolean { - if (!found) return false; - if (!last) return true; - if (now - last.at < FINISH_COOLDOWN_MS) return false; - return ( - found.count > last.count || found.over.some((id) => !last.over.includes(id)) - ); + if (!found) return false; + if (!last) return true; + if (now - last.at < FINISH_COOLDOWN_MS) return false; + return found.count > last.count || found.over.some(id => !last.over.includes(id)); } /** One number at the top of the list you can act on, and which PRs it * points at, so clicking it can open their group. */ export interface FinishStat { - key: "open" | "old" | "ready"; - text: string; - /** the PRs this number counts */ - counts: (p: FinishPull) => boolean; + key: 'open' | 'old' | 'ready'; + text: string; + /** the PRs this number counts */ + counts: (p: FinishPull) => boolean; } /** "1 PR", "2 PRs" */ -export const prCount = (n: number) => (n === 1 ? "1 PR" : `${n} PRs`); +export const prCount = (n: number) => (n === 1 ? '1 PR' : `${n} PRs`); /** * The numbers you can act on, each only when above zero: how many open @@ -177,27 +165,27 @@ export const prCount = (n: number) => (n === 1 ? "1 PR" : `${n} PRs`); * Waiting on others is left out: there's nothing to do about it here. */ export function finishStats( - found: Pick, "count" | "over" | "ready">, - limits: { maxOpen: number; maxDays: number } + found: Pick, 'count' | 'over' | 'ready'>, + limits: { maxOpen: number; maxDays: number } ): FinishStat[] { - const stats: FinishStat[] = []; - if (found.count > limits.maxOpen) - stats.push({ - key: "open", - text: `${found.count} open PRs, aim for ${limits.maxOpen}`, - counts: () => true, - }); - if (found.over.length) - stats.push({ - key: "old", - text: `${prCount(found.over.length)} older than ${limits.maxDays} days`, - counts: (p) => p.ageDays > limits.maxDays, - }); - if (found.ready) - stats.push({ - key: "ready", - text: `${prCount(found.ready)} ready to merge`, - counts: isFinishReady, - }); - return stats; + const stats: FinishStat[] = []; + if (found.count > limits.maxOpen) + stats.push({ + key: 'open', + text: `${found.count} open PRs, aim for ${limits.maxOpen}`, + counts: () => true, + }); + if (found.over.length) + stats.push({ + key: 'old', + text: `${prCount(found.over.length)} older than ${limits.maxDays} days`, + counts: p => p.ageDays > limits.maxDays, + }); + if (found.ready) + stats.push({ + key: 'ready', + text: `${prCount(found.ready)} ready to merge`, + counts: isFinishReady, + }); + return stats; }