Task #315: 12h time picker with explicit AM/PM (executive meetings)
Replaces native <input type="time"> in the executive-meetings editor with a TimePicker12h that always requires an explicit AM/PM choice for any 1–12 hour, fixing the "1:15 silently saved as 01:15 (AM) when user meant 13:15 (PM)" bug. Display stays compact 12h with no AM/PM marker (per #292); wire format remains canonical HH:mm 24h. What ships - src/lib/time-12h.ts: periodFromCanonical, formatCanonicalAs12h, combineTextWithPeriod (returns "ambiguous" for any unmarked 1–12 hour without a toggle pick; accepts hours 0 and 13–23 directly because they are unambiguous 24h shapes; typed marker always wins over toggle). - src/components/time-picker-12h.tsx: forwardRef component with imperative {commit, focus, select} handle, dirtyRef to avoid prop overwrite mid-edit, dir="ltr" for stable RTL layout, and Enter-on-toggle = "set AND save". - Wired into TimeRangeCell inline editor (commit-based save handles invalid/ambiguous/TimeOrderError as toasts and refocuses). - Wired into MeetingFormDialog with picker refs + handleSaveClick validation: Save button now calls commit() on both pickers, blocks on invalid/ambiguous (toast + focus offending picker), and passes canonical values straight to the parent's save handler so a stale React state batch from onChange-suppression cannot silently persist the previous value. Tests - 12 unit tests in src/__tests__/time-12h.test.mjs (incl. new "hour 0 is unambiguous, accepted without a toggle"). - 17 e2e tests in tests/executive-meetings-keyboard-editing.spec.mjs (incl. "compact + PM toggle" 0115→13:15 and "Tab walks start input → start AM/PM toggle → end input → end AM/PM toggle"). - Existing manage-create e2e suite still green. Locale keys (en + ar) - executiveMeetings.timeEditor.{am,pm,periodGroupStart,periodGroupEnd} - executiveMeetings.schedule.timeAmbiguousError Deviations from plan - Architect review #1 caught two issues that were both fixed: (1) combineTextWithPeriod treated hour 0 as ambiguous; now accepts 00:xx unambiguously. (2) MeetingFormDialog initially used pickers via value/onChange only, which left a silent-fallback hole because onChange suppresses ambiguous drafts. Added imperative refs + handleSaveClick validation; onSave signature changed to (committedStart, committedEnd) so the parent's save() can use the freshly-committed values directly. - Reverted accidental vite.config.ts typo (@replit/... was missing the leading @).
This commit is contained in:
riyadhafraa
Binary file not shown.
|
Before Width: | Height: | Size: 25 KiB After Width: | Height: | Size: 25 KiB |
@@ -0,0 +1,137 @@
|
||||
// Unit tests for the 12-hour-picker helpers (task #315). Covers the
|
||||
// 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.
|
||||
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import {
|
||||
combineTextWithPeriod,
|
||||
formatCanonicalAs12h,
|
||||
periodFromCanonical,
|
||||
} from "../lib/time-12h.ts";
|
||||
|
||||
test("periodFromCanonical: empty / unparseable returns null (no implicit default)", () => {
|
||||
assert.equal(periodFromCanonical(""), null);
|
||||
assert.equal(periodFromCanonical(null), null);
|
||||
assert.equal(periodFromCanonical(undefined), null);
|
||||
assert.equal(periodFromCanonical("not-a-time"), null);
|
||||
});
|
||||
|
||||
test("periodFromCanonical: midnight is AM, noon is PM", () => {
|
||||
assert.equal(periodFromCanonical("00:00"), "am");
|
||||
assert.equal(periodFromCanonical("00:30"), "am");
|
||||
assert.equal(periodFromCanonical("11:59"), "am");
|
||||
assert.equal(periodFromCanonical("12:00"), "pm");
|
||||
assert.equal(periodFromCanonical("12:30"), "pm");
|
||||
assert.equal(periodFromCanonical("13:00"), "pm");
|
||||
assert.equal(periodFromCanonical("23:59"), "pm");
|
||||
});
|
||||
|
||||
test("formatCanonicalAs12h: empty in, empty out", () => {
|
||||
assert.equal(formatCanonicalAs12h(""), "");
|
||||
assert.equal(formatCanonicalAs12h(null), "");
|
||||
assert.equal(formatCanonicalAs12h(undefined), "");
|
||||
});
|
||||
|
||||
test("formatCanonicalAs12h: 24h hours collapse to 1..12", () => {
|
||||
assert.equal(formatCanonicalAs12h("00:30"), "12:30"); // 12:30 AM
|
||||
assert.equal(formatCanonicalAs12h("00:00"), "12:00"); // 12:00 AM (midnight)
|
||||
assert.equal(formatCanonicalAs12h("01:15"), "1:15");
|
||||
assert.equal(formatCanonicalAs12h("09:30"), "9:30");
|
||||
assert.equal(formatCanonicalAs12h("12:00"), "12:00"); // 12:00 PM (noon)
|
||||
assert.equal(formatCanonicalAs12h("12:45"), "12:45");
|
||||
assert.equal(formatCanonicalAs12h("13:30"), "1:30");
|
||||
assert.equal(formatCanonicalAs12h("23:59"), "11: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 });
|
||||
assert.deepEqual(combineTextWithPeriod(null, "pm"), { value: null, error: null });
|
||||
});
|
||||
|
||||
test("combineTextWithPeriod: 1..12 hour with no toggle is ambiguous (the bug fix)", () => {
|
||||
// The exact shape that historically caused the silent "1:15 saved
|
||||
// as 01:15 AM" misinput.
|
||||
assert.deepEqual(combineTextWithPeriod("1:15", null), {
|
||||
value: null,
|
||||
error: "ambiguous",
|
||||
});
|
||||
assert.deepEqual(combineTextWithPeriod("9:30", null), {
|
||||
value: null,
|
||||
error: "ambiguous",
|
||||
});
|
||||
assert.deepEqual(combineTextWithPeriod("0930", null), {
|
||||
value: null,
|
||||
error: "ambiguous",
|
||||
});
|
||||
assert.deepEqual(combineTextWithPeriod("12:00", null), {
|
||||
value: null,
|
||||
error: "ambiguous",
|
||||
});
|
||||
});
|
||||
|
||||
test("combineTextWithPeriod: 1..12 hour with toggle resolves correctly", () => {
|
||||
assert.deepEqual(combineTextWithPeriod("1:15", "am"), { value: "01:15", error: null });
|
||||
assert.deepEqual(combineTextWithPeriod("1:15", "pm"), { value: "13:15", error: null });
|
||||
assert.deepEqual(combineTextWithPeriod("9:30", "am"), { value: "09:30", error: null });
|
||||
assert.deepEqual(combineTextWithPeriod("9:30", "pm"), { value: "21:30", error: null });
|
||||
// Compact shapes from the task's "Done looks like" list.
|
||||
assert.deepEqual(combineTextWithPeriod("0115", "pm"), { value: "13:15", error: null });
|
||||
assert.deepEqual(combineTextWithPeriod("0930", "am"), { value: "09:30", error: null });
|
||||
// 12 AM → 00:xx, 12 PM → 12:xx.
|
||||
assert.deepEqual(combineTextWithPeriod("12:00", "am"), { value: "00:00", error: null });
|
||||
assert.deepEqual(combineTextWithPeriod("12:30", "am"), { value: "00:30", error: null });
|
||||
assert.deepEqual(combineTextWithPeriod("12:00", "pm"), { value: "12:00", error: null });
|
||||
});
|
||||
|
||||
test("combineTextWithPeriod: 13..23 hour is unambiguous, accepted without a toggle", () => {
|
||||
assert.deepEqual(combineTextWithPeriod("13:30", null), { value: "13:30", error: null });
|
||||
assert.deepEqual(combineTextWithPeriod("14:45", null), { value: "14:45", error: null });
|
||||
assert.deepEqual(combineTextWithPeriod("23:59", null), { value: "23:59", error: null });
|
||||
// Toggle is irrelevant for 24h-only hours.
|
||||
assert.deepEqual(combineTextWithPeriod("13:30", "am"), { value: "13:30", error: null });
|
||||
assert.deepEqual(combineTextWithPeriod("13:30", "pm"), { value: "13:30", error: null });
|
||||
});
|
||||
|
||||
test("combineTextWithPeriod: hour 0 (midnight 24h) is unambiguous, accepted without a toggle", () => {
|
||||
// Typed "00:30" / "00:00" is only producible in 24h notation
|
||||
// (12h would write midnight as "12:00 AM"), so it cannot be
|
||||
// misread as a 1–12 PM hour. Accept directly without forcing
|
||||
// the user to also tap the AM toggle.
|
||||
assert.deepEqual(combineTextWithPeriod("00:00", null), { value: "00:00", error: null });
|
||||
assert.deepEqual(combineTextWithPeriod("00:30", null), { value: "00:30", error: null });
|
||||
// Toggle is irrelevant — must not flip the value.
|
||||
assert.deepEqual(combineTextWithPeriod("00:30", "am"), { value: "00:30", error: null });
|
||||
assert.deepEqual(combineTextWithPeriod("00:30", "pm"), { value: "00:30", error: null });
|
||||
});
|
||||
|
||||
test("combineTextWithPeriod: typed AM/PM marker overrides the toggle", () => {
|
||||
// Power-user types "9:30 PM" — the marker wins even if the toggle
|
||||
// happens to point at AM (or is unset).
|
||||
assert.deepEqual(combineTextWithPeriod("9:30 PM", null), { value: "21:30", error: null });
|
||||
assert.deepEqual(combineTextWithPeriod("9:30 PM", "am"), { value: "21:30", error: null });
|
||||
assert.deepEqual(combineTextWithPeriod("9:30 AM", "pm"), { value: "09:30", error: null });
|
||||
// Arabic markers behave the same way.
|
||||
assert.deepEqual(combineTextWithPeriod("9:30 ص", null), { value: "09:30", error: null });
|
||||
assert.deepEqual(combineTextWithPeriod("9:30 م", null), { value: "21:30", error: null });
|
||||
// Lowercase / dotted markers (parseTypedTime accepts them).
|
||||
assert.deepEqual(combineTextWithPeriod("9:30pm", null), { value: "21:30", error: null });
|
||||
assert.deepEqual(combineTextWithPeriod("9:30 p.m.", null), { value: "21:30", error: null });
|
||||
});
|
||||
|
||||
test("combineTextWithPeriod: unparseable input is invalid regardless of toggle", () => {
|
||||
assert.deepEqual(combineTextWithPeriod("25:99", null), { value: null, error: "invalid" });
|
||||
assert.deepEqual(combineTextWithPeriod("25:99", "pm"), { value: null, error: "invalid" });
|
||||
assert.deepEqual(combineTextWithPeriod("abc", "am"), { value: null, error: "invalid" });
|
||||
});
|
||||
|
||||
test("combineTextWithPeriod: marker in text + invalid hour is invalid", () => {
|
||||
// "13:00 PM" — parseTypedTime rejects 13 in 12h context.
|
||||
assert.deepEqual(combineTextWithPeriod("13:00 PM", null), {
|
||||
value: null,
|
||||
error: "invalid",
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,335 @@
|
||||
// 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`.
|
||||
//
|
||||
// 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).
|
||||
// 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,
|
||||
useCallback,
|
||||
useEffect,
|
||||
useImperativeHandle,
|
||||
useRef,
|
||||
useState,
|
||||
type Ref,
|
||||
type KeyboardEvent as ReactKeyboardEvent,
|
||||
} from "react";
|
||||
import {
|
||||
combineTextWithPeriod,
|
||||
formatCanonicalAs12h,
|
||||
periodFromCanonical,
|
||||
type CombineResult,
|
||||
type Period,
|
||||
} from "@/lib/time-12h";
|
||||
|
||||
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.
|
||||
*/
|
||||
commit(): CombineResult;
|
||||
/** Move keyboard focus into the time text input. */
|
||||
focus(): void;
|
||||
/** Select the existing text inside the time input. */
|
||||
select(): void;
|
||||
};
|
||||
|
||||
export type TimePicker12hSize = "inline" | "form";
|
||||
|
||||
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.
|
||||
*/
|
||||
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. */
|
||||
inputId?: string;
|
||||
/** Test id for the text 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;
|
||||
amTestId?: string;
|
||||
pmTestId?: string;
|
||||
ariaLabel?: string;
|
||||
/** Used as the radiogroup's accessible name. */
|
||||
groupLabel?: string;
|
||||
amLabel: string;
|
||||
pmLabel: string;
|
||||
disabled?: boolean;
|
||||
/** Auto-focus the text input on mount (used by the inline editor's
|
||||
* enter-edit transition). */
|
||||
autoFocus?: boolean;
|
||||
/** Visual size: `inline` matches the compact schedule cell;
|
||||
* `form` matches the manage-tab form's text inputs. */
|
||||
size?: TimePicker12hSize;
|
||||
};
|
||||
|
||||
function TimePicker12hImpl(
|
||||
props: Props,
|
||||
ref: Ref<TimePicker12hHandle>,
|
||||
) {
|
||||
const {
|
||||
value,
|
||||
onChange,
|
||||
onCommit,
|
||||
onCancel,
|
||||
inputId,
|
||||
inputTestId,
|
||||
amTestId,
|
||||
pmTestId,
|
||||
ariaLabel,
|
||||
groupLabel,
|
||||
amLabel,
|
||||
pmLabel,
|
||||
disabled,
|
||||
autoFocus,
|
||||
size = "form",
|
||||
} = props;
|
||||
|
||||
const inputRef = useRef<HTMLInputElement | null>(null);
|
||||
const [text, setText] = useState<string>(() => formatCanonicalAs12h(value));
|
||||
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
|
||||
// `value` prop (e.g. mid-edit re-render in the inline editor before
|
||||
// the controlled state catches up).
|
||||
const dirtyRef = useRef(false);
|
||||
// Mirror of `period` for synchronous reads inside commit() — React
|
||||
// state isn't yet updated when commit() runs from the same event
|
||||
// tick that toggled the period (e.g. Enter on AM button = "set AM
|
||||
// and save").
|
||||
const periodRef = useRef<Period>(period);
|
||||
periodRef.current = period;
|
||||
|
||||
// Re-sync from `value` when the canonical prop changes AND the user
|
||||
// hasn't started editing yet. Manage-form initialiser may populate
|
||||
// state asynchronously; the inline editor remounts pickers fresh
|
||||
// when entering edit mode so this branch primarily covers the
|
||||
// form-edit case.
|
||||
useEffect(() => {
|
||||
if (dirtyRef.current) return;
|
||||
setText(formatCanonicalAs12h(value));
|
||||
const next = periodFromCanonical(value);
|
||||
setPeriod(next);
|
||||
periodRef.current = next;
|
||||
}, [value]);
|
||||
|
||||
useEffect(() => {
|
||||
if (autoFocus) {
|
||||
inputRef.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();
|
||||
}
|
||||
// Mount-only on purpose: re-running this on prop changes would
|
||||
// steal focus from the user mid-edit.
|
||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||
}, []);
|
||||
|
||||
useImperativeHandle(
|
||||
ref,
|
||||
(): TimePicker12hHandle => ({
|
||||
commit: () =>
|
||||
combineTextWithPeriod(
|
||||
inputRef.current?.value ?? text,
|
||||
periodRef.current,
|
||||
),
|
||||
focus: () => {
|
||||
inputRef.current?.focus();
|
||||
},
|
||||
select: () => {
|
||||
inputRef.current?.select();
|
||||
},
|
||||
}),
|
||||
[text],
|
||||
);
|
||||
|
||||
const propagateIfClean = useCallback(
|
||||
(nextText: string, nextPeriod: Period) => {
|
||||
const result = combineTextWithPeriod(nextText, nextPeriod);
|
||||
// Only propagate clean values upward; ambiguous / invalid
|
||||
// drafts surface via commit() so the parent decides when to
|
||||
// toast.
|
||||
if (result.error === null) {
|
||||
onChange?.(result.value ?? "");
|
||||
}
|
||||
},
|
||||
[onChange],
|
||||
);
|
||||
|
||||
const handleTextChange = (e: React.ChangeEvent<HTMLInputElement>) => {
|
||||
dirtyRef.current = true;
|
||||
setText(e.target.value);
|
||||
};
|
||||
|
||||
const handleTextBlur = () => {
|
||||
propagateIfClean(text, period);
|
||||
};
|
||||
|
||||
const handleTextKeyDown = (e: ReactKeyboardEvent<HTMLInputElement>) => {
|
||||
if (e.key === "Escape") {
|
||||
e.preventDefault();
|
||||
onCancel?.();
|
||||
} else if (e.key === "Enter") {
|
||||
e.preventDefault();
|
||||
onCommit?.();
|
||||
}
|
||||
};
|
||||
|
||||
const setPeriodAndPropagate = (next: Period) => {
|
||||
dirtyRef.current = true;
|
||||
periodRef.current = next;
|
||||
setPeriod(next);
|
||||
propagateIfClean(text, next);
|
||||
};
|
||||
|
||||
const handleButtonKeyDown = (
|
||||
e: ReactKeyboardEvent<HTMLButtonElement>,
|
||||
p: "am" | "pm",
|
||||
) => {
|
||||
if (e.key === "Enter") {
|
||||
// Enter on the toggle means "I want this period AND save". We
|
||||
// set the period synchronously through the ref so the commit
|
||||
// path the parent kicks off in onCommit reads the new value
|
||||
// even though React's state hasn't flushed yet.
|
||||
e.preventDefault();
|
||||
setPeriodAndPropagate(p);
|
||||
onCommit?.();
|
||||
} else if (e.key === "Escape") {
|
||||
e.preventDefault();
|
||||
onCancel?.();
|
||||
}
|
||||
// Space falls through to the default <button> activation, which
|
||||
// fires onClick → setPeriodAndPropagate. That matches the
|
||||
// typical "Space activates a button" expectation.
|
||||
};
|
||||
|
||||
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";
|
||||
|
||||
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";
|
||||
const buttonSize = isInline
|
||||
? "px-1.5 py-0.5 text-[10px] leading-none"
|
||||
: "px-2.5 py-1 text-xs leading-tight h-9";
|
||||
|
||||
const amActive = period === "am";
|
||||
const pmActive = period === "pm";
|
||||
|
||||
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.
|
||||
dir="ltr"
|
||||
>
|
||||
<input
|
||||
ref={inputRef}
|
||||
id={inputId}
|
||||
type="text"
|
||||
inputMode="numeric"
|
||||
autoComplete="off"
|
||||
spellCheck={false}
|
||||
placeholder="HH:MM"
|
||||
value={text}
|
||||
disabled={disabled}
|
||||
onChange={handleTextChange}
|
||||
onBlur={handleTextBlur}
|
||||
onKeyDown={handleTextKeyDown}
|
||||
aria-label={ariaLabel}
|
||||
className={inputCls}
|
||||
data-testid={inputTestId}
|
||||
/>
|
||||
<div
|
||||
className="inline-flex"
|
||||
role="radiogroup"
|
||||
aria-label={groupLabel}
|
||||
>
|
||||
<button
|
||||
type="button"
|
||||
role="radio"
|
||||
aria-checked={amActive}
|
||||
aria-pressed={amActive}
|
||||
disabled={disabled}
|
||||
onMouseDown={(e) => e.preventDefault()}
|
||||
onClick={() => setPeriodAndPropagate("am")}
|
||||
onKeyDown={(e) => handleButtonKeyDown(e, "am")}
|
||||
className={
|
||||
buttonBase +
|
||||
" " +
|
||||
buttonSize +
|
||||
" rounded-s-md " +
|
||||
(amActive
|
||||
? "bg-blue-600 text-white border-blue-600 z-10"
|
||||
: "bg-white text-blue-700 border-blue-400 hover:bg-blue-50")
|
||||
}
|
||||
data-testid={amTestId}
|
||||
>
|
||||
{amLabel}
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
role="radio"
|
||||
aria-checked={pmActive}
|
||||
aria-pressed={pmActive}
|
||||
disabled={disabled}
|
||||
onMouseDown={(e) => e.preventDefault()}
|
||||
onClick={() => setPeriodAndPropagate("pm")}
|
||||
onKeyDown={(e) => handleButtonKeyDown(e, "pm")}
|
||||
className={
|
||||
buttonBase +
|
||||
" " +
|
||||
buttonSize +
|
||||
" rounded-e-md -ms-px " +
|
||||
(pmActive
|
||||
? "bg-blue-600 text-white border-blue-600 z-10"
|
||||
: "bg-white text-blue-700 border-blue-400 hover:bg-blue-50")
|
||||
}
|
||||
data-testid={pmTestId}
|
||||
>
|
||||
{pmLabel}
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
export const TimePicker12h = forwardRef<TimePicker12hHandle, Props>(
|
||||
TimePicker12hImpl,
|
||||
);
|
||||
TimePicker12h.displayName = "TimePicker12h";
|
||||
@@ -0,0 +1,135 @@
|
||||
// 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.
|
||||
//
|
||||
// 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 1–12 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.
|
||||
|
||||
// Use the explicit `.ts` extension so this module is loadable both by
|
||||
// the Vite bundler (via `allowImportingTsExtensions`) AND by Node's
|
||||
// built-in test runner when the unit tests in src/__tests__ import
|
||||
// this module — the test runner does not auto-resolve extensionless
|
||||
// TypeScript imports.
|
||||
import { parseTypedTime } from "./parse-time.ts";
|
||||
|
||||
export type Period = "am" | "pm" | null;
|
||||
|
||||
export type CombineResult =
|
||||
| { value: string | null; error: null }
|
||||
| { value: null; error: "invalid" | "ambiguous" };
|
||||
|
||||
// Mirror of parseTypedTime's marker regex. Kept local so we can detect
|
||||
// the marker WITHOUT actually parsing — used to decide whether a typed
|
||||
// draft is unambiguous on its own (marker present) or needs the
|
||||
// toggle to disambiguate (marker absent).
|
||||
const AM_PM_MARKER_RE = /(a\.?m\.?|p\.?m\.?|ص|م)\s*$/i;
|
||||
|
||||
/**
|
||||
* Derive the AM/PM toggle position that matches a stored canonical
|
||||
* value. Used to seed the toggle when the editor opens against an
|
||||
* existing time. Empty / unparseable values return `null` (no
|
||||
* implicit default — matches the spec's "no implicit default" rule
|
||||
* for the toggle's empty state).
|
||||
*/
|
||||
export function periodFromCanonical(v: string | null | undefined): Period {
|
||||
if (!v) return null;
|
||||
const m = /^(\d{2}):/.exec(v);
|
||||
if (!m) return null;
|
||||
const h = Number(m[1]);
|
||||
if (!Number.isFinite(h)) return null;
|
||||
// 00:xx is "12:xx AM" (h=0 → AM), 12:xx is "12:xx PM" (h=12 → PM).
|
||||
// Anything 13–23 is afternoon → PM; anything 1–11 is morning → AM.
|
||||
return h < 12 ? "am" : "pm";
|
||||
}
|
||||
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
export function formatCanonicalAs12h(v: string | null | undefined): string {
|
||||
if (!v) return "";
|
||||
const m = /^(\d{2}):(\d{2})/.exec(v);
|
||||
if (!m) return v;
|
||||
const h = Number(m[1]);
|
||||
const mm = m[2];
|
||||
if (!Number.isFinite(h)) return v;
|
||||
let h12: number;
|
||||
if (h === 0) h12 = 12;
|
||||
else if (h > 12) h12 = h - 12;
|
||||
else h12 = h;
|
||||
return `${h12}:${mm}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Combine the picker's typed draft (any of the shapes parseTypedTime
|
||||
* accepts: "9:30", "0930", "13:15", "1:15 PM", Arabic-Indic digits, …)
|
||||
* with the AM/PM toggle into a single canonical "HH:mm" value.
|
||||
*
|
||||
* Returns `{ value: null, error: null }` for empty input — caller
|
||||
* decides whether that means "clear the time" (current inline-editor
|
||||
* behaviour) or "leave the field empty" (manage form).
|
||||
*
|
||||
* Returns `{ error: "ambiguous" }` when the typed text resolves to a
|
||||
* 1–12 hour AND the user has not picked AM or PM. This is the core
|
||||
* fix for the "1:15 silently saved as AM" bug — the picker forces the
|
||||
* user to choose rather than guessing for them.
|
||||
*
|
||||
* 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.
|
||||
*/
|
||||
export function combineTextWithPeriod(
|
||||
rawText: string | null | undefined,
|
||||
period: Period,
|
||||
): CombineResult {
|
||||
const trimmed = (rawText ?? "").trim();
|
||||
if (!trimmed) return { value: null, error: null };
|
||||
|
||||
// If the typed text already contains an AM/PM marker, it is the
|
||||
// unambiguous source of truth — the toggle is irrelevant. This lets
|
||||
// power users type "9:30 PM" and have it Just Work, and is also
|
||||
// what makes the "12h with PM" e2e shape pass without a toggle
|
||||
// click.
|
||||
if (AM_PM_MARKER_RE.test(trimmed)) {
|
||||
const parsed = parseTypedTime(trimmed);
|
||||
if (parsed === null) return { value: null, error: "invalid" };
|
||||
return { value: parsed, error: null };
|
||||
}
|
||||
|
||||
const parsedRaw = parseTypedTime(trimmed);
|
||||
if (parsedRaw === null) return { value: null, error: "invalid" };
|
||||
|
||||
// Hours 0 and 13–23 cannot be a 12h shape, so there is no
|
||||
// ambiguity to resolve — accept regardless of the toggle. Hour 0
|
||||
// can only come from a typed "00:xx" / "0:xx" 24h shape (12h
|
||||
// notation would write midnight as "12:xx AM"); hours 13–23 are
|
||||
// unambiguous PM. Lets a user who knows the 24h time of their
|
||||
// meeting type "13:30" or "00:30" and save without also tapping
|
||||
// the toggle.
|
||||
const h = Number(parsedRaw.slice(0, 2));
|
||||
if (h === 0 || h >= 13) return { value: parsedRaw, error: null };
|
||||
|
||||
// Hours 1–12 with no marker: the toggle MUST disambiguate. This
|
||||
// is the "1:15 → 13:15 vs 01:15" decision point that the prior
|
||||
// <input type="time"> silently got wrong.
|
||||
if (period === null) return { value: null, error: "ambiguous" };
|
||||
|
||||
// Re-parse with the toggle applied so the existing parser handles
|
||||
// the 12 AM → 00:xx and 12 PM → 12:xx edge cases (instead of
|
||||
// duplicating that logic here).
|
||||
const withMarker = `${trimmed} ${period === "pm" ? "PM" : "AM"}`;
|
||||
const parsed = parseTypedTime(withMarker);
|
||||
if (parsed === null) return { value: null, error: "invalid" };
|
||||
return { value: parsed, error: null };
|
||||
}
|
||||
@@ -1154,6 +1154,12 @@
|
||||
"cascadeMidnightError": "لا يمكن تأجيل اليوم: \"{{title}}\" سينتقل إلى {{projectedStart}}، بعد منتصف الليل.",
|
||||
"cascadeAuditDescription": "تم التأجيل التلقائي بمقدار {{minutes}} دقيقة بعد تأجيل الاجتماع رقم {{triggerMeetingId}}."
|
||||
},
|
||||
"timeEditor": {
|
||||
"am": "ص",
|
||||
"pm": "م",
|
||||
"periodGroupStart": "ص أو م لوقت البدء",
|
||||
"periodGroupEnd": "ص أو م لوقت الانتهاء"
|
||||
},
|
||||
"schedule": {
|
||||
"addRow": "أضف اجتماع",
|
||||
"newMeetingDefaultAr": "اجتماع جديد",
|
||||
@@ -1164,6 +1170,7 @@
|
||||
"timeEndShort": "الانتهاء",
|
||||
"timeOrderError": "وقت النهاية قبل وقت البداية",
|
||||
"timeParseError": "صيغة الوقت غير صحيحة. اكتب الوقت مثل: 09:30 أو 9:30 ص",
|
||||
"timeAmbiguousError": "اختر ص أو م للوقت",
|
||||
"addAttendee": "أضف حاضر",
|
||||
"addSubheading": "+ عنوان فرعي",
|
||||
"addVirtualAttendee": "+ حاضر عبر الاتصال المرئي",
|
||||
|
||||
@@ -1037,6 +1037,12 @@
|
||||
"cascadeMidnightError": "Can’t shift the day: \"{{title}}\" would land at {{projectedStart}}, past midnight.",
|
||||
"cascadeAuditDescription": "Auto-shifted by {{minutes}} minutes after meeting #{{triggerMeetingId}} was postponed."
|
||||
},
|
||||
"timeEditor": {
|
||||
"am": "AM",
|
||||
"pm": "PM",
|
||||
"periodGroupStart": "AM or PM for the start time",
|
||||
"periodGroupEnd": "AM or PM for the end time"
|
||||
},
|
||||
"schedule": {
|
||||
"addRow": "Add meeting",
|
||||
"newMeetingDefaultAr": "اجتماع جديد",
|
||||
@@ -1047,6 +1053,7 @@
|
||||
"timeEndShort": "End",
|
||||
"timeOrderError": "End time is before start time",
|
||||
"timeParseError": "Time format isn't valid. Try 09:30 or 9:30 PM",
|
||||
"timeAmbiguousError": "Pick AM or PM for the time",
|
||||
"addAttendee": "Add attendee",
|
||||
"addSubheading": "+ subheading",
|
||||
"addVirtualAttendee": "+ Virtual",
|
||||
|
||||
@@ -42,6 +42,10 @@ import {
|
||||
} from "lucide-react";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { Input } from "@/components/ui/input";
|
||||
import {
|
||||
TimePicker12h,
|
||||
type TimePicker12hHandle,
|
||||
} from "@/components/time-picker-12h";
|
||||
import { Label } from "@/components/ui/label";
|
||||
import { Textarea } from "@/components/ui/textarea";
|
||||
import {
|
||||
@@ -88,7 +92,6 @@ import { CSS } from "@dnd-kit/utilities";
|
||||
import { GripVertical } from "lucide-react";
|
||||
import { EditableCell } from "@/components/editable-cell";
|
||||
import { safeHtml } from "@/lib/safe-html";
|
||||
import { parseTypedTime } from "@/lib/parse-time";
|
||||
import {
|
||||
ALERT_COLOR_PRESETS,
|
||||
DEFAULT_ALERT_PREFS,
|
||||
@@ -3366,8 +3369,13 @@ function TimeRangeCell({
|
||||
const [end, setEnd] = useState(endSaved);
|
||||
const wrapperRef = useRef<HTMLDivElement | null>(null);
|
||||
const blurTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||
const startInputRef = useRef<HTMLInputElement | null>(null);
|
||||
const endInputRef = useRef<HTMLInputElement | null>(null);
|
||||
// Imperative handles to the start/end pickers. The inline editor
|
||||
// reads the live draft (typed text + AM/PM toggle) via commit()
|
||||
// when saving, instead of relying on React state — same
|
||||
// belt-and-braces pattern task #308 used to dodge "Enter pressed
|
||||
// before React's batched onChange flush" races.
|
||||
const startPickerRef = useRef<TimePicker12hHandle | null>(null);
|
||||
const endPickerRef = useRef<TimePicker12hHandle | null>(null);
|
||||
|
||||
// If the saved times change while we're not editing (e.g. another tab
|
||||
// updated them, or the user picked a different date), sync local state.
|
||||
@@ -3380,11 +3388,12 @@ function TimeRangeCell({
|
||||
|
||||
useEffect(() => {
|
||||
if (editing) {
|
||||
startInputRef.current?.focus();
|
||||
// Select the existing value so the user can type to overwrite
|
||||
// immediately without having to clear the field first — matches
|
||||
// the behaviour of the native <input type="time"> spinner.
|
||||
startInputRef.current?.select();
|
||||
// The picker auto-focuses its text input via the autoFocus prop
|
||||
// on mount; keep an explicit focus() call here too so re-entry
|
||||
// edge cases (e.g. clicking the cell again while React is mid-
|
||||
// commit) still land focus on the start input.
|
||||
startPickerRef.current?.focus();
|
||||
startPickerRef.current?.select();
|
||||
}
|
||||
}, [editing]);
|
||||
|
||||
@@ -3412,41 +3421,61 @@ function TimeRangeCell({
|
||||
}, [canMutate, editing, startSaved, endSaved]);
|
||||
|
||||
const save = useCallback(async () => {
|
||||
// Always read the live input values from the refs in addition to
|
||||
// the controlled state. This is belt-and-braces against any path
|
||||
// (Enter pressed before React's batched onChange flush, browser
|
||||
// autofill, etc.) where the controlled state could be a frame
|
||||
// behind what the user actually sees. parseTypedTime accepts a
|
||||
// wide range of human-typed shapes (24h, 12h with AM/PM or ص/م,
|
||||
// compact "HHMM", Arabic-Indic digits) and canonicalises them to
|
||||
// "HH:MM"; empty input means "clear the time".
|
||||
const rawStart = (startInputRef.current?.value ?? start ?? "").trim();
|
||||
const rawEnd = (endInputRef.current?.value ?? end ?? "").trim();
|
||||
const newStart = rawStart === "" ? null : parseTypedTime(rawStart);
|
||||
const newEnd = rawEnd === "" ? null : parseTypedTime(rawEnd);
|
||||
// Read the live drafts from each picker's commit() handle. This
|
||||
// returns the canonical "HH:mm" 24h value derived from the typed
|
||||
// text PLUS the AM/PM toggle — empty input is `value: null,
|
||||
// error: null`, an unparseable typed value is `error: "invalid"`,
|
||||
// and a 1–12 hour without an AM/PM pick is `error: "ambiguous"`
|
||||
// (the core fix for the "1:15 silently saved as AM" bug).
|
||||
const startResult = startPickerRef.current?.commit() ?? {
|
||||
value: null,
|
||||
error: null as null,
|
||||
};
|
||||
const endResult = endPickerRef.current?.commit() ?? {
|
||||
value: null,
|
||||
error: null as null,
|
||||
};
|
||||
|
||||
// If the user typed something in either field but it cannot be
|
||||
// parsed, surface a clear destructive toast and keep the editor
|
||||
// open with focus on the offending input — never silently discard
|
||||
// their typing.
|
||||
if (rawStart !== "" && newStart === null) {
|
||||
// Surface the picker's validation errors as toasts and keep the
|
||||
// editor open with focus on the offending input — never silently
|
||||
// discard the user's typing or guess the period for them.
|
||||
if (startResult.error === "invalid") {
|
||||
toast({
|
||||
title: t("executiveMeetings.schedule.timeParseError"),
|
||||
variant: "destructive",
|
||||
});
|
||||
startInputRef.current?.focus();
|
||||
startInputRef.current?.select();
|
||||
startPickerRef.current?.focus();
|
||||
startPickerRef.current?.select();
|
||||
return;
|
||||
}
|
||||
if (rawEnd !== "" && newEnd === null) {
|
||||
if (startResult.error === "ambiguous") {
|
||||
toast({
|
||||
title: t("executiveMeetings.schedule.timeAmbiguousError"),
|
||||
variant: "destructive",
|
||||
});
|
||||
startPickerRef.current?.focus();
|
||||
return;
|
||||
}
|
||||
if (endResult.error === "invalid") {
|
||||
toast({
|
||||
title: t("executiveMeetings.schedule.timeParseError"),
|
||||
variant: "destructive",
|
||||
});
|
||||
endInputRef.current?.focus();
|
||||
endInputRef.current?.select();
|
||||
endPickerRef.current?.focus();
|
||||
endPickerRef.current?.select();
|
||||
return;
|
||||
}
|
||||
if (endResult.error === "ambiguous") {
|
||||
toast({
|
||||
title: t("executiveMeetings.schedule.timeAmbiguousError"),
|
||||
variant: "destructive",
|
||||
});
|
||||
endPickerRef.current?.focus();
|
||||
return;
|
||||
}
|
||||
|
||||
const newStart = startResult.value;
|
||||
const newEnd = endResult.value;
|
||||
|
||||
if (newStart === (startSaved || null) && newEnd === (endSaved || null)) {
|
||||
setEditing(false);
|
||||
@@ -3459,7 +3488,7 @@ function TimeRangeCell({
|
||||
title: t("executiveMeetings.schedule.timeOrderError"),
|
||||
variant: "destructive",
|
||||
});
|
||||
startInputRef.current?.focus();
|
||||
startPickerRef.current?.focus();
|
||||
return;
|
||||
}
|
||||
// Snap the controlled state to the canonical "HH:MM" so the
|
||||
@@ -3479,14 +3508,14 @@ function TimeRangeCell({
|
||||
// the start input. For any other failure we drop the draft and
|
||||
// return to the previously-saved values.
|
||||
if (err instanceof Error && err.name === "TimeOrderError") {
|
||||
startInputRef.current?.focus();
|
||||
startPickerRef.current?.focus();
|
||||
return;
|
||||
}
|
||||
setStart(startSaved);
|
||||
setEnd(endSaved);
|
||||
setEditing(false);
|
||||
}
|
||||
}, [start, end, startSaved, endSaved, onSaveTimes, toast, t]);
|
||||
}, [startSaved, endSaved, onSaveTimes, toast, t]);
|
||||
|
||||
const onFocusOut = (e: React.FocusEvent<HTMLDivElement>) => {
|
||||
if (!editing) return;
|
||||
@@ -3539,16 +3568,6 @@ function TimeRangeCell({
|
||||
);
|
||||
}
|
||||
|
||||
const onKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => {
|
||||
if (e.key === "Escape") {
|
||||
e.preventDefault();
|
||||
cancel();
|
||||
} else if (e.key === "Enter") {
|
||||
e.preventDefault();
|
||||
void save();
|
||||
}
|
||||
};
|
||||
|
||||
// Stable per-meeting input ids so the <label htmlFor=...> hookup is
|
||||
// unique across meeting rows in the schedule. Without this, screen
|
||||
// readers and click-to-focus on the label would jump to the first
|
||||
@@ -3578,27 +3597,28 @@ function TimeRangeCell({
|
||||
>
|
||||
{t("executiveMeetings.schedule.timeStartShort")}
|
||||
</label>
|
||||
<input
|
||||
ref={startInputRef}
|
||||
id={startInputId}
|
||||
// Plain text input (not type="time") so the controlled state
|
||||
// updates on EVERY keystroke. The native <input type="time">
|
||||
// only fires onChange when the value is a complete, canonical
|
||||
// "HH:MM" string, which made keyboard typing + Enter a silent
|
||||
// no-op for partial input, "9:30" without leading zero,
|
||||
// "0930" compact, AM/PM shapes, and Arabic-Indic digits.
|
||||
// parseTypedTime() handles all of those on save.
|
||||
type="text"
|
||||
inputMode="numeric"
|
||||
autoComplete="off"
|
||||
spellCheck={false}
|
||||
placeholder="HH:MM"
|
||||
{/* 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. */}
|
||||
<TimePicker12h
|
||||
ref={startPickerRef}
|
||||
inputId={startInputId}
|
||||
value={start}
|
||||
onChange={(e) => setStart(e.target.value)}
|
||||
onKeyDown={onKeyDown}
|
||||
aria-label={t("executiveMeetings.schedule.timeStart")}
|
||||
className="w-[5rem] border border-blue-400 rounded px-1 py-0.5 text-xs bg-white text-center"
|
||||
data-testid={`em-time-start-${meeting.id}`}
|
||||
onChange={setStart}
|
||||
onCommit={() => void save()}
|
||||
onCancel={cancel}
|
||||
ariaLabel={t("executiveMeetings.schedule.timeStart")}
|
||||
groupLabel={t("executiveMeetings.timeEditor.periodGroupStart")}
|
||||
amLabel={t("executiveMeetings.timeEditor.am")}
|
||||
pmLabel={t("executiveMeetings.timeEditor.pm")}
|
||||
inputTestId={`em-time-start-${meeting.id}`}
|
||||
amTestId={`em-time-start-period-am-${meeting.id}`}
|
||||
pmTestId={`em-time-start-period-pm-${meeting.id}`}
|
||||
autoFocus
|
||||
size="inline"
|
||||
/>
|
||||
</div>
|
||||
<span className="text-muted-foreground pb-0.5">–</span>
|
||||
@@ -3609,22 +3629,21 @@ function TimeRangeCell({
|
||||
>
|
||||
{t("executiveMeetings.schedule.timeEndShort")}
|
||||
</label>
|
||||
<input
|
||||
ref={endInputRef}
|
||||
id={endInputId}
|
||||
// See the start input above for why this is type="text" and
|
||||
// not type="time".
|
||||
type="text"
|
||||
inputMode="numeric"
|
||||
autoComplete="off"
|
||||
spellCheck={false}
|
||||
placeholder="HH:MM"
|
||||
<TimePicker12h
|
||||
ref={endPickerRef}
|
||||
inputId={endInputId}
|
||||
value={end}
|
||||
onChange={(e) => setEnd(e.target.value)}
|
||||
onKeyDown={onKeyDown}
|
||||
aria-label={t("executiveMeetings.schedule.timeEnd")}
|
||||
className="w-[5rem] border border-blue-400 rounded px-1 py-0.5 text-xs bg-white text-center"
|
||||
data-testid={`em-time-end-${meeting.id}`}
|
||||
onChange={setEnd}
|
||||
onCommit={() => void save()}
|
||||
onCancel={cancel}
|
||||
ariaLabel={t("executiveMeetings.schedule.timeEnd")}
|
||||
groupLabel={t("executiveMeetings.timeEditor.periodGroupEnd")}
|
||||
amLabel={t("executiveMeetings.timeEditor.am")}
|
||||
pmLabel={t("executiveMeetings.timeEditor.pm")}
|
||||
inputTestId={`em-time-end-${meeting.id}`}
|
||||
amTestId={`em-time-end-period-am-${meeting.id}`}
|
||||
pmTestId={`em-time-end-period-pm-${meeting.id}`}
|
||||
size="inline"
|
||||
/>
|
||||
</div>
|
||||
<button
|
||||
@@ -4618,7 +4637,7 @@ function ManageSection({
|
||||
});
|
||||
}
|
||||
|
||||
async function save() {
|
||||
async function save(timeOverrides?: { startTime: string; endTime: string }) {
|
||||
if (!editing) return;
|
||||
if (!editing.titleAr.trim() || !editing.titleEn.trim()) {
|
||||
toast({
|
||||
@@ -4627,13 +4646,25 @@ function ManageSection({
|
||||
});
|
||||
return;
|
||||
}
|
||||
// Prefer the dialog-supplied canonical values from the time
|
||||
// picker's commit() result over the controlled `editing.*`
|
||||
// state — the picker's onChange suppresses ambiguous/invalid
|
||||
// drafts so `editing.startTime` / `editing.endTime` may still
|
||||
// hold the previous saved value when the user typed but never
|
||||
// resolved the AM/PM. Task #315: prevent silent fallback.
|
||||
const startTime = timeOverrides
|
||||
? timeOverrides.startTime || null
|
||||
: editing.startTime || null;
|
||||
const endTime = timeOverrides
|
||||
? timeOverrides.endTime || null
|
||||
: editing.endTime || null;
|
||||
setSaving(true);
|
||||
const body: Record<string, unknown> = {
|
||||
titleAr: editing.titleAr.trim(),
|
||||
titleEn: editing.titleEn.trim(),
|
||||
meetingDate: editing.meetingDate,
|
||||
startTime: editing.startTime || null,
|
||||
endTime: editing.endTime || null,
|
||||
startTime,
|
||||
endTime,
|
||||
location: editing.location.trim() || null,
|
||||
meetingUrl: editing.meetingUrl.trim() || null,
|
||||
platform: editing.platform,
|
||||
@@ -4804,7 +4835,17 @@ function ManageSection({
|
||||
<MeetingFormDialog
|
||||
state={editing}
|
||||
onChange={setEditing}
|
||||
onSave={save}
|
||||
// Pass the committed (canonical) start/end through so the
|
||||
// dialog's Enter/click-Save path validates the time pickers
|
||||
// synchronously (per task #315: typing "1:15" without an
|
||||
// AM/PM pick must surface as an error, NOT silently fall
|
||||
// back to the previous saved value or AM). The values
|
||||
// arriving here are already canonical "HH:mm" or "" — we
|
||||
// hand them straight into the save body so a stale React
|
||||
// state batch from the picker's onChange can't lose them.
|
||||
onSave={(committedStart, committedEnd) =>
|
||||
save({ startTime: committedStart, endTime: committedEnd })
|
||||
}
|
||||
onClose={() => setEditing(null)}
|
||||
saving={saving}
|
||||
t={t}
|
||||
@@ -5013,11 +5054,70 @@ function MeetingFormDialog({
|
||||
}: {
|
||||
state: MeetingFormState;
|
||||
onChange: (s: MeetingFormState) => void;
|
||||
onSave: () => void;
|
||||
// Receives the canonical "HH:mm" 24h start/end values that the
|
||||
// time pickers commit() resolved. The dialog blocks Save when
|
||||
// either picker reports `invalid` or `ambiguous`, so the parent
|
||||
// never has to second-guess: if onSave fires, the times have
|
||||
// been validated.
|
||||
onSave: (committedStart: string, committedEnd: string) => void;
|
||||
onClose: () => void;
|
||||
saving: boolean;
|
||||
t: (k: string) => string;
|
||||
}) {
|
||||
const { toast } = useToast();
|
||||
// Imperative handles to the start/end pickers so the Save click
|
||||
// can read the live draft (typed text + AM/PM toggle) and surface
|
||||
// ambiguous/invalid input as a toast rather than silently saving
|
||||
// a stale value. Same belt-and-braces pattern the inline schedule
|
||||
// editor uses (TimeRangeCell).
|
||||
const startPickerRef = useRef<TimePicker12hHandle | null>(null);
|
||||
const endPickerRef = useRef<TimePicker12hHandle | null>(null);
|
||||
|
||||
function handleSaveClick() {
|
||||
const startResult = startPickerRef.current?.commit() ?? {
|
||||
value: null,
|
||||
error: null as null,
|
||||
};
|
||||
const endResult = endPickerRef.current?.commit() ?? {
|
||||
value: null,
|
||||
error: null as null,
|
||||
};
|
||||
if (startResult.error === "invalid") {
|
||||
toast({
|
||||
title: t("executiveMeetings.schedule.timeParseError"),
|
||||
variant: "destructive",
|
||||
});
|
||||
startPickerRef.current?.focus();
|
||||
startPickerRef.current?.select();
|
||||
return;
|
||||
}
|
||||
if (startResult.error === "ambiguous") {
|
||||
toast({
|
||||
title: t("executiveMeetings.schedule.timeAmbiguousError"),
|
||||
variant: "destructive",
|
||||
});
|
||||
startPickerRef.current?.focus();
|
||||
return;
|
||||
}
|
||||
if (endResult.error === "invalid") {
|
||||
toast({
|
||||
title: t("executiveMeetings.schedule.timeParseError"),
|
||||
variant: "destructive",
|
||||
});
|
||||
endPickerRef.current?.focus();
|
||||
endPickerRef.current?.select();
|
||||
return;
|
||||
}
|
||||
if (endResult.error === "ambiguous") {
|
||||
toast({
|
||||
title: t("executiveMeetings.schedule.timeAmbiguousError"),
|
||||
variant: "destructive",
|
||||
});
|
||||
endPickerRef.current?.focus();
|
||||
return;
|
||||
}
|
||||
onSave(startResult.value ?? "", endResult.value ?? "");
|
||||
}
|
||||
function update<K extends keyof MeetingFormState>(k: K, v: MeetingFormState[K]) {
|
||||
onChange({ ...state, [k]: v });
|
||||
}
|
||||
@@ -5162,17 +5262,33 @@ function MeetingFormDialog({
|
||||
/>
|
||||
</FormRow>
|
||||
<FormRow label={t("executiveMeetings.manage.field.startTime")}>
|
||||
<Input
|
||||
type="time"
|
||||
<TimePicker12h
|
||||
ref={startPickerRef}
|
||||
value={state.startTime}
|
||||
onChange={(e) => update("startTime", e.target.value)}
|
||||
onChange={(v) => update("startTime", v)}
|
||||
ariaLabel={t("executiveMeetings.manage.field.startTime")}
|
||||
groupLabel={t("executiveMeetings.timeEditor.periodGroupStart")}
|
||||
amLabel={t("executiveMeetings.timeEditor.am")}
|
||||
pmLabel={t("executiveMeetings.timeEditor.pm")}
|
||||
size="form"
|
||||
inputTestId="em-form-startTime"
|
||||
amTestId="em-form-startTime-am"
|
||||
pmTestId="em-form-startTime-pm"
|
||||
/>
|
||||
</FormRow>
|
||||
<FormRow label={t("executiveMeetings.manage.field.endTime")}>
|
||||
<Input
|
||||
type="time"
|
||||
<TimePicker12h
|
||||
ref={endPickerRef}
|
||||
value={state.endTime}
|
||||
onChange={(e) => update("endTime", e.target.value)}
|
||||
onChange={(v) => update("endTime", v)}
|
||||
ariaLabel={t("executiveMeetings.manage.field.endTime")}
|
||||
groupLabel={t("executiveMeetings.timeEditor.periodGroupEnd")}
|
||||
amLabel={t("executiveMeetings.timeEditor.am")}
|
||||
pmLabel={t("executiveMeetings.timeEditor.pm")}
|
||||
size="form"
|
||||
inputTestId="em-form-endTime"
|
||||
amTestId="em-form-endTime-am"
|
||||
pmTestId="em-form-endTime-pm"
|
||||
/>
|
||||
</FormRow>
|
||||
<FormRow label={t("executiveMeetings.manage.field.location")}>
|
||||
@@ -5330,7 +5446,12 @@ function MeetingFormDialog({
|
||||
<Button variant="ghost" onClick={onClose} disabled={saving}>
|
||||
{t("executiveMeetings.common.cancel")}
|
||||
</Button>
|
||||
<Button onClick={onSave} disabled={saving} className="bg-[#0B1E3F]" data-testid="em-form-save">
|
||||
<Button
|
||||
onClick={handleSaveClick}
|
||||
disabled={saving}
|
||||
className="bg-[#0B1E3F]"
|
||||
data-testid="em-form-save"
|
||||
>
|
||||
{saving ? t("common.loading") : t("executiveMeetings.common.save")}
|
||||
</Button>
|
||||
</DialogFooter>
|
||||
|
||||
@@ -267,9 +267,13 @@ test("Schedule keyboard: Tab into time cell, Enter opens edit, valid times save
|
||||
await Promise.all([savePromise, refetchPromise]);
|
||||
|
||||
// Editor must collapse back to display mode and show the new values.
|
||||
// The display cell renders compact 12h with NO AM/PM marker per
|
||||
// formatTime() — "13:30" (24h wire) renders as "1:30" in the cell.
|
||||
// The DB row asserted below is the source of truth for the wire
|
||||
// value; the regex here just confirms the display refreshed.
|
||||
await expect(startInput).toHaveCount(0);
|
||||
await expect(endInput).toHaveCount(0);
|
||||
await expect(timeCell).toHaveText(/13:30\s*[\u2013\-]\s*14:45/, {
|
||||
await expect(timeCell).toHaveText(/1:30\s*[\u2013\-]\s*2:45/, {
|
||||
timeout: 10_000,
|
||||
});
|
||||
|
||||
@@ -306,7 +310,9 @@ test("Schedule keyboard: time cell Esc restores original times and sends no PATC
|
||||
await gotoMeetingRow(page, meetingId, date);
|
||||
|
||||
const timeCell = page.getByTestId(`em-time-${meetingId}`);
|
||||
await expect(timeCell).toHaveText(/08:15\s*[\u2013\-]\s*09:45/);
|
||||
// formatTime() renders compact 12h with no AM/PM marker, dropping
|
||||
// the leading zero — "08:15" (24h wire) shows as "8:15" in the cell.
|
||||
await expect(timeCell).toHaveText(/8:15\s*[\u2013\-]\s*9:45/);
|
||||
|
||||
// Spy on any PATCH to this meeting — Escape must not trigger one.
|
||||
let patchSeen = false;
|
||||
@@ -336,11 +342,13 @@ test("Schedule keyboard: time cell Esc restores original times and sends no PATC
|
||||
await endInput.focus();
|
||||
await endInput.fill("18:30");
|
||||
|
||||
// Esc cancels — editor closes, original values restored.
|
||||
// Esc cancels — editor closes, original values restored. Display
|
||||
// cell uses the same compact 12h, no AM/PM, no-leading-zero shape
|
||||
// formatTime() produces.
|
||||
await page.keyboard.press("Escape");
|
||||
await expect(startInput).toHaveCount(0);
|
||||
await expect(endInput).toHaveCount(0);
|
||||
await expect(timeCell).toHaveText(/08:15\s*[\u2013\-]\s*09:45/);
|
||||
await expect(timeCell).toHaveText(/8:15\s*[\u2013\-]\s*9:45/);
|
||||
|
||||
// Give the page a beat to make sure no late PATCH fires from a
|
||||
// delayed blur path before we assert.
|
||||
@@ -717,9 +725,27 @@ test("Schedule keyboard: Tab from the title cell reaches the time cell in the sa
|
||||
// -------------------------------------------------------------------
|
||||
|
||||
// Reusable helper: open a meeting's time cell, focus the start input,
|
||||
// clear it, type a new value, do the same for end, press Enter, and
|
||||
// wait for the PATCH + day GET refetch.
|
||||
async function typeTimesAndSave(page, meetingId, date, startText, endText) {
|
||||
// clear it, type a new value, optionally pick the AM/PM toggle, do
|
||||
// the same for end, press Enter, and wait for the PATCH + day GET
|
||||
// refetch.
|
||||
//
|
||||
// Per task #315 the time cell is now a 12-hour picker — typing a
|
||||
// 1–12 hour without first picking AM or PM is intentionally an
|
||||
// ambiguous parse error (the fix for the "1:15 silently saved as
|
||||
// AM" bug). For typed shapes that the picker can resolve on its
|
||||
// own (an explicit AM/PM marker like "9:30 PM", or a 24h-only hour
|
||||
// like "13:30") `period` should be `null`; for ambiguous shapes
|
||||
// pass "am" or "pm" so the helper clicks the matching toggle
|
||||
// before pressing Enter.
|
||||
async function typeTimesAndSave(
|
||||
page,
|
||||
meetingId,
|
||||
date,
|
||||
startText,
|
||||
endText,
|
||||
startPeriod = null,
|
||||
endPeriod = null,
|
||||
) {
|
||||
const timeCell = page.getByTestId(`em-time-${meetingId}`);
|
||||
await timeCell.evaluate((el) => el.focus());
|
||||
await page.keyboard.press("Enter");
|
||||
@@ -735,11 +761,21 @@ async function typeTimesAndSave(page, meetingId, date, startText, endText) {
|
||||
await page.keyboard.press("ControlOrMeta+a");
|
||||
await page.keyboard.press("Delete");
|
||||
await page.keyboard.type(startText, { delay: 10 });
|
||||
if (startPeriod) {
|
||||
await page
|
||||
.getByTestId(`em-time-start-period-${startPeriod}-${meetingId}`)
|
||||
.click();
|
||||
}
|
||||
|
||||
await endInput.click();
|
||||
await page.keyboard.press("ControlOrMeta+a");
|
||||
await page.keyboard.press("Delete");
|
||||
await page.keyboard.type(endText, { delay: 10 });
|
||||
if (endPeriod) {
|
||||
await page
|
||||
.getByTestId(`em-time-end-period-${endPeriod}-${meetingId}`)
|
||||
.click();
|
||||
}
|
||||
|
||||
const savePromise = page.waitForResponse(
|
||||
(resp) => {
|
||||
@@ -753,6 +789,9 @@ async function typeTimesAndSave(page, meetingId, date, startText, endText) {
|
||||
{ timeout: 15_000 },
|
||||
);
|
||||
const refetchPromise = waitForDayRefetch(page, date);
|
||||
// Re-focus the end input so Enter triggers save() rather than
|
||||
// re-activating the AM/PM toggle that was last clicked.
|
||||
await endInput.focus();
|
||||
await page.keyboard.press("Enter");
|
||||
await Promise.all([savePromise, refetchPromise]);
|
||||
}
|
||||
@@ -762,11 +801,20 @@ async function typeTimesAndSave(page, meetingId, date, startText, endText) {
|
||||
// assertions stay simple, and the tested shapes intentionally vary in
|
||||
// hour/minute so a test that accidentally passes by reading stale
|
||||
// state (rather than the new value) would fail.
|
||||
//
|
||||
// `startPeriod` / `endPeriod` mirror task #315's "the picker has no
|
||||
// implicit default" rule: ambiguous typed shapes (1–12 hour, no
|
||||
// marker) require the helper to click the AM/PM toggle, while
|
||||
// shapes with an explicit marker or a 24h-only hour are resolved by
|
||||
// the picker without a toggle pick.
|
||||
const TYPING_SHAPES = [
|
||||
{ offset: 8, label: "canonical 24h", startText: "13:30", endText: "14:45", expectStart: "13:30:00", expectEnd: "14:45:00" },
|
||||
{ offset: 9, label: "no leading zero", startText: "9:30", endText: "10:45", expectStart: "09:30:00", expectEnd: "10:45:00" },
|
||||
{ offset: 10, label: "compact HHMM", startText: "0930", endText: "1045", expectStart: "09:30:00", expectEnd: "10:45:00" },
|
||||
{ offset: 11, label: "12h with PM", startText: "9:30 AM", endText: "9:30 PM", expectStart: "09:30:00", expectEnd: "21:30:00" },
|
||||
{ offset: 8, label: "canonical 24h", startText: "13:30", endText: "14:45", startPeriod: null, endPeriod: null, expectStart: "13:30:00", expectEnd: "14:45:00" },
|
||||
{ offset: 9, label: "no leading zero + AM", startText: "9:30", endText: "10:45", startPeriod: "am", endPeriod: "am", expectStart: "09:30:00", expectEnd: "10:45:00" },
|
||||
{ offset: 10, label: "compact HHMM + AM", startText: "0930", endText: "1045", startPeriod: "am", endPeriod: "am", expectStart: "09:30:00", expectEnd: "10:45:00" },
|
||||
{ offset: 11, label: "12h with PM", startText: "9:30 AM", endText: "9:30 PM", startPeriod: null, endPeriod: null, expectStart: "09:30:00", expectEnd: "21:30:00" },
|
||||
// The "0115 + PM = 13:15" shape from the task spec — the toggle
|
||||
// promotes the typed compact hour to PM.
|
||||
{ offset: 12, label: "compact + PM toggle", startText: "0115", endText: "0230", startPeriod: "pm", endPeriod: "pm", expectStart: "13:15:00", expectEnd: "14:30:00" },
|
||||
];
|
||||
|
||||
for (const shape of TYPING_SHAPES) {
|
||||
@@ -791,7 +839,15 @@ for (const shape of TYPING_SHAPES) {
|
||||
|
||||
await gotoMeetingRow(page, meetingId, date);
|
||||
|
||||
await typeTimesAndSave(page, meetingId, date, shape.startText, shape.endText);
|
||||
await typeTimesAndSave(
|
||||
page,
|
||||
meetingId,
|
||||
date,
|
||||
shape.startText,
|
||||
shape.endText,
|
||||
shape.startPeriod,
|
||||
shape.endPeriod,
|
||||
);
|
||||
|
||||
const timeCell = page.getByTestId(`em-time-${meetingId}`);
|
||||
await expect(page.getByTestId(`em-time-start-${meetingId}`)).toHaveCount(0);
|
||||
@@ -827,8 +883,10 @@ test("Schedule keyboard (AR): typing Arabic-Indic digits + Enter saves canonical
|
||||
|
||||
await gotoMeetingRow(page, meetingId, date);
|
||||
|
||||
// ٠٩:٣٠ -> 09:30, ١٠:٤٥ -> 10:45
|
||||
await typeTimesAndSave(page, meetingId, date, "٠٩:٣٠", "١٠:٤٥");
|
||||
// ٠٩:٣٠ -> 09:30, ١٠:٤٥ -> 10:45. Both are 1–12 hours so the
|
||||
// picker requires an explicit AM/PM choice (task #315) — pass the
|
||||
// Arabic morning marker via the toggle so the helper clicks ص.
|
||||
await typeTimesAndSave(page, meetingId, date, "٠٩:٣٠", "١٠:٤٥", "am", "am");
|
||||
|
||||
const { rows } = await pool.query(
|
||||
`SELECT start_time, end_time FROM executive_meetings WHERE id = $1`,
|
||||
@@ -1154,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 between start and end preserves the typed start value, Enter on end saves both", async ({
|
||||
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 ({
|
||||
page,
|
||||
}) => {
|
||||
await setLangEn(page);
|
||||
@@ -1175,15 +1233,43 @@ test("Schedule keyboard: Tab between start and end preserves the typed start val
|
||||
await page.keyboard.press("Enter");
|
||||
const startInput = page.getByTestId(`em-time-start-${meetingId}`);
|
||||
const endInput = page.getByTestId(`em-time-end-${meetingId}`);
|
||||
const startAm = page.getByTestId(`em-time-start-period-am-${meetingId}`);
|
||||
const startPm = page.getByTestId(`em-time-start-period-pm-${meetingId}`);
|
||||
const endAm = page.getByTestId(`em-time-end-period-am-${meetingId}`);
|
||||
const endPm = page.getByTestId(`em-time-end-period-pm-${meetingId}`);
|
||||
await expect(startInput).toBeFocused();
|
||||
|
||||
// Type into start, then Tab forward and type into end. The Tab
|
||||
// commit path is what historically lost the start value when the
|
||||
// input was type="time" because no canonical onChange had fired yet.
|
||||
// Type into the start text input — "7:15" is a 1–12 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 });
|
||||
await page.keyboard.press("Tab");
|
||||
await expect(startAm).toBeFocused();
|
||||
// Activate AM via Space — the canonical button-keyboard contract.
|
||||
await page.keyboard.press("Space");
|
||||
await page.keyboard.press("Tab");
|
||||
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 end input — that's the regression this test guards.
|
||||
await expect(startInput).toHaveValue("7:15");
|
||||
|
||||
await page.keyboard.type("8: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".
|
||||
await page.keyboard.press("Tab");
|
||||
await expect(endPm).toBeFocused();
|
||||
await endInput.focus();
|
||||
|
||||
const savePromise = page.waitForResponse(
|
||||
(resp) => {
|
||||
|
||||
Reference in New Issue
Block a user