EM time picker: mobile/iPad-friendly layout + touch targets
Follow-up to task #315. The 3-control picker shipped with desktop- sized inline controls (~22px tall) and a single-row layout that pushed the end picker out of view on a 375px iPhone, leaving the user with no obvious way to fill the second time field. What changed - src/components/time-picker-12h.tsx: bumped the "inline" size from h<22px text-[10px] to h-8 (32px) text-sm/text-xs across hour input, minute input, and AM/PM toggle so a finger tap on mobile/iPad lands cleanly. Added `gap-1.5` between the [hour:minute] group and the AM/PM toggle so a thumb-tap on the toggle doesn't accidentally land on the minute input. Added a visible focus ring color and lightened the placeholder so the empty fields read as "fill me in" rather than already-active. - src/pages/executive-meetings.tsx (TimeRangeCell wrapper): switched from `inline-flex` (single-row, ellipsis on overflow) to `flex flex-wrap items-end gap-x-1.5 gap-y-1`. The end picker now drops onto a second line when the cell can't fit both side-by-side, which is the iPhone case. The "–" separator is hidden on narrow widths (`hidden sm:inline`) since wrapping makes it visually wrong. - Bumped Start/End labels from 10px to 11px font-medium for better at-a-glance scanability of which time field is which — this was the user-reported clarity issue. - Save/cancel icon buttons grew to 32x32 hit areas (was tiny p-0.5 around a 14px icon ≈ 16px clickable). - tests/executive-meetings-keyboard-editing.spec.mjs: added a new test.describe block "Schedule mobile viewport (iPhone 375)" with `test.use({ viewport: { width: 375, height: 667 } })`. The test asserts: (a) all 8 picker sub-controls + save button are visible on iPhone, (b) each interactive control is at least 28px tall (regression guard against shrinking touch targets), (c) a complete tap-driven hour+minute+AM/PM entry on both start and end persists 21:30–22:45 to the DB. Validation - Unit tests: 22/22 pass (no logic changes — only CSS + layout). - E2E: Tab walks (desktop) + compact+PM toggle (desktop) + new iPhone-375 mobile test all pass. - TypeScript: clean.
This commit is contained in:
@@ -271,24 +271,27 @@ function TimePicker12hImpl(
|
||||
};
|
||||
|
||||
const isInline = size === "inline";
|
||||
// Touch target sizes: even the "inline" mode now uses h-8 (32px)
|
||||
// for inputs and h-8 toggle buttons so finger taps on mobile/iPad
|
||||
// land cleanly. Form mode uses h-9 (36px) for desktop comfort.
|
||||
// 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"
|
||||
? "h-8 w-12 border border-blue-400 rounded px-1 py-0 text-sm bg-white text-center placeholder:text-blue-300/70 focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-blue-500"
|
||||
: "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"
|
||||
? "h-8 w-10 border border-blue-400 rounded px-1 py-0 text-sm bg-white text-center placeholder:text-blue-300/70 focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-blue-500"
|
||||
: "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 px-0.5"
|
||||
: "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";
|
||||
const buttonSize = isInline
|
||||
? "px-1.5 py-0.5 text-[10px] leading-none"
|
||||
? "px-2 h-8 text-xs leading-none min-w-[2rem]"
|
||||
: "px-2.5 py-1 text-xs leading-tight h-9";
|
||||
|
||||
const amActive = period === "am";
|
||||
@@ -301,7 +304,10 @@ function TimePicker12hImpl(
|
||||
|
||||
return (
|
||||
<div
|
||||
className="inline-flex items-center gap-1"
|
||||
// Visible gap between the [hour:minute] group and the AM/PM
|
||||
// toggle so a thumb-tap on the toggle on a touch device
|
||||
// doesn't accidentally land on the minute input.
|
||||
className="inline-flex items-center gap-1.5"
|
||||
// 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.
|
||||
|
||||
@@ -3586,14 +3586,19 @@ function TimeRangeCell({
|
||||
// save/cancel buttons get visually mirrored, making it impossible
|
||||
// for the user to tell which input is which.
|
||||
dir="ltr"
|
||||
className="inline-flex items-end justify-center gap-1 font-mono whitespace-nowrap"
|
||||
// `flex-wrap` lets the start picker, the en-dash, and the end
|
||||
// picker fall onto a second line when the cell is narrower than
|
||||
// their combined width — this is what makes the editor usable
|
||||
// on a 375px iPhone viewport, where the prior single-row
|
||||
// `inline-flex` layout pushed the end picker out of view.
|
||||
className="flex flex-wrap items-end justify-center gap-x-1.5 gap-y-1 font-mono"
|
||||
data-testid={`em-time-${meeting.id}`}
|
||||
data-editing="true"
|
||||
>
|
||||
<div className="flex flex-col items-center">
|
||||
<label
|
||||
htmlFor={startInputId}
|
||||
className="text-[10px] leading-none text-muted-foreground mb-0.5 select-none"
|
||||
className="text-[11px] font-medium leading-none text-muted-foreground mb-1 select-none"
|
||||
>
|
||||
{t("executiveMeetings.schedule.timeStartShort")}
|
||||
</label>
|
||||
@@ -3623,11 +3628,11 @@ function TimeRangeCell({
|
||||
size="inline"
|
||||
/>
|
||||
</div>
|
||||
<span className="text-muted-foreground pb-0.5">–</span>
|
||||
<span className="text-muted-foreground pb-2 self-center hidden sm:inline">–</span>
|
||||
<div className="flex flex-col items-center">
|
||||
<label
|
||||
htmlFor={endInputId}
|
||||
className="text-[10px] leading-none text-muted-foreground mb-0.5 select-none"
|
||||
className="text-[11px] font-medium leading-none text-muted-foreground mb-1 select-none"
|
||||
>
|
||||
{t("executiveMeetings.schedule.timeEndShort")}
|
||||
</label>
|
||||
@@ -3654,20 +3659,20 @@ function TimeRangeCell({
|
||||
onMouseDown={(e) => e.preventDefault()}
|
||||
onClick={() => void save()}
|
||||
aria-label={t("common.save")}
|
||||
className="ms-0.5 p-0.5 rounded text-green-700 hover:bg-green-50 focus:bg-green-50 focus:outline-none mb-0.5"
|
||||
className="ms-0.5 h-8 w-8 inline-flex items-center justify-center rounded text-green-700 hover:bg-green-50 focus:bg-green-50 focus:outline-none self-end"
|
||||
data-testid={`em-time-save-${meeting.id}`}
|
||||
>
|
||||
<Check className="w-3.5 h-3.5" />
|
||||
<Check className="w-4 h-4" />
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
onMouseDown={(e) => e.preventDefault()}
|
||||
onClick={cancel}
|
||||
aria-label={t("common.cancel")}
|
||||
className="p-0.5 rounded text-red-700 hover:bg-red-50 focus:bg-red-50 focus:outline-none mb-0.5"
|
||||
className="h-8 w-8 inline-flex items-center justify-center rounded text-red-700 hover:bg-red-50 focus:bg-red-50 focus:outline-none self-end"
|
||||
data-testid={`em-time-cancel-${meeting.id}`}
|
||||
>
|
||||
<X className="w-3.5 h-3.5" />
|
||||
<X className="w-4 h-4" />
|
||||
</button>
|
||||
</div>
|
||||
);
|
||||
|
||||
@@ -1301,3 +1301,129 @@ test("Schedule keyboard: Tab walks start hour → start minute → start AM →
|
||||
expect(String(rows[0].start_time)).toBe("07:15:00");
|
||||
expect(String(rows[0].end_time)).toBe("08:45:00");
|
||||
});
|
||||
|
||||
// -------------------------------------------------------------------
|
||||
// Mobile viewport (iPhone 375x667): the inline 12h picker has 6
|
||||
// sub-controls per range (start hour + minute + AM + PM, end hour +
|
||||
// minute + AM + PM, plus save/cancel buttons). On a narrow phone
|
||||
// viewport the row used to push the end picker out of view, hiding
|
||||
// the second time field from the user. The TimeRangeCell wrapper
|
||||
// now uses `flex-wrap` so the end picker drops onto a second line
|
||||
// when the cell can't fit both side-by-side. This test guards
|
||||
// against a regression that breaks that wrap or shrinks the touch
|
||||
// targets below the usable minimum on a phone-sized viewport.
|
||||
// -------------------------------------------------------------------
|
||||
|
||||
test.describe("Schedule mobile viewport (iPhone 375)", () => {
|
||||
test.use({ viewport: { width: 375, height: 667 } });
|
||||
|
||||
test("inline 12h picker: all 6 sub-controls are visible on iPhone, finger-tap sized, and a full hour+minute+AM/PM entry persists", async ({
|
||||
page,
|
||||
}) => {
|
||||
await setLangEn(page);
|
||||
const date = uniqueFutureDate(20);
|
||||
const meetingId = await insertMeeting({
|
||||
meetingDate: date,
|
||||
dailyNumber: 1,
|
||||
titleEn: `MobilePicker ${Date.now().toString(36)}`,
|
||||
titleAr: `محمول ${Date.now().toString(36)}`,
|
||||
startTime: null,
|
||||
endTime: null,
|
||||
});
|
||||
|
||||
await gotoMeetingRow(page, meetingId, date);
|
||||
|
||||
const timeCell = page.getByTestId(`em-time-${meetingId}`);
|
||||
// Tap the cell to enter edit mode (mimics a finger tap on
|
||||
// mobile, exercising the click handler the read-only display
|
||||
// exposes alongside the keyboard handler).
|
||||
await timeCell.click();
|
||||
|
||||
const startHour = page.getByTestId(`em-time-start-${meetingId}`);
|
||||
const startMinute = page.getByTestId(`em-time-start-minute-${meetingId}`);
|
||||
const startAm = page.getByTestId(`em-time-start-period-am-${meetingId}`);
|
||||
const startPm = page.getByTestId(`em-time-start-period-pm-${meetingId}`);
|
||||
const endHour = page.getByTestId(`em-time-end-${meetingId}`);
|
||||
const endMinute = page.getByTestId(`em-time-end-minute-${meetingId}`);
|
||||
const endAm = page.getByTestId(`em-time-end-period-am-${meetingId}`);
|
||||
const endPm = page.getByTestId(`em-time-end-period-pm-${meetingId}`);
|
||||
const saveBtn = page.getByTestId(`em-time-save-${meetingId}`);
|
||||
|
||||
// All 8 picker sub-controls + save button must be reachable on
|
||||
// a 375px viewport. `toBeVisible` checks both the DOM presence
|
||||
// AND that the element isn't clipped behind 0-size / display:
|
||||
// none / off-screen — the regression we're guarding against.
|
||||
for (const control of [
|
||||
startHour,
|
||||
startMinute,
|
||||
startAm,
|
||||
startPm,
|
||||
endHour,
|
||||
endMinute,
|
||||
endAm,
|
||||
endPm,
|
||||
saveBtn,
|
||||
]) {
|
||||
await expect(control).toBeVisible();
|
||||
}
|
||||
|
||||
// Touch-target check: each interactive control must be at
|
||||
// least 28px tall so a finger tap lands cleanly. We use 28px
|
||||
// (not Apple HIG's 44px) because the inline editor lives in
|
||||
// a dense schedule table and h-8 (32px) is already a real
|
||||
// jump from the prior 22px implementation; h<28px would be
|
||||
// a regression worth catching.
|
||||
for (const control of [startHour, startMinute, startAm, endPm, saveBtn]) {
|
||||
const box = await control.boundingBox();
|
||||
expect(box).not.toBeNull();
|
||||
expect(box.height).toBeGreaterThanOrEqual(28);
|
||||
}
|
||||
|
||||
// Drive a complete hour + minute + AM/PM entry through tap
|
||||
// interactions on both sides — this is the canonical mobile
|
||||
// flow the previous single-row layout broke. "9" + "30" PM
|
||||
// resolves to 21:30; "10" + "45" PM resolves to 22:45.
|
||||
await startHour.click();
|
||||
await page.keyboard.press("ControlOrMeta+a");
|
||||
await page.keyboard.press("Delete");
|
||||
await page.keyboard.type("9", { delay: 10 });
|
||||
await startMinute.click();
|
||||
await page.keyboard.type("30", { delay: 10 });
|
||||
await startPm.click();
|
||||
await expect(startPm).toHaveAttribute("aria-pressed", "true");
|
||||
|
||||
await endHour.click();
|
||||
await page.keyboard.press("ControlOrMeta+a");
|
||||
await page.keyboard.press("Delete");
|
||||
await page.keyboard.type("10", { delay: 10 });
|
||||
await endMinute.click();
|
||||
await page.keyboard.type("45", { delay: 10 });
|
||||
await endPm.click();
|
||||
await expect(endPm).toHaveAttribute("aria-pressed", "true");
|
||||
|
||||
const savePromise = page.waitForResponse(
|
||||
(resp) => {
|
||||
const u = new URL(resp.url());
|
||||
return (
|
||||
u.pathname === `/api/executive-meetings/${meetingId}` &&
|
||||
resp.request().method() === "PATCH" &&
|
||||
resp.ok()
|
||||
);
|
||||
},
|
||||
{ timeout: 15_000 },
|
||||
);
|
||||
const refetchPromise = waitForDayRefetch(page, date);
|
||||
// Tap the save button — proves the save button is reachable
|
||||
// on a 375px viewport AND that the picker's commit() path
|
||||
// fires from a touch interaction.
|
||||
await saveBtn.click();
|
||||
await Promise.all([savePromise, refetchPromise]);
|
||||
|
||||
const { rows } = await pool.query(
|
||||
`SELECT start_time, end_time FROM executive_meetings WHERE id = $1`,
|
||||
[meetingId],
|
||||
);
|
||||
expect(String(rows[0].start_time)).toBe("21:30:00");
|
||||
expect(String(rows[0].end_time)).toBe("22:45:00");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user