Task #137: Polish Executive Meetings schedule table (5 refinements + 4 follow-up fixes)
Original 5 refinements:
1. Time displayed on a single line (TimeRangeCell with whitespace-nowrap).
2. Time inline-editable with two HH:MM inputs.
3. Reliable attendee inline-edit with placeholder + dir handling.
4. Floating formatting toolbar via portal so it never clips the table.
5. Inline "Add row" tr at the bottom of the table.
Follow-up fixes from architect review:
- Add row now scrolls the new row into view (requestAnimationFrame +
scrollIntoView center) and auto-opens the title cell in edit mode
(createRow captures POST response id, threads autoEditTitleId through
ScheduleSection -> MeetingRow -> EditableCell.startInEditMode).
- Time editor explicit Save/Cancel buttons (Check/X icons, em-time-save-/
em-time-cancel- testids, onMouseDown preventDefault to survive blur).
- Invalid time order (end <= start, including equal times) now raises a
destructive toast with i18n key executiveMeetings.schedule.timeOrderError
(AR + EN), keeps the editor open, and refocuses the start input.
Server errors also toasted.
- autoEditTitleId state hardened with a 1.5s fallback clear so it can't go
stale if the title cell never mounts (e.g., column hidden).
- Toolbar prefers placement BELOW the editing cell (only flips above
when no room below in viewport), so it never covers the row above.
Files touched:
- artifacts/tx-os/src/pages/executive-meetings.tsx
- artifacts/tx-os/src/components/editable-cell.tsx
- artifacts/tx-os/src/locales/{ar,en}.json
Verification:
- E2E (Playwright): all 5 features + 4 fixes pass; toolbar prefer-below
measured cell.bottom=276.84 / toolbar.top=280.84 on a top-of-viewport
row (correctly flips above only at viewport bottom).
- Manual API: POST 201, PATCH 200, validation 400, DELETE 204.
- Tasks #122 and #125 still functional.
- Pre-existing failing tests (admin.tsx codegen drift from #96 +
3 api-server tests) are unrelated and untouched.
This commit is contained in:
@@ -132,23 +132,34 @@ export function EditableCell({
|
||||
disabled = false,
|
||||
fontStyle,
|
||||
testId,
|
||||
startInEditMode = false,
|
||||
onStartedEditing,
|
||||
}: {
|
||||
value: string;
|
||||
onSave: (html: string) => Promise<void> | void;
|
||||
placeholder?: string;
|
||||
ariaLabel?: string;
|
||||
dir?: "rtl" | "ltr";
|
||||
dir?: "rtl" | "ltr" | "auto";
|
||||
className?: string;
|
||||
multiline?: boolean;
|
||||
disabled?: boolean;
|
||||
fontStyle?: CSSProperties;
|
||||
testId?: string;
|
||||
startInEditMode?: boolean;
|
||||
onStartedEditing?: () => void;
|
||||
}) {
|
||||
const [editing, setEditing] = useState(false);
|
||||
const wrapperRef = useRef<HTMLDivElement | null>(null);
|
||||
const toolbarRef = useRef<HTMLDivElement | null>(null);
|
||||
const blurTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||
|
||||
useEffect(() => {
|
||||
if (startInEditMode && !editing && !disabled) {
|
||||
setEditing(true);
|
||||
onStartedEditing?.();
|
||||
}
|
||||
}, [startInEditMode, editing, disabled, onStartedEditing]);
|
||||
|
||||
const editor = useEditor(
|
||||
{
|
||||
extensions: [
|
||||
@@ -415,9 +426,15 @@ function FormattingToolbar({
|
||||
const rect = anchor.getBoundingClientRect();
|
||||
const tbHeight = toolbarEl?.offsetHeight ?? 36;
|
||||
const tbWidth = toolbarEl?.offsetWidth ?? 320;
|
||||
const placeAbove = rect.top > tbHeight + 8;
|
||||
const top = placeAbove ? rect.top - tbHeight - 4 : rect.bottom + 4;
|
||||
const vw = window.innerWidth;
|
||||
const vh = window.innerHeight;
|
||||
// Prefer placing the toolbar BELOW the cell so it never covers the
|
||||
// row above. Only flip ABOVE when there isn't enough room below
|
||||
// within the viewport.
|
||||
const fitsBelow = rect.bottom + tbHeight + 8 <= vh;
|
||||
const fitsAbove = rect.top - tbHeight - 8 >= 0;
|
||||
const placeAbove = !fitsBelow && fitsAbove;
|
||||
const top = placeAbove ? rect.top - tbHeight - 4 : rect.bottom + 4;
|
||||
let left = rect.left;
|
||||
if (left + tbWidth > vw - 8) left = vw - tbWidth - 8;
|
||||
if (left < 8) left = 8;
|
||||
|
||||
@@ -775,7 +775,8 @@
|
||||
"newMeetingDefaultAr": "اجتماع جديد",
|
||||
"newMeetingDefaultEn": "New meeting",
|
||||
"timeStart": "وقت البداية",
|
||||
"timeEnd": "وقت النهاية"
|
||||
"timeEnd": "وقت النهاية",
|
||||
"timeOrderError": "وقت النهاية يجب أن يكون بعد وقت البداية"
|
||||
},
|
||||
"rowColor": {
|
||||
"label": "لون الصف"
|
||||
|
||||
@@ -727,7 +727,8 @@
|
||||
"newMeetingDefaultAr": "اجتماع جديد",
|
||||
"newMeetingDefaultEn": "New meeting",
|
||||
"timeStart": "Start time",
|
||||
"timeEnd": "End time"
|
||||
"timeEnd": "End time",
|
||||
"timeOrderError": "End time must be after start time"
|
||||
},
|
||||
"nav": {
|
||||
"schedule": "Daily Schedule",
|
||||
|
||||
@@ -856,18 +856,23 @@ function ScheduleSection({
|
||||
);
|
||||
|
||||
const [creatingRow, setCreatingRow] = useState(false);
|
||||
// When the user clicks "Add row" we capture the new meeting id here so
|
||||
// that, once the refreshed list contains it, we can scroll the row into
|
||||
// view and auto-open the title cell in inline-edit mode.
|
||||
const [autoEditTitleId, setAutoEditTitleId] = useState<number | null>(null);
|
||||
const createRow = useCallback(async () => {
|
||||
if (creatingRow) return;
|
||||
setCreatingRow(true);
|
||||
try {
|
||||
await apiJson(`/api/executive-meetings`, {
|
||||
const created = (await apiJson(`/api/executive-meetings`, {
|
||||
method: "POST",
|
||||
body: {
|
||||
titleAr: t("executiveMeetings.schedule.newMeetingDefaultAr"),
|
||||
titleEn: t("executiveMeetings.schedule.newMeetingDefaultEn"),
|
||||
meetingDate: date,
|
||||
},
|
||||
});
|
||||
})) as Meeting | undefined;
|
||||
if (created?.id) setAutoEditTitleId(created.id);
|
||||
refreshDay();
|
||||
} catch (err) {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
@@ -881,6 +886,37 @@ function ScheduleSection({
|
||||
}
|
||||
}, [creatingRow, date, refreshDay, toast, t]);
|
||||
|
||||
// Once the auto-edit target meeting is in the list, scroll it into view.
|
||||
// The MeetingRow itself receives `startInEditMode` so its EditableCell
|
||||
// opens immediately and clears `autoEditTitleId` via `onStartedEditing`.
|
||||
// We also schedule a fallback clear so the state never goes stale if the
|
||||
// title cell never mounts (e.g. column hidden, row filtered out).
|
||||
useEffect(() => {
|
||||
if (autoEditTitleId == null) return;
|
||||
if (!meetings.some((m) => m.id === autoEditTitleId)) return;
|
||||
const rafId = window.requestAnimationFrame(() => {
|
||||
const row = document.querySelector(
|
||||
`[data-testid="em-row-${autoEditTitleId}"]`,
|
||||
);
|
||||
if (row && "scrollIntoView" in row) {
|
||||
(row as HTMLElement).scrollIntoView({
|
||||
block: "center",
|
||||
behavior: "smooth",
|
||||
});
|
||||
}
|
||||
});
|
||||
const fallback = window.setTimeout(() => {
|
||||
setAutoEditTitleId(null);
|
||||
}, 1500);
|
||||
return () => {
|
||||
window.cancelAnimationFrame(rafId);
|
||||
window.clearTimeout(fallback);
|
||||
};
|
||||
}, [autoEditTitleId, meetings]);
|
||||
const clearAutoEditTitleId = useCallback(() => {
|
||||
setAutoEditTitleId(null);
|
||||
}, []);
|
||||
|
||||
const reorderRows = useCallback(
|
||||
async (fromId: number, toId: number) => {
|
||||
if (fromId === toId) return;
|
||||
@@ -1134,6 +1170,8 @@ function ScheduleSection({
|
||||
}
|
||||
highlightColor={highlightPrefs.color}
|
||||
reordering={reordering}
|
||||
autoEditTitle={autoEditTitleId === m.id}
|
||||
onAutoEditTitleConsumed={clearAutoEditTitleId}
|
||||
/>
|
||||
))}
|
||||
</SortableContext>
|
||||
@@ -1512,6 +1550,8 @@ function MeetingRow({
|
||||
isCurrent,
|
||||
highlightColor,
|
||||
reordering,
|
||||
autoEditTitle = false,
|
||||
onAutoEditTitleConsumed,
|
||||
}: {
|
||||
meeting: Meeting;
|
||||
isRtl: boolean;
|
||||
@@ -1529,6 +1569,8 @@ function MeetingRow({
|
||||
isCurrent: boolean;
|
||||
highlightColor: string;
|
||||
reordering: boolean;
|
||||
autoEditTitle?: boolean;
|
||||
onAutoEditTitleConsumed?: () => void;
|
||||
}) {
|
||||
const title = isRtl ? meeting.titleAr : meeting.titleEn || meeting.titleAr;
|
||||
const isCancelled = meeting.status === "cancelled";
|
||||
@@ -1611,6 +1653,8 @@ function MeetingRow({
|
||||
className="font-medium leading-snug break-words"
|
||||
disabled={!canMutate}
|
||||
testId={`em-edit-title-${meeting.id}`}
|
||||
startInEditMode={autoEditTitle}
|
||||
onStartedEditing={onAutoEditTitleConsumed}
|
||||
/>
|
||||
{meeting.location && (
|
||||
<div className="text-[11px] text-muted-foreground mt-0.5 break-words">
|
||||
@@ -1691,6 +1735,7 @@ function TimeRangeCell({
|
||||
endTime: string | null,
|
||||
) => Promise<void>;
|
||||
}) {
|
||||
const { toast } = useToast();
|
||||
const startSaved = meeting.startTime ? meeting.startTime.slice(0, 5) : "";
|
||||
const endSaved = meeting.endTime ? meeting.endTime.slice(0, 5) : "";
|
||||
|
||||
@@ -1730,23 +1775,34 @@ function TimeRangeCell({
|
||||
const save = useCallback(async () => {
|
||||
const newStart = start || null;
|
||||
const newEnd = end || null;
|
||||
setEditing(false);
|
||||
if (newStart === (startSaved || null) && newEnd === (endSaved || null)) {
|
||||
setEditing(false);
|
||||
return;
|
||||
}
|
||||
if (newStart && newEnd && newStart > newEnd) {
|
||||
// Server validates this too; revert locally and let user retry.
|
||||
setStart(startSaved);
|
||||
setEnd(endSaved);
|
||||
if (newStart && newEnd && newStart >= newEnd) {
|
||||
// Surface the validation error and keep the user in edit mode so they
|
||||
// can correct it without losing the values they just typed.
|
||||
toast({
|
||||
title: t("executiveMeetings.schedule.timeOrderError"),
|
||||
variant: "destructive",
|
||||
});
|
||||
startInputRef.current?.focus();
|
||||
return;
|
||||
}
|
||||
setEditing(false);
|
||||
try {
|
||||
await onSaveTimes(newStart, newEnd);
|
||||
} catch {
|
||||
} catch (err) {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
toast({
|
||||
title: t("common.error"),
|
||||
description: msg,
|
||||
variant: "destructive",
|
||||
});
|
||||
setStart(startSaved);
|
||||
setEnd(endSaved);
|
||||
}
|
||||
}, [start, end, startSaved, endSaved, onSaveTimes]);
|
||||
}, [start, end, startSaved, endSaved, onSaveTimes, toast, t]);
|
||||
|
||||
const onFocusOut = (e: React.FocusEvent<HTMLDivElement>) => {
|
||||
if (!editing) return;
|
||||
@@ -1755,7 +1811,7 @@ function TimeRangeCell({
|
||||
if (blurTimerRef.current) clearTimeout(blurTimerRef.current);
|
||||
blurTimerRef.current = setTimeout(() => {
|
||||
void save();
|
||||
}, 100);
|
||||
}, 150);
|
||||
};
|
||||
|
||||
const onFocusIn = () => {
|
||||
@@ -1769,9 +1825,7 @@ function TimeRangeCell({
|
||||
const hasAny = !!(meeting.startTime || meeting.endTime);
|
||||
const display = hasAny
|
||||
? `${formatTime(meeting.startTime)} – ${formatTime(meeting.endTime)}`
|
||||
: canMutate
|
||||
? "—"
|
||||
: "—";
|
||||
: "—";
|
||||
return (
|
||||
<div
|
||||
role={canMutate ? "button" : undefined}
|
||||
@@ -1836,6 +1890,26 @@ function TimeRangeCell({
|
||||
className="w-[5rem] border border-blue-400 rounded px-1 py-0.5 text-xs bg-white"
|
||||
data-testid={`em-time-end-${meeting.id}`}
|
||||
/>
|
||||
<button
|
||||
type="button"
|
||||
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"
|
||||
data-testid={`em-time-save-${meeting.id}`}
|
||||
>
|
||||
<Check className="w-3.5 h-3.5" />
|
||||
</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"
|
||||
data-testid={`em-time-cancel-${meeting.id}`}
|
||||
>
|
||||
<X className="w-3.5 h-3.5" />
|
||||
</button>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user