From 22efdec153ac080deb3a3c53db1a023ee5b7a988 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 10:22:21 +0000 Subject: [PATCH] refactor: collapse paired picker-open booleans and tidy boot-size cleanup Three paydowns on last week's merges. The event form tracked its start/end date pickers as two independent booleans, so both could be open at once and every open handler had to remember to close its sibling. EventForm threaded four props through DateTimeSection into DatePickers to do it, and TimePickers hand-rolled the same pair again with its own open helpers. One `useOpenDatePickerSide` hook holds the open side instead, and `close(side)` ignores a close from the side that is not open, which is what the sibling-closing dance was protecting against. NotificationPermissionBanner re-declared the banner kind union instead of importing NotificationBlockedBannerKind from the util that picks it. pr-boot-size-delta exited inside its try block on a budget failure, which skipped the finally that removes the base worktree and the temp metrics file, and the same finally removed a worktree that `git worktree add` may never have created, throwing over the real error. The sticky comment now posts and patches through one `gh api` call instead of writing a temp file for the create path. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015fhM8mEj3LaYCn8My4M9u3 --- .github/perf/pr-boot-size-delta.ts | 108 +++++++++--------- .../NotificationPermissionBanner.tsx | 3 +- .../DatePickers/DatePickers.tsx | 44 +++---- .../DateTimeSection/DateTimeSection.tsx | 16 +-- .../TimePicker/TimePickers.tsx | 28 ++--- .../useOpenDatePickerSide.test.ts | 53 +++++++++ .../DateTimeSection/useOpenDatePickerSide.ts | 37 ++++++ .../src/views/Forms/EventForm/EventForm.tsx | 18 +-- 8 files changed, 177 insertions(+), 130 deletions(-) create mode 100644 apps/calendar-web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/useOpenDatePickerSide.test.ts create mode 100644 apps/calendar-web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/useOpenDatePickerSide.ts diff --git a/.github/perf/pr-boot-size-delta.ts b/.github/perf/pr-boot-size-delta.ts index 8748129cc..5770f81fe 100644 --- a/.github/perf/pr-boot-size-delta.ts +++ b/.github/perf/pr-boot-size-delta.ts @@ -4,7 +4,7 @@ * the PR has the bundle-size-ok label. */ import { execFileSync } from "node:child_process"; -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { mkdtempSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; @@ -56,14 +56,6 @@ function measureBootGzip(cwd: string, metricsPath: string): BootMetrics { return parseMetricsFile(metricsPath); } -function ghJson(args: string[]): unknown { - const out = execFileSync("gh", args, { - encoding: "utf8", - cwd: repoRoot, - }).trim(); - return out ? JSON.parse(out) : null; -} - function hasOverrideLabel(prNumber: string): boolean { const out = execFileSync( "gh", @@ -73,48 +65,41 @@ function hasOverrideLabel(prNumber: string): boolean { return out.split("\n").filter(Boolean).includes(overrideLabel); } +function listPrComments( + prNumber: string, +): Array<{ id: number; body?: string }> { + const out = execFileSync( + "gh", + [ + "api", + `repos/${process.env.GITHUB_REPOSITORY}/issues/${prNumber}/comments`, + "--paginate", + ], + { encoding: "utf8", cwd: repoRoot }, + ).trim(); + return out ? (JSON.parse(out) as Array<{ id: number; body?: string }>) : []; +} + function upsertStickyComment(prNumber: string, body: string): void { - const marker = commentMarker; - const fullBody = `${marker}\n${body}`; - const comments = ghJson([ - "api", - `repos/${process.env.GITHUB_REPOSITORY}/issues/${prNumber}/comments`, - "--paginate", - ]) as Array<{ id: number; body?: string }> | null; - - const existing = (comments ?? []).find((comment) => - (comment.body ?? "").includes(marker), + const fullBody = `${commentMarker}\n${body}`; + const existing = listPrComments(prNumber).find((comment) => + (comment.body ?? "").includes(commentMarker), ); - if (existing) { - execFileSync( - "gh", - [ - "api", - "--method", - "PATCH", - `repos/${process.env.GITHUB_REPOSITORY}/issues/comments/${existing.id}`, - "-f", - `body=${fullBody}`, - ], - { cwd: repoRoot, stdio: "inherit" }, - ); - return; - } - - const commentFile = path.join( - tmpdir(), - `compass-boot-comment-${process.pid}.md`, + const repo = process.env.GITHUB_REPOSITORY; + execFileSync( + "gh", + [ + "api", + "--method", + existing ? "PATCH" : "POST", + existing + ? `repos/${repo}/issues/comments/${existing.id}` + : `repos/${repo}/issues/${prNumber}/comments`, + "-f", + `body=${fullBody}`, + ], + { cwd: repoRoot, stdio: "inherit" }, ); - writeFileSync(commentFile, fullBody, "utf8"); - try { - execFileSync( - "gh", - ["pr", "comment", prNumber, "--body-file", commentFile], - { cwd: repoRoot, stdio: "inherit" }, - ); - } finally { - rmSync(commentFile, { force: true }); - } } function main(): void { @@ -137,6 +122,8 @@ function main(): void { const headMetrics = measureBootGzip(repoRoot, headMetricsPath); const baseWorktree = mkdtempSync(path.join(tmpdir(), "compass-boot-base-")); + let worktreeAdded = false; + let overBudget = false; try { execFileSync( "git", @@ -146,6 +133,7 @@ function main(): void { stdio: "inherit", }, ); + worktreeAdded = true; execFileSync("bun", ["install", "--frozen-lockfile"], { cwd: baseWorktree, stdio: "inherit", @@ -170,18 +158,30 @@ function main(): void { } const overridden = prNumber ? hasOverrideLabel(prNumber) : false; - if (deltaBytes > maxDeltaBytes && !overridden) { + overBudget = deltaBytes > maxDeltaBytes && !overridden; + if (overBudget) { console.error( `Boot gzip grew ${deltaBytes} B (> ${maxDeltaBytes} B) vs ${baseSha}.`, ); - process.exit(1); } } finally { rmSync(headMetricsPath, { force: true }); - execFileSync("git", ["worktree", "remove", "--force", baseWorktree], { - cwd: repoRoot, - stdio: "inherit", - }); + // `git worktree add` can fail (bad sha, dirty state); removing a worktree + // that was never added throws over the real error, so clean up the bare + // mkdtemp directory instead. + if (worktreeAdded) { + execFileSync("git", ["worktree", "remove", "--force", baseWorktree], { + cwd: repoRoot, + stdio: "inherit", + }); + } else { + rmSync(baseWorktree, { force: true, recursive: true }); + } + } + + // Exiting inside the try above would skip the cleanup in `finally`. + if (overBudget) { + process.exit(1); } } diff --git a/apps/calendar-web/src/notifications/NotificationPermissionBanner.tsx b/apps/calendar-web/src/notifications/NotificationPermissionBanner.tsx index 6e63a057c..ddc9da348 100644 --- a/apps/calendar-web/src/notifications/NotificationPermissionBanner.tsx +++ b/apps/calendar-web/src/notifications/NotificationPermissionBanner.tsx @@ -1,10 +1,11 @@ import { notificationActions } from "@web/notifications/notification.state"; +import { type NotificationBlockedBannerKind } from "@web/notifications/notification-blocked-banner.util"; export function NotificationPermissionBanner({ kind, onDismiss, }: { - kind: "permission" | "show_blocked"; + kind: NotificationBlockedBannerKind; onDismiss: () => void; }) { const message = diff --git a/apps/calendar-web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/DatePickers/DatePickers.tsx b/apps/calendar-web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/DatePickers/DatePickers.tsx index 003847420..a624e01ca 100644 --- a/apps/calendar-web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/DatePickers/DatePickers.tsx +++ b/apps/calendar-web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/DatePickers/DatePickers.tsx @@ -4,31 +4,28 @@ import dayjs from "@core/util/date/dayjs"; import { shouldAdjustComplimentDate } from "@web/common/utils/datetime/web.datetime.util"; import { type GridEventDraft } from "@web/events/event-draft.types"; import { EventFormDateField } from "@web/views/Forms/EventForm/DateControlsSection/DateTimeSection/EventFormDateField"; +import { type OpenDatePickerSide } from "@web/views/Forms/EventForm/DateControlsSection/DateTimeSection/useOpenDatePickerSide"; import { type OnEventFormScheduleChange } from "@web/views/Forms/EventForm/types"; interface Props { + datePickers: OpenDatePickerSide; displayEndDate: Date; draft: GridEventDraft; - isEndDatePickerOpen: boolean; - isStartDatePickerOpen: boolean; selectedEndDate: Date; selectedStartDate: Date; onScheduleChange: OnEventFormScheduleChange; - setIsStartDatePickerOpen: (arg0: boolean) => void; - setIsEndDatePickerOpen: (arg0: boolean) => void; } export const DatePickers: FC = ({ + datePickers, displayEndDate, draft, - isEndDatePickerOpen, - isStartDatePickerOpen, selectedEndDate, selectedStartDate, onScheduleChange, - setIsEndDatePickerOpen, - setIsStartDatePickerOpen, }) => { + const { openSide, open, close, closeAll } = datePickers; + const toAllDayDate = (date: Date) => dayjs(dayjs(date).format(dayjs.DateFormat.YEAR_MONTH_DAY_FORMAT)).toDate(); @@ -41,7 +38,7 @@ export const DatePickers: FC = ({ }; const onSelectStartDate = (start: Date) => { - setIsStartDatePickerOpen(false); + close("start"); const { shouldAdjust: shouldAdjustEnd, compliment } = shouldAdjustComplimentDate("start", { @@ -59,7 +56,7 @@ export const DatePickers: FC = ({ }; const onSelectEndDate = (end: Date) => { - setIsEndDatePickerOpen(false); + close("end"); const { shouldAdjust, compliment } = shouldAdjustComplimentDate("end", { start: selectedStartDate, @@ -81,15 +78,8 @@ export const DatePickers: FC = ({ } }; - const closeOpenPickers = () => { - const wasOpen = isStartDatePickerOpen || isEndDatePickerOpen; - setIsStartDatePickerOpen(false); - setIsEndDatePickerOpen(false); - return wasOpen; - }; - const onPickerEscape = (event: React.KeyboardEvent) => { - if (event.key === "Escape" && closeOpenPickers()) { + if (event.key === "Escape" && closeAll()) { event.stopPropagation(); } }; @@ -100,12 +90,9 @@ export const DatePickers: FC = ({ arrowDownOpensCalendar calendarClassName="startDatePicker" inputId="eventFormStartDate" - isOpen={isStartDatePickerOpen} - onCalendarClose={() => setIsStartDatePickerOpen(false)} - onCalendarOpen={() => { - setIsEndDatePickerOpen(false); - setIsStartDatePickerOpen(true); - }} + isOpen={openSide === "start"} + onCalendarClose={() => close("start")} + onCalendarOpen={() => open("start")} onCommit={onSelectStartDate} onKeyDown={onPickerEscape} selected={selectedStartDate} @@ -116,12 +103,9 @@ export const DatePickers: FC = ({ arrowDownOpensCalendar calendarClassName="endDatePicker" inputId="eventFormEndDate" - isOpen={isEndDatePickerOpen} - onCalendarClose={() => setIsEndDatePickerOpen(false)} - onCalendarOpen={() => { - setIsStartDatePickerOpen(false); - setIsEndDatePickerOpen(true); - }} + isOpen={openSide === "end"} + onCalendarClose={() => close("end")} + onCalendarOpen={() => open("end")} onCommit={onSelectEndDate} onKeyDown={onPickerEscape} selected={displayEndDate} diff --git a/apps/calendar-web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/DateTimeSection.tsx b/apps/calendar-web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/DateTimeSection.tsx index 2a7ff883f..05541b9db 100644 --- a/apps/calendar-web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/DateTimeSection.tsx +++ b/apps/calendar-web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/DateTimeSection.tsx @@ -3,36 +3,31 @@ import { type SelectOption } from "@web/common/types/component.types"; import { type GridEventDraft } from "@web/events/event-draft.types"; import { DatePickers } from "@web/views/Forms/EventForm/DateControlsSection/DateTimeSection/DatePickers/DatePickers"; import { TimePickers } from "@web/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/TimePickers"; +import { type OpenDatePickerSide } from "@web/views/Forms/EventForm/DateControlsSection/DateTimeSection/useOpenDatePickerSide"; import { type OnEventFormScheduleChange } from "@web/views/Forms/EventForm/types"; export interface Props { + datePickers: OpenDatePickerSide; displayEndDate: Date; draft: GridEventDraft; endTime: SelectOption; - isEndDatePickerOpen: boolean; - isStartDatePickerOpen: boolean; onScheduleChange: OnEventFormScheduleChange; scheduleEnd: Date; scheduleStart: Date; selectedEndDate: Date; selectedStartDate: Date; - setIsEndDatePickerOpen: (arg0: boolean) => void; - setIsStartDatePickerOpen: (arg0: boolean) => void; startTime: SelectOption; } export const DateTimeSection: FC = ({ + datePickers, displayEndDate, draft, - isEndDatePickerOpen, - isStartDatePickerOpen, selectedEndDate, selectedStartDate, onScheduleChange, scheduleEnd, scheduleStart, - setIsStartDatePickerOpen, - setIsEndDatePickerOpen, startTime, endTime, }) => { @@ -43,15 +38,12 @@ export const DateTimeSection: FC = ({ {isAllDay && (
)} diff --git a/apps/calendar-web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/TimePickers.tsx b/apps/calendar-web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/TimePickers.tsx index 12adc04aa..ceb693871 100644 --- a/apps/calendar-web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/TimePickers.tsx +++ b/apps/calendar-web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/TimePickers.tsx @@ -10,6 +10,7 @@ import { import { shouldAdjustComplimentTime } from "@web/common/utils/datetime/web.datetime.util"; import { EventFormDateField } from "@web/views/Forms/EventForm/DateControlsSection/DateTimeSection/EventFormDateField"; import { timedScheduleFromDateChange } from "@web/views/Forms/EventForm/DateControlsSection/DateTimeSection/eventFormDateField.util"; +import { useOpenDatePickerSide } from "@web/views/Forms/EventForm/DateControlsSection/DateTimeSection/useOpenDatePickerSide"; import { type OnEventFormScheduleChange } from "@web/views/Forms/EventForm/types"; import { TimePicker } from "./TimePicker"; @@ -38,8 +39,7 @@ export const TimePickers: FC = ({ }) => { const [isStartMenuOpen, setIsStartMenuOpen] = useState(false); const [isEndMenuOpen, setIsEndMenuOpen] = useState(false); - const [isStartDateOpen, setIsStartDateOpen] = useState(false); - const [isEndDateOpen, setIsEndDateOpen] = useState(false); + const datePickers = useOpenDatePickerSide(); const [timeError, setTimeError] = useState(null); const timeErrorId = useId(); @@ -105,7 +105,7 @@ export const TimePickers: FC = ({ }; const onDateSelected = (changed: "start" | "end", nextDate: Date) => { - (changed === "start" ? setIsStartDateOpen : setIsEndDateOpen)(false); + datePickers.close(changed); const mapped = timedScheduleFromDateChange({ changed, nextDate, @@ -155,25 +155,15 @@ export const TimePickers: FC = ({ closeMenu(changed); }; - const openStartDate = () => { - setIsEndDateOpen(false); - setIsStartDateOpen(true); - }; - - const openEndDate = () => { - setIsStartDateOpen(false); - setIsEndDateOpen(true); - }; - return (
setIsStartDateOpen(false)} - onCalendarOpen={openStartDate} + isOpen={datePickers.openSide === "start"} + onCalendarClose={() => datePickers.close("start")} + onCalendarOpen={() => datePickers.open("start")} onCommit={(date) => onDateSelected("start", date)} selected={selectedStartDate} sideLabel="start" @@ -196,9 +186,9 @@ export const TimePickers: FC = ({ setIsEndDateOpen(false)} - onCalendarOpen={openEndDate} + isOpen={datePickers.openSide === "end"} + onCalendarClose={() => datePickers.close("end")} + onCalendarOpen={() => datePickers.open("end")} onCommit={(date) => onDateSelected("end", date)} selected={selectedEndDate} sideLabel="end" diff --git a/apps/calendar-web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/useOpenDatePickerSide.test.ts b/apps/calendar-web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/useOpenDatePickerSide.test.ts new file mode 100644 index 000000000..5ac1190d4 --- /dev/null +++ b/apps/calendar-web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/useOpenDatePickerSide.test.ts @@ -0,0 +1,53 @@ +import { renderHook } from "@testing-library/react"; +import { act } from "react"; +import { useOpenDatePickerSide } from "./useOpenDatePickerSide"; +import { describe, expect, it } from "bun:test"; + +describe("useOpenDatePickerSide", () => { + it("opens one side at a time", () => { + const { result } = renderHook(() => useOpenDatePickerSide()); + + expect(result.current.openSide).toBeNull(); + + act(() => result.current.open("start")); + expect(result.current.openSide).toBe("start"); + + act(() => result.current.open("end")); + expect(result.current.openSide).toBe("end"); + }); + + it("ignores a close from the side that is not open", () => { + const { result } = renderHook(() => useOpenDatePickerSide()); + + act(() => result.current.open("end")); + act(() => result.current.close("start")); + + expect(result.current.openSide).toBe("end"); + }); + + it("closes the open side", () => { + const { result } = renderHook(() => useOpenDatePickerSide()); + + act(() => result.current.open("start")); + act(() => result.current.close("start")); + + expect(result.current.openSide).toBeNull(); + }); + + it("reports whether closeAll had anything to close", () => { + const { result } = renderHook(() => useOpenDatePickerSide()); + + let wasOpen: boolean | undefined; + act(() => { + wasOpen = result.current.closeAll(); + }); + expect(wasOpen).toBe(false); + + act(() => result.current.open("end")); + act(() => { + wasOpen = result.current.closeAll(); + }); + expect(wasOpen).toBe(true); + expect(result.current.openSide).toBeNull(); + }); +}); diff --git a/apps/calendar-web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/useOpenDatePickerSide.ts b/apps/calendar-web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/useOpenDatePickerSide.ts new file mode 100644 index 000000000..b8ab78e2a --- /dev/null +++ b/apps/calendar-web/src/views/Forms/EventForm/DateControlsSection/DateTimeSection/useOpenDatePickerSide.ts @@ -0,0 +1,37 @@ +import { useState } from "react"; + +export type DatePickerSide = "start" | "end"; + +export interface OpenDatePickerSide { + /** The side whose calendar is open, or null when neither is. */ + openSide: DatePickerSide | null; + open: (side: DatePickerSide) => void; + /** + * Closes `side` only when it is the open one: react-datepicker reports the + * outgoing picker's close after the incoming one opened, so an + * unconditional close would shut the calendar that just replaced it. + */ + close: (side: DatePickerSide) => void; + /** Closes whichever side is open; returns whether one was. */ + closeAll: () => boolean; +} + +/** + * Which of a start/end date picker pair is open. Two independent booleans + * allowed both at once and made every open handler responsible for closing + * its sibling; the open side is the whole state. + */ +export const useOpenDatePickerSide = (): OpenDatePickerSide => { + const [openSide, setOpenSide] = useState(null); + + return { + openSide, + open: setOpenSide, + close: (side) => + setOpenSide((current) => (current === side ? null : current)), + closeAll: () => { + setOpenSide(null); + return openSide !== null; + }, + }; +}; diff --git a/apps/calendar-web/src/views/Forms/EventForm/EventForm.tsx b/apps/calendar-web/src/views/Forms/EventForm/EventForm.tsx index 02e212eed..a218c216b 100644 --- a/apps/calendar-web/src/views/Forms/EventForm/EventForm.tsx +++ b/apps/calendar-web/src/views/Forms/EventForm/EventForm.tsx @@ -12,7 +12,6 @@ import { useCallback, useEffect, useRef, - useState, } from "react"; import { type EventFormFocusField } from "@core/shortcuts/event-form-focus-field"; import { KEYMAP } from "@core/shortcuts/keymap"; @@ -83,6 +82,7 @@ import { CalendarSelect } from "@web/views/Forms/EventForm/CalendarSelect/Calend import { DateControlsSection } from "@web/views/Forms/EventForm/DateControlsSection/DateControlsSection/DateControlsSection"; import { getFormDates } from "@web/views/Forms/EventForm/DateControlsSection/DateTimeSection/form.datetime.util"; import { commitFocusedTimePickerDraft } from "@web/views/Forms/EventForm/DateControlsSection/DateTimeSection/TimePicker/timePickerCommitDraft"; +import { useOpenDatePickerSide } from "@web/views/Forms/EventForm/DateControlsSection/DateTimeSection/useOpenDatePickerSide"; import { RecurrenceSection } from "@web/views/Forms/EventForm/DateControlsSection/RecurrenceSection/RecurrenceSection"; import { DiscardUnsavedChangesDialog } from "@web/views/Forms/EventForm/DiscardUnsavedChangesDialog"; import { EventColorPicker } from "@web/views/Forms/EventForm/EventColorPicker/EventColorPicker"; @@ -370,8 +370,7 @@ export const EventForm: React.FC = memo( /******** * State ********/ - const [isEndDatePickerOpen, setIsEndDatePickerOpen] = useState(false); - const [isStartDatePickerOpen, setIsStartDatePickerOpen] = useState(false); + const datePickers = useOpenDatePickerSide(); const setLatestDraft = useCallback( (nextDraft: SetStateAction) => { @@ -679,19 +678,16 @@ export const EventForm: React.FC = memo( : selectedEndDate; const dateTimeSectionProps = { + datePickers, displayEndDate, draft, endTime, - isEndDatePickerOpen, - isStartDatePickerOpen, onScheduleChange, scheduleEnd, scheduleStart, selectedEndDate, selectedStartDate, startTime, - setIsEndDatePickerOpen, - setIsStartDatePickerOpen, }; const recurrenceSectionProps = { @@ -859,13 +855,7 @@ export const EventForm: React.FC = memo( {...props} name={ID_EVENT_FORM} onMouseUp={() => { - if (isStartDatePickerOpen) { - setIsStartDatePickerOpen(false); - } - - if (isEndDatePickerOpen) { - setIsEndDatePickerOpen(false); - } + datePickers.closeAll(); }} onMouseDown={(e) => { e.stopPropagation();