diff --git a/artifacts/tx-os/public/opengraph.jpg b/artifacts/tx-os/public/opengraph.jpg index d0db04b6..d08d6341 100644 Binary files a/artifacts/tx-os/public/opengraph.jpg and b/artifacts/tx-os/public/opengraph.jpg differ diff --git a/artifacts/tx-os/src/__tests__/time-12h.test.mjs b/artifacts/tx-os/src/__tests__/time-12h.test.mjs index 9be5722a..355605dd 100644 --- a/artifacts/tx-os/src/__tests__/time-12h.test.mjs +++ b/artifacts/tx-os/src/__tests__/time-12h.test.mjs @@ -2,14 +2,17 @@ // AM/PM disambiguation rules that fix the "1:15 silently saved as AM" // bug — the picker MUST require an explicit toggle for any 1–12 hour // without a typed AM/PM marker, and MUST accept any 13–23 hour or -// any explicit-marker shape directly. +// any explicit-marker shape directly. Also covers the 3-control +// `combineSplit` path that joins separate hour + minute fields. import { test } from "node:test"; import assert from "node:assert/strict"; import { + combineSplit, combineTextWithPeriod, formatCanonicalAs12h, periodFromCanonical, + splitCanonicalForPicker, } from "../lib/time-12h.ts"; test("periodFromCanonical: empty / unparseable returns null (no implicit default)", () => { @@ -46,6 +49,23 @@ test("formatCanonicalAs12h: 24h hours collapse to 1..12", () => { assert.equal(formatCanonicalAs12h("23:59"), "11:59"); }); +test("splitCanonicalForPicker: empty / unparseable returns blank fields", () => { + assert.deepEqual(splitCanonicalForPicker(""), { hour: "", minute: "" }); + assert.deepEqual(splitCanonicalForPicker(null), { hour: "", minute: "" }); + assert.deepEqual(splitCanonicalForPicker(undefined), { hour: "", minute: "" }); + assert.deepEqual(splitCanonicalForPicker("not-a-time"), { hour: "", minute: "" }); +}); + +test("splitCanonicalForPicker: seeds hour (1..12, no leading zero) and minute (zero-padded)", () => { + assert.deepEqual(splitCanonicalForPicker("00:30"), { hour: "12", minute: "30" }); + assert.deepEqual(splitCanonicalForPicker("00:00"), { hour: "12", minute: "00" }); + assert.deepEqual(splitCanonicalForPicker("01:15"), { hour: "1", minute: "15" }); + assert.deepEqual(splitCanonicalForPicker("09:30"), { hour: "9", minute: "30" }); + assert.deepEqual(splitCanonicalForPicker("12:00"), { hour: "12", minute: "00" }); + assert.deepEqual(splitCanonicalForPicker("13:30"), { hour: "1", minute: "30" }); + assert.deepEqual(splitCanonicalForPicker("23:59"), { hour: "11", minute: "59" }); +}); + test("combineTextWithPeriod: empty input is { value: null, error: null }", () => { assert.deepEqual(combineTextWithPeriod("", null), { value: null, error: null }); assert.deepEqual(combineTextWithPeriod(" ", "am"), { value: null, error: null }); @@ -135,3 +155,86 @@ test("combineTextWithPeriod: marker in text + invalid hour is invalid", () => { error: "invalid", }); }); + +// ------------------------------------------------------------------- +// combineSplit — the 3-control path (hour input + minute input + AM/PM toggle) +// ------------------------------------------------------------------- + +test("combineSplit: both fields blank is { value: null, error: null }", () => { + assert.deepEqual(combineSplit("", "", null), { value: null, error: null }); + assert.deepEqual(combineSplit(" ", " ", "am"), { value: null, error: null }); + assert.deepEqual(combineSplit(null, undefined, "pm"), { value: null, error: null }); +}); + +test("combineSplit: split-mode (hour + minute typed separately) requires period for 1..12", () => { + // The canonical 3-control flow: hour=9, minute=30, no toggle pick + // → ambiguous (the bug fix). + assert.deepEqual(combineSplit("9", "30", null), { value: null, error: "ambiguous" }); + assert.deepEqual(combineSplit("1", "15", null), { value: null, error: "ambiguous" }); + assert.deepEqual(combineSplit("12", "00", null), { value: null, error: "ambiguous" }); + // With AM toggle → AM resolution. + assert.deepEqual(combineSplit("9", "30", "am"), { value: "09:30", error: null }); + assert.deepEqual(combineSplit("1", "15", "am"), { value: "01:15", error: null }); + // With PM toggle → PM resolution. + assert.deepEqual(combineSplit("9", "30", "pm"), { value: "21:30", error: null }); + assert.deepEqual(combineSplit("1", "15", "pm"), { value: "13:15", error: null }); + // 12 AM → 00:xx, 12 PM → 12:xx. + assert.deepEqual(combineSplit("12", "00", "am"), { value: "00:00", error: null }); + assert.deepEqual(combineSplit("12", "30", "am"), { value: "00:30", error: null }); + assert.deepEqual(combineSplit("12", "00", "pm"), { value: "12:00", error: null }); +}); + +test("combineSplit: split-mode 13..23 hour is unambiguous, accepted without a toggle", () => { + // User who knows the 24h time of their meeting can type "13" + "30" + // and save without also tapping PM — same convenience as the + // single-text path. + assert.deepEqual(combineSplit("13", "30", null), { value: "13:30", error: null }); + assert.deepEqual(combineSplit("23", "59", null), { value: "23:59", error: null }); + // Toggle is irrelevant for 24h-only hours. + assert.deepEqual(combineSplit("13", "30", "am"), { value: "13:30", error: null }); +}); + +test("combineSplit: split-mode hour 0 (midnight) is unambiguous, accepted without a toggle", () => { + assert.deepEqual(combineSplit("00", "30", null), { value: "00:30", error: null }); + assert.deepEqual(combineSplit("00", "00", null), { value: "00:00", error: null }); +}); + +test("combineSplit: hour-field power-user shortcut (whole time in hour input)", () => { + // Hour field contains a separator → parsed standalone, minute + // field ignored. This preserves the e2e tests that fill the + // entire time string into the hour input. + assert.deepEqual(combineSplit("13:30", "", null), { value: "13:30", error: null }); + assert.deepEqual(combineSplit("9:30", "", "pm"), { value: "21:30", error: null }); + // Marker wins over toggle. + assert.deepEqual(combineSplit("9:30 PM", "", null), { value: "21:30", error: null }); + assert.deepEqual(combineSplit("9:30 AM", "", "pm"), { value: "09:30", error: null }); + // Compact 4-digit form. + assert.deepEqual(combineSplit("0930", "", "am"), { value: "09:30", error: null }); + assert.deepEqual(combineSplit("0115", "", "pm"), { value: "13:15", error: null }); + // Compact 3-digit form ("930" → 9:30). + assert.deepEqual(combineSplit("930", "", "am"), { value: "09:30", error: null }); + // Arabic-Indic compact form. + assert.deepEqual(combineSplit("٠٩٣٠", "", "am"), { value: "09:30", error: null }); + // Even when the minute field IS filled, the hour-field power-user + // shortcut wins (separator in hour signals "I typed the whole time + // here, ignore the minute field"). + assert.deepEqual(combineSplit("13:30", "99", null), { value: "13:30", error: null }); +}); + +test("combineSplit: bare hour with no minute is invalid", () => { + // User typed an hour but never filled the minute — surface as + // invalid so the picker re-focuses the minute input rather than + // saving a malformed value. + assert.deepEqual(combineSplit("9", "", "am"), { value: null, error: "invalid" }); + assert.deepEqual(combineSplit("12", "", "pm"), { value: null, error: "invalid" }); +}); + +test("combineSplit: minute-only with no hour is invalid", () => { + assert.deepEqual(combineSplit("", "30", "am"), { value: null, error: "invalid" }); +}); + +test("combineSplit: invalid numeric values are invalid", () => { + assert.deepEqual(combineSplit("25", "99", null), { value: null, error: "invalid" }); + assert.deepEqual(combineSplit("abc", "30", "am"), { value: null, error: "invalid" }); + assert.deepEqual(combineSplit("9", "60", "am"), { value: null, error: "invalid" }); +}); diff --git a/artifacts/tx-os/src/components/time-picker-12h.tsx b/artifacts/tx-os/src/components/time-picker-12h.tsx index c72fb870..e04c9a7b 100644 --- a/artifacts/tx-os/src/components/time-picker-12h.tsx +++ b/artifacts/tx-os/src/components/time-picker-12h.tsx @@ -1,26 +1,30 @@ // 12-hour time picker for the executive-meetings editor (task #315). // -// Renders a free-text time input ("HH:MM", accepts the same shapes -// parseTypedTime accepts) plus an AM/PM segmented toggle. Storage -// stays canonical "HH:mm" 24h; this component is purely a UI shell -// around `combineTextWithPeriod` from `@/lib/time-12h`. +// Renders THREE controls per field, per the task spec: +// 1. Hour input (1–12, but also accepts a full time as a power-user +// shortcut: "13:30", "9:30 PM", "0930", "٠٩:٣٠"). +// 2. Minute input (00–59). +// 3. AM/PM segmented toggle (no implicit default — opening an +// empty field seeds the toggle to null, and saving with a +// 1–12 typed hour but no toggle pick surfaces an "ambiguous" +// parse error to the parent rather than silently committing +// AM). +// +// Storage stays canonical "HH:mm" 24h; this component is purely a UI +// shell around `combineSplit` from `@/lib/time-12h`. // // Two consumers: // 1) The Manage-tab edit form: the picker is uncontrolled aside // from `value` / `onChange` (clean canonical values flow up on // blur and on toggle change; ambiguous drafts stay inside the -// picker until the user resolves them). +// picker until the user resolves them, and the dialog's Save +// button calls `commit()` to validate before persisting). // 2) The inline schedule time-cell editor (TimeRangeCell): the // parent reads the latest draft via the imperative `commit()` // handle when the user presses Enter or blurs the wrapper, so // a save that races React state batching still sees the right // value (matches the belt-and-braces pattern task #308 set up // against the prior text input). -// -// The AM/PM toggle has NO implicit default — opening the picker -// against an empty value seeds period=null, and saving with a 1–12 -// typed hour but no toggle pick surfaces an "ambiguous" parse error -// to the parent rather than silently committing AM. import { forwardRef, @@ -33,9 +37,9 @@ import { type KeyboardEvent as ReactKeyboardEvent, } from "react"; import { - combineTextWithPeriod, - formatCanonicalAs12h, + combineSplit, periodFromCanonical, + splitCanonicalForPicker, type CombineResult, type Period, } from "@/lib/time-12h"; @@ -43,15 +47,16 @@ import { export type TimePicker12hHandle = { /** * Compute the canonical "HH:mm" 24h string from the picker's current - * draft (typed text + toggle) WITHOUT waiting for an onChange - * round-trip. Used by the inline schedule editor's save path so a - * save triggered by Enter or wrapper-blur reads the live value the - * user sees in the input. + * draft (typed hour + minute + toggle) WITHOUT waiting for an + * onChange round-trip. Used by the inline schedule editor's save + * path so a save triggered by Enter or wrapper-blur reads the live + * value the user sees in the inputs, and by the Manage form's + * Save button to validate before persisting. */ commit(): CombineResult; - /** Move keyboard focus into the time text input. */ + /** Move keyboard focus into the hour input. */ focus(): void; - /** Select the existing text inside the time input. */ + /** Select the existing text inside the hour input. */ select(): void; }; @@ -61,33 +66,43 @@ type Props = { /** Canonical "HH:mm" 24h string, or "" for empty. */ value: string; /** - * Fires when the user lands on a clean canonical value: text input - * blurred with valid input, AM/PM toggled with valid input, or - * cleared. Ambiguous / invalid mid-edit drafts are NOT propagated - * — the parent learns about those via `commit()` from the imperative - * handle. + * Fires when the user lands on a clean canonical value: any of the + * sub-controls blurred with valid input, AM/PM toggled with valid + * input, or cleared. Ambiguous / invalid mid-edit drafts are NOT + * propagated — the parent learns about those via `commit()` from + * the imperative handle. */ onChange?: (canonical: string) => void; /** Enter pressed inside any sub-control. */ onCommit?: () => void; /** Escape pressed inside any sub-control. */ onCancel?: () => void; - /** Stable id for the text input — paired with a wrapping