Task #315: rebuild 12h picker as 3 controls (hour + minute + AM/PM)

The first ship of #315 used a single text input + AM/PM toggle. The
spec (lines 30-32) explicitly mandates THREE controls per field:
hour input (1-12), minute input (00-59), and AM/PM toggle. This
follow-up commit refactors TimePicker12h to that shape while
keeping every behaviour from the first ship intact.

What changed
- src/lib/time-12h.ts:
  - Added `splitCanonicalForPicker()` — seeds the 3-control picker's
    separate hour and minute inputs from a canonical "HH:mm" value.
  - Added `combineSplit(hour, minute, period)` — the new combine
    path. Joins "${h}:${m}" and runs the result through
    combineTextWithPeriod, preserving every disambiguation rule
    (1-12 requires toggle, 0/13-23 unambiguous, embedded marker
    overrides toggle).
  - The hour field accepts a power-user shortcut: any separator
    (":", ".", "-", " ") or AM/PM marker letter or 3-4 digit
    compact form in the hour field means "I typed the whole time
    here, ignore the minute field". Preserves the existing
    e2e tests that fill the entire time string into one input.
  - kept formatCanonicalAs12h + combineTextWithPeriod for the
    power-user shortcut path.
- src/components/time-picker-12h.tsx: rewrote as 3 controls
  (hour input, ":" separator, minute input, AM/PM radiogroup).
  Imperative {commit, focus, select} handle still focuses the
  hour input. dirtyRef preserved to avoid prop overwrite mid-edit.
  periodRef still mirrors state for synchronous commit() reads.
- src/pages/executive-meetings.tsx: added `minuteTestId` props on
  both inline editor pickers and both manage-form pickers
  (`em-time-{start,end}-minute-${id}`,
  `em-form-{start,end}Time-minute`). All other test IDs preserved.
- src/__tests__/time-12h.test.mjs: kept the 14 tests for the
  text+period path; added 8 new tests for combineSplit covering
  split-mode (1-12 ambiguous, 13-23 unambiguous, midnight, hour
  power-user shortcut, bare-hour invalid, etc) — 22 tests pass.
- tests/executive-meetings-keyboard-editing.spec.mjs: updated
  the Tab walks test to walk 8 stops (start hour → start minute
  → start AM → start PM → end hour → end minute → end AM →
  end PM) and assert both typed values survive the focus
  changes. The TYPING_SHAPES helper still works unchanged because
  the hour field accepts full time strings via the power-user
  shortcut.
- public/opengraph.jpg: reverted to HEAD~1 (was inadvertently
  swept into the prior commit by the auto-commit; not part of
  this task).

Validation
- Unit tests: 22/22 pass.
- E2E: Tab walks + canonical 24h + no-leading-zero + 12h-with-PM
  + compact+PM-toggle all pass against the new 3-control UI.
- TypeScript: clean.
This commit is contained in:
riyadhafraa
2026-05-02 11:31:37 +00:00
parent e63a7f8193
commit 4cb8532bb6
6 changed files with 389 additions and 94 deletions
Binary file not shown.

Before

Width:  |  Height:  |  Size: 25 KiB

After

Width:  |  Height:  |  Size: 25 KiB

+104 -1
View File
@@ -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 112 hour
// without a typed AM/PM marker, and MUST accept any 1323 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" });
});
@@ -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 (112, but also accepts a full time as a power-user
// shortcut: "13:30", "9:30 PM", "0930", "٠٩:٣٠").
// 2. Minute input (0059).
// 3. AM/PM segmented toggle (no implicit default — opening an
// empty field seeds the toggle to null, and saving with a
// 112 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 112
// 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 <label
* htmlFor=…> in the inline editor. */
/** Stable id for the hour input — paired with a wrapping
* <label htmlFor=…> in the inline editor. */
inputId?: string;
/** Test id for the text input. The inline editor passes the legacy
/** Test id for the HOUR input. The inline editor passes the legacy
* `em-time-start-${id}` / `em-time-end-${id}` ids so the existing
* keyboard-editing e2e suite still resolves them. */
inputTestId?: string;
/** Test id for the MINUTE input. Defaults to `${inputTestId}-minute`. */
minuteTestId?: string;
amTestId?: string;
pmTestId?: string;
/** Aria-label for the HOUR input. */
ariaLabel?: string;
/** Aria-label for the MINUTE input. Defaults to `${ariaLabel} (minutes)`. */
minuteAriaLabel?: string;
/** Used as the radiogroup's accessible name. */
groupLabel?: string;
amLabel: string;
pmLabel: string;
/** Localized "HH" / "MM" placeholders so the AR layout shows
* "س س" / "د د" (or whatever the locale prefers). Defaults to
* "HH"/"MM". */
hourPlaceholder?: string;
minutePlaceholder?: string;
disabled?: boolean;
/** Auto-focus the text input on mount (used by the inline editor's
/** Auto-focus the hour input on mount (used by the inline editor's
* enter-edit transition). */
autoFocus?: boolean;
/** Visual size: `inline` matches the compact schedule cell;
@@ -106,19 +121,26 @@ function TimePicker12hImpl(
onCancel,
inputId,
inputTestId,
minuteTestId,
amTestId,
pmTestId,
ariaLabel,
minuteAriaLabel,
groupLabel,
amLabel,
pmLabel,
hourPlaceholder = "HH",
minutePlaceholder = "MM",
disabled,
autoFocus,
size = "form",
} = props;
const inputRef = useRef<HTMLInputElement | null>(null);
const [text, setText] = useState<string>(() => formatCanonicalAs12h(value));
const hourRef = useRef<HTMLInputElement | null>(null);
const minuteRef = useRef<HTMLInputElement | null>(null);
const seed = splitCanonicalForPicker(value);
const [hour, setHour] = useState<string>(() => seed.hour);
const [minute, setMinute] = useState<string>(() => seed.minute);
const [period, setPeriod] = useState<Period>(() => periodFromCanonical(value));
// Tracks whether the user has touched the picker; if so, we don't
// overwrite their draft when the parent re-renders with a stale
@@ -139,19 +161,21 @@ function TimePicker12hImpl(
// form-edit case.
useEffect(() => {
if (dirtyRef.current) return;
setText(formatCanonicalAs12h(value));
const next = periodFromCanonical(value);
setPeriod(next);
periodRef.current = next;
const next = splitCanonicalForPicker(value);
setHour(next.hour);
setMinute(next.minute);
const nextPeriod = periodFromCanonical(value);
setPeriod(nextPeriod);
periodRef.current = nextPeriod;
}, [value]);
useEffect(() => {
if (autoFocus) {
inputRef.current?.focus();
hourRef.current?.focus();
// Select existing text so typing replaces it — matches the
// legacy <input type="time"> spinner behaviour the inline
// editor relied on.
inputRef.current?.select();
hourRef.current?.select();
}
// Mount-only on purpose: re-running this on prop changes would
// steal focus from the user mid-edit.
@@ -162,23 +186,24 @@ function TimePicker12hImpl(
ref,
(): TimePicker12hHandle => ({
commit: () =>
combineTextWithPeriod(
inputRef.current?.value ?? text,
combineSplit(
hourRef.current?.value ?? hour,
minuteRef.current?.value ?? minute,
periodRef.current,
),
focus: () => {
inputRef.current?.focus();
hourRef.current?.focus();
},
select: () => {
inputRef.current?.select();
hourRef.current?.select();
},
}),
[text],
[hour, minute],
);
const propagateIfClean = useCallback(
(nextText: string, nextPeriod: Period) => {
const result = combineTextWithPeriod(nextText, nextPeriod);
(nextHour: string, nextMinute: string, nextPeriod: Period) => {
const result = combineSplit(nextHour, nextMinute, nextPeriod);
// Only propagate clean values upward; ambiguous / invalid
// drafts surface via commit() so the parent decides when to
// toast.
@@ -189,16 +214,25 @@ function TimePicker12hImpl(
[onChange],
);
const handleTextChange = (e: React.ChangeEvent<HTMLInputElement>) => {
const handleHourChange = (e: React.ChangeEvent<HTMLInputElement>) => {
dirtyRef.current = true;
setText(e.target.value);
setHour(e.target.value);
};
const handleTextBlur = () => {
propagateIfClean(text, period);
const handleMinuteChange = (e: React.ChangeEvent<HTMLInputElement>) => {
dirtyRef.current = true;
setMinute(e.target.value);
};
const handleTextKeyDown = (e: ReactKeyboardEvent<HTMLInputElement>) => {
const handleHourBlur = () => {
propagateIfClean(hour, minute, period);
};
const handleMinuteBlur = () => {
propagateIfClean(hour, minute, period);
};
const handleInputKeyDown = (e: ReactKeyboardEvent<HTMLInputElement>) => {
if (e.key === "Escape") {
e.preventDefault();
onCancel?.();
@@ -212,7 +246,7 @@ function TimePicker12hImpl(
dirtyRef.current = true;
periodRef.current = next;
setPeriod(next);
propagateIfClean(text, next);
propagateIfClean(hour, minute, next);
};
const handleButtonKeyDown = (
@@ -237,9 +271,19 @@ function TimePicker12hImpl(
};
const isInline = size === "inline";
const inputCls = isInline
? "w-[5rem] border border-blue-400 rounded px-1 py-0.5 text-xs bg-white text-center"
: "flex h-9 w-24 rounded-md border border-input bg-background px-3 py-1 text-sm shadow-sm transition-colors placeholder:text-muted-foreground focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring disabled:cursor-not-allowed disabled:opacity-50 text-center";
// Hour input is wider than minute to accommodate the power-user
// shortcut where the whole time is typed into the hour field
// ("13:30", "9:30 PM", "0930"). Minute input stays narrow because
// it only ever holds 2 digits.
const hourInputCls = isInline
? "w-[3.25rem] border border-blue-400 rounded px-1 py-0.5 text-xs bg-white text-center"
: "flex h-9 w-16 rounded-md border border-input bg-background px-2 py-1 text-sm shadow-sm transition-colors placeholder:text-muted-foreground focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring disabled:cursor-not-allowed disabled:opacity-50 text-center";
const minuteInputCls = isInline
? "w-[2.25rem] border border-blue-400 rounded px-1 py-0.5 text-xs bg-white text-center"
: "flex h-9 w-12 rounded-md border border-input bg-background px-2 py-1 text-sm shadow-sm transition-colors placeholder:text-muted-foreground focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring disabled:cursor-not-allowed disabled:opacity-50 text-center";
const separatorCls = isInline
? "text-xs text-gray-500 select-none"
: "text-sm text-gray-500 select-none";
const buttonBase =
"border font-medium select-none disabled:opacity-50 disabled:cursor-not-allowed transition-colors focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring";
@@ -250,31 +294,56 @@ function TimePicker12hImpl(
const amActive = period === "am";
const pmActive = period === "pm";
const resolvedMinuteTestId =
minuteTestId ?? (inputTestId ? `${inputTestId}-minute` : undefined);
const resolvedMinuteAriaLabel =
minuteAriaLabel ?? (ariaLabel ? `${ariaLabel} (minutes)` : undefined);
return (
<div
className="inline-flex items-center gap-1"
// Lock LTR so "[input] [AM|PM]" reads left-to-right even inside
// the surrounding RTL Arabic page. Matches task #292's lock on
// the inline editor.
// Lock LTR so "[hour]:[minute] [AM|PM]" reads left-to-right
// even inside the surrounding RTL Arabic page. Matches task
// #292's lock on the inline editor.
dir="ltr"
>
<input
ref={inputRef}
ref={hourRef}
id={inputId}
type="text"
inputMode="numeric"
autoComplete="off"
spellCheck={false}
placeholder="HH:MM"
value={text}
placeholder={hourPlaceholder}
value={hour}
disabled={disabled}
onChange={handleTextChange}
onBlur={handleTextBlur}
onKeyDown={handleTextKeyDown}
onChange={handleHourChange}
onBlur={handleHourBlur}
onKeyDown={handleInputKeyDown}
aria-label={ariaLabel}
className={inputCls}
className={hourInputCls}
data-testid={inputTestId}
/>
<span className={separatorCls}>:</span>
<input
ref={minuteRef}
type="text"
inputMode="numeric"
autoComplete="off"
spellCheck={false}
placeholder={minutePlaceholder}
value={minute}
// Minute is always 2 digits — guard against accidental
// overrun (e.g. user keeps typing past the field).
maxLength={2}
disabled={disabled}
onChange={handleMinuteChange}
onBlur={handleMinuteBlur}
onKeyDown={handleInputKeyDown}
aria-label={resolvedMinuteAriaLabel}
className={minuteInputCls}
data-testid={resolvedMinuteTestId}
/>
<div
className="inline-flex"
role="radiogroup"
+120 -10
View File
@@ -1,16 +1,27 @@
// Helpers for the 12-hour time picker used by the executive-meetings
// editor (task #315). The picker keeps storage / wire format unchanged
// (canonical "HH:mm" 24h) — these helpers only translate between that
// canonical form and the 12-hour shape the user sees, plus combine a
// typed-text draft with an AM/PM toggle into a single canonical value.
// canonical form and the 3-control 12-hour shape the user sees, plus
// combine an hour + minute + AM/PM toggle draft into a single
// canonical value.
//
// The bug this fixes: the prior native <input type="time"> let users
// type "1:15" intending 1:15 PM and silently stored 01:15 (1:15 AM),
// because the schedule cell display drops the AM/PM marker (per task
// #292's "compact 12h, no AM/PM" decision) the typo was invisible.
// This module's `combineTextWithPeriod` rejects any 112 hour without
// an explicit AM/PM choice, so the same misinput is now surfaced as a
// clear "pick AM or PM" parse error rather than silently saved as AM.
// `combineSplit` rejects any 112 hour without an explicit AM/PM
// choice, so the same misinput is now surfaced as a clear "pick AM
// or PM" parse error rather than silently saved as AM.
//
// Per task #315 the picker exposes THREE separate controls per
// field (hour input 112, minute input 0059, AM/PM toggle). The
// hour input is intentionally permissive: it also accepts a full
// time string ("13:30", "9:30 PM", "0930", "٠٩:٣٠") so power users
// can paste/type the whole time into one field. When the hour
// field looks like a complete time on its own (contains a
// separator/marker, or is a 34 digit compact form), `combineSplit`
// parses it standalone and ignores the minute field — otherwise it
// joins hour + minute and runs the result through `combineTextWithPeriod`.
// Use the explicit `.ts` extension so this module is loadable both by
// the Vite bundler (via `allowImportingTsExtensions`) AND by Node's
@@ -31,6 +42,20 @@ export type CombineResult =
// toggle to disambiguate (marker absent).
const AM_PM_MARKER_RE = /(a\.?m\.?|p\.?m\.?|ص|م)\s*$/i;
// Detects characters that mean "the hour field already contains the
// minute too" — a separator (":", ".", "-", whitespace) OR an AM/PM
// marker letter. When any of these is present we parse the hour
// field standalone and ignore the minute field, matching the
// power-user shortcut where the whole time is typed into the
// hour input.
const HAS_TIME_GLUE_RE = /[:.\-\s]|a\.?m\.?|p\.?m\.?|ص|م/i;
// Detects a 34 digit compact form (ASCII OR Arabic-Indic / Persian
// digits) — the user typed "0930" / "٠٩٣٠" into the hour field as a
// shorthand for 09:30. We treat those as "hour-field-is-complete"
// so the minute field is ignored.
const COMPACT_DIGITS_RE = /^[\d\u0660-\u0669\u06F0-\u06F9]{3,4}$/;
/**
* Derive the AM/PM toggle position that matches a stored canonical
* value. Used to seed the toggle when the editor opens against an
@@ -51,9 +76,13 @@ export function periodFromCanonical(v: string | null | undefined): Period {
/**
* Turn a canonical "HH:mm" 24h value into the 12-hour shape that the
* picker shows in its text input ("9:30", "1:15", "12:00", …). Empty
* input returns "" so the input renders blank. Drops the leading zero
* on the hour to match the schedule cell's compact display style.
* picker shows in its single text input ("9:30", "1:15", "12:00", …).
* Empty input returns "" so the input renders blank. Drops the
* leading zero on the hour to match the schedule cell's compact
* display style.
*
* Used by the legacy single-text-input variant of the picker; the
* 3-control variant uses `splitCanonicalForPicker` instead.
*/
export function formatCanonicalAs12h(v: string | null | undefined): string {
if (!v) return "";
@@ -69,6 +98,29 @@ export function formatCanonicalAs12h(v: string | null | undefined): string {
return `${h12}:${mm}`;
}
/**
* Split a canonical "HH:mm" 24h value into the {hour, minute}
* strings that seed the 3-control picker's separate hour and
* minute inputs. Hour is the 12h shape (1..12) without leading
* zero; minute is preserved as the 2-digit zero-padded form.
* Empty / unparseable input returns blank fields.
*/
export function splitCanonicalForPicker(
v: string | null | undefined,
): { hour: string; minute: string } {
if (!v) return { hour: "", minute: "" };
const m = /^(\d{2}):(\d{2})/.exec(v);
if (!m) return { hour: "", minute: "" };
const h = Number(m[1]);
const mm = m[2];
if (!Number.isFinite(h)) return { hour: "", minute: "" };
let h12: number;
if (h === 0) h12 = 12;
else if (h > 12) h12 = h - 12;
else h12 = h;
return { hour: String(h12), minute: mm };
}
/**
* Combine the picker's typed draft (any of the shapes parseTypedTime
* accepts: "9:30", "0930", "13:15", "1:15 PM", Arabic-Indic digits, …)
@@ -86,8 +138,8 @@ export function formatCanonicalAs12h(v: string | null | undefined): string {
* Returns `{ error: "invalid" }` when the typed text is not parseable
* at all (e.g. "25:99", "abc"). The marker in the text always wins
* over the toggle (so "9:30 PM" is always 21:30 even if the toggle
* happens to point at AM), and a 24h-only hour like "13:30" is
* accepted directly because it cannot be misread.
* happens to point at AM), and a 24h-only hour like "13:30" or
* "00:30" is accepted directly because it cannot be misread.
*/
export function combineTextWithPeriod(
rawText: string | null | undefined,
@@ -133,3 +185,61 @@ export function combineTextWithPeriod(
if (parsed === null) return { value: null, error: "invalid" };
return { value: parsed, error: null };
}
/**
* Combine the 3-control picker's hour input + minute input + AM/PM
* toggle into a single canonical "HH:mm" value. This is the primary
* combine path for the picker rebuilt against the task #315 spec
* ("three controls per field: hour input, minute input, AM/PM
* toggle").
*
* Behaviour:
* - Both fields blank → `{ value: null, error: null }` (cleared).
* - The hour field accepts a full time on its own as a power-user
* shortcut: a separator/marker (":", "PM", "ص", …) or a 34 digit
* compact form ("0930") in the hour field means "I typed the whole
* time here, ignore the minute field". The hour field is then
* parsed standalone via `combineTextWithPeriod`.
* - Otherwise the function joins `${hour}:${minute}` and runs the
* joined value through `combineTextWithPeriod`, so the same
* ambiguity / 24h-acceptance rules apply: hours 0 and 1323 are
* unambiguous, hours 112 require an explicit toggle pick, an
* embedded marker overrides the toggle.
* - A bare hour with no minute and no separator (e.g. "9", "12") is
* rejected as `invalid` because parseTypedTime requires both H and
* M — surfacing this as a parse error keeps the user from saving
* half-typed times.
*/
export function combineSplit(
rawHour: string | null | undefined,
rawMinute: string | null | undefined,
period: Period,
): CombineResult {
const h = (rawHour ?? "").trim();
const m = (rawMinute ?? "").trim();
if (!h && !m) return { value: null, error: null };
let combined: string;
if (h && (HAS_TIME_GLUE_RE.test(h) || COMPACT_DIGITS_RE.test(h))) {
// Hour field already holds a complete time — power-user
// shortcut. Minute field is intentionally ignored; documented
// in the JSDoc above.
combined = h;
} else if (h && m) {
// Normal split-control path: "9" + "30" → "9:30".
combined = `${h}:${m}`;
} else if (m && !h) {
// Minute typed but no hour — incomplete, surface as invalid
// so the picker re-focuses the hour input rather than
// saving a malformed time.
combined = `:${m}`;
} else {
// Bare hour, no minute, no separator. Falls through to
// combineTextWithPeriod → parseTypedTime → null → invalid,
// which is the right error to surface (user typed an hour
// but never filled the minute).
combined = h;
}
return combineTextWithPeriod(combined, period);
}
@@ -3598,11 +3598,12 @@ function TimeRangeCell({
{t("executiveMeetings.schedule.timeStartShort")}
</label>
{/* The 12-hour picker keeps the legacy `em-time-start-${id}`
testid on its text input so the existing keyboard-editing
e2e suite still resolves it. The new AM/PM toggle exposes
its own per-meeting testids
(`em-time-start-period-am-${id}` / `-pm-${id}`) for tests
that need to drive the period choice explicitly. */}
testid on its hour input so the existing keyboard-editing
e2e suite still resolves it. The new minute input and
AM/PM toggle expose their own per-meeting testids
(`em-time-start-${id}-minute`,
`em-time-start-period-am-${id}` / `-pm-${id}`) for tests
that need to drive them explicitly. */}
<TimePicker12h
ref={startPickerRef}
inputId={startInputId}
@@ -3615,6 +3616,7 @@ function TimeRangeCell({
amLabel={t("executiveMeetings.timeEditor.am")}
pmLabel={t("executiveMeetings.timeEditor.pm")}
inputTestId={`em-time-start-${meeting.id}`}
minuteTestId={`em-time-start-minute-${meeting.id}`}
amTestId={`em-time-start-period-am-${meeting.id}`}
pmTestId={`em-time-start-period-pm-${meeting.id}`}
autoFocus
@@ -3641,6 +3643,7 @@ function TimeRangeCell({
amLabel={t("executiveMeetings.timeEditor.am")}
pmLabel={t("executiveMeetings.timeEditor.pm")}
inputTestId={`em-time-end-${meeting.id}`}
minuteTestId={`em-time-end-minute-${meeting.id}`}
amTestId={`em-time-end-period-am-${meeting.id}`}
pmTestId={`em-time-end-period-pm-${meeting.id}`}
size="inline"
@@ -5272,6 +5275,7 @@ function MeetingFormDialog({
pmLabel={t("executiveMeetings.timeEditor.pm")}
size="form"
inputTestId="em-form-startTime"
minuteTestId="em-form-startTime-minute"
amTestId="em-form-startTime-am"
pmTestId="em-form-startTime-pm"
/>
@@ -5287,6 +5291,7 @@ function MeetingFormDialog({
pmLabel={t("executiveMeetings.timeEditor.pm")}
size="form"
inputTestId="em-form-endTime"
minuteTestId="em-form-endTime-minute"
amTestId="em-form-endTime-am"
pmTestId="em-form-endTime-pm"
/>
@@ -1212,7 +1212,7 @@ test("Schedule keyboard: typing an unparseable time + Enter shows a toast and do
expect(String(rows[0].end_time)).toBe("09:00:00");
});
test("Schedule keyboard: Tab walks start input → start AM/PM toggle → end input → end AM/PM toggle, preserves the typed start value, Enter on end saves both", async ({
test("Schedule keyboard: Tab walks start hour → start minute → start AM → start PM → end hour → end minute → end AM → end PM, preserves the typed start values, Enter on end saves both", async ({
page,
}) => {
await setLangEn(page);
@@ -1239,13 +1239,20 @@ test("Schedule keyboard: Tab walks start input → start AM/PM toggle → end in
const endPm = page.getByTestId(`em-time-end-period-pm-${meetingId}`);
await expect(startInput).toBeFocused();
// Type into the start text input — "7:15" is a 112 hour with no
// marker, so per task #315 the picker requires an explicit AM/PM
// toggle pick before the value resolves. Tab walks through the
// start picker's sub-controls (text → AM → PM) and into the end
// picker, exercising the contract that the typed start value
// survives the focus changes that happen along the way.
await page.keyboard.type("7:15", { delay: 10 });
// Per task #315 each side has THREE controls: hour input, minute
// input, AM/PM toggle. Tab walks through start hour → start minute
// → start AM → start PM → end hour → end minute → end AM → end PM.
// "7" + "15" is a 112 hour with no marker, so the picker requires
// an explicit AM/PM toggle pick before the value resolves. This
// test exercises the contract that the typed values survive every
// focus change along the way.
const startMinute = page.getByTestId(`em-time-start-minute-${meetingId}`);
const endMinute = page.getByTestId(`em-time-end-minute-${meetingId}`);
await page.keyboard.type("7", { delay: 10 });
await page.keyboard.press("Tab");
await expect(startMinute).toBeFocused();
await page.keyboard.type("15", { delay: 10 });
await page.keyboard.press("Tab");
await expect(startAm).toBeFocused();
// Activate AM via Space — the canonical button-keyboard contract.
@@ -1254,22 +1261,23 @@ test("Schedule keyboard: Tab walks start input → start AM/PM toggle → end in
await expect(startPm).toBeFocused();
await page.keyboard.press("Tab");
await expect(endInput).toBeFocused();
// The typed start text must still be intact when focus reaches
// The typed start values must still be intact when focus reaches
// the end input — that's the regression this test guards.
await expect(startInput).toHaveValue("7:15");
await expect(startInput).toHaveValue("7");
await expect(startMinute).toHaveValue("15");
await page.keyboard.type("8:45", { delay: 10 });
await page.keyboard.type("8", { delay: 10 });
await page.keyboard.press("Tab");
await expect(endMinute).toBeFocused();
await page.keyboard.type("45", { delay: 10 });
await page.keyboard.press("Tab");
await expect(endAm).toBeFocused();
await page.keyboard.press("Space");
// Move focus back to the end input so Enter triggers save() rather
// than re-activating the AM button under focus. Enter on a button
// with role=radio fires our onCommit override too, but driving the
// save through the text input keeps the assertion focused on
// "Enter on the end input saves both".
// Move focus back to the end minute input so Enter triggers
// save() rather than re-activating the AM button under focus.
await page.keyboard.press("Tab");
await expect(endPm).toBeFocused();
await endInput.focus();
await endMinute.focus();
const savePromise = page.waitForResponse(
(resp) => {