Task #191: Consolidate row-cell overlay icons into single kebab menu

The schedule's per-row affordances (delete, row color, merge cells)
used to render as three separate hover-revealed icon buttons stacked
inside the same narrow cell, which collided visually on iPad and
small screens. Replaced them with a single MoreVertical kebab trigger
that opens a Radix popover with three views (main / color / merge),
plus a "Back" affordance to return from sub-views to the main menu.

Implementation:
- New RowActionsMenu component (artifacts/tx-os/src/pages/executive-meetings.tsx)
  rendered once per row, anchored at the trailing-top of the first
  visible cell (matching the previous overlay anchor logic). Resets
  to "main" view on close.
- Old DeleteRowButton, RowColorPicker, and MergeMenu components removed.
- Existing testids preserved on the popover items so external tests
  still target em-delete-row-{id}, em-row-color-trigger, and
  em-merge-trigger-{id}. New testids added: em-row-actions-{id} for
  the kebab trigger, em-row-actions-back for the Back button.
- Locale keys added in ar.json/en.json: rowActions.label, rowActions.back.
- New imports: MoreVertical, ChevronLeft, ChevronRight (RTL-aware Back arrow).
- Merged-cell branch also uses RowActionsMenu so merge/unmerge stays
  reachable when the # column is hidden.

Tests:
- New regression spec: executive-meetings-row-actions-menu.spec.mjs
  exercises the full menu flow (open → color → back → main → merge → escape).
- Existing executive-meetings-edit-toggle.spec.mjs and
  executive-meetings-schedule-features.spec.mjs continue to pass —
  legacy testids remain absent in view mode and present inside the
  popover when opened in edit mode.

Pre-existing test failures (NOT caused by this change):
- "non-mutate user (executive_viewer)" spec fails because seeded user
  "ahmed" is missing from the dev DB (parallel task #131 drizzle work).
- TS errors in admin.tsx tracked by other tasks (api-client codegen
  out of sync).

Follow-up proposed:
- #192: show row color + merge previews inline in the main menu items.
This commit is contained in:
riyadhafraa
2026-04-30 06:24:57 +00:00
parent 6949ab1009
commit 311d59ccad
5 changed files with 326 additions and 181 deletions
Binary file not shown.

Before

Width:  |  Height:  |  Size: 24 KiB

After

Width:  |  Height:  |  Size: 25 KiB

+4
View File
@@ -877,6 +877,10 @@
"rowColor": {
"label": "لون الصف"
},
"rowActions": {
"label": "إجراءات الصف",
"back": "رجوع"
},
"merge": {
"label": "دمج الخلايا",
"editLabel": "تعديل الخلية المدموجة",
+4
View File
@@ -793,6 +793,10 @@
"rowColor": {
"label": "Row color"
},
"rowActions": {
"label": "Row actions",
"back": "Back"
},
"merge": {
"label": "Merge cells",
"editLabel": "Edit merged cell",
+212 -181
View File
@@ -39,6 +39,9 @@ import {
Palette,
Sliders,
Combine,
MoreVertical,
ChevronLeft,
ChevronRight,
} from "lucide-react";
import { Button } from "@/components/ui/button";
import { Input } from "@/components/ui/input";
@@ -1856,74 +1859,214 @@ function RowColorPickerSwatches({
);
}
function RowColorPicker({
current,
onPick,
// Consolidated row-level actions menu. Replaces the previous three
// separate overlay triggers (Trash, Palette, Combine icons) that all
// floated inside the same narrow cell and visually collided on small
// screens / iPad. Now there is exactly one kebab button per row, and
// it opens a small popover that exposes:
// 1. Delete row — direct action, fires onDelete()
// 2. Row color — expands inline to RowColorPickerSwatches
// 3. Merge cells — expands inline to the merge options list
// Both expansion views show a "Back" link so the user can return to
// the main menu without closing the popover.
//
// Anchor / positioning rules match the old triggers exactly:
// - Hover-reveal on desktop, low-opacity-always on touch
// - Hidden in print
// - Uses `inline-end-1` so RTL layouts flip the button to the
// correct corner.
//
// The host <td> must carry `relative group` for the positioning +
// hover reveal to work — every renderCell branch already does.
//
// Test ids are preserved on the corresponding menu items so existing
// Playwright assertions about presence/absence keep working:
// em-delete-row-{id}, em-row-color-trigger, em-merge-trigger-{id}.
function RowActionsMenu({
meetingId,
isDeleting,
onDelete,
rowColorKey,
onPickRowColor,
hasStoredMerge,
existingMergeText,
onChangeMerge,
t,
}: {
current: string;
onPick: (key: string) => void;
meetingId: number;
isDeleting: boolean;
onDelete: () => void;
rowColorKey: string;
onPickRowColor: (key: string) => void;
hasStoredMerge: boolean;
existingMergeText: string | null;
onChangeMerge: (
merge:
| { mergeStartColumn: ColumnId; mergeEndColumn: ColumnId; mergeText: string }
| null,
) => Promise<void>;
t: (k: string) => string;
}) {
const [open, setOpen] = useState(false);
const [view, setView] = useState<"main" | "color" | "merge">("main");
// Always reset to the main menu whenever the popover closes, so the
// next open lands the user back on Delete/Color/Merge instead of a
// half-finished sub-view.
useEffect(() => {
if (!open) setView("main");
}, [open]);
const applyMerge = async (start: ColumnId, end: ColumnId) => {
setOpen(false);
await onChangeMerge({
mergeStartColumn: start,
mergeEndColumn: end,
mergeText: existingMergeText ?? "",
});
};
const itemCls =
"w-full text-start px-2 py-1.5 text-sm rounded hover:bg-accent flex items-center gap-2 disabled:opacity-50 disabled:cursor-not-allowed";
return (
<Popover>
<Popover open={open} onOpenChange={setOpen}>
<PopoverTrigger asChild>
<button
type="button"
className="absolute top-1 inline-end-1 p-1 rounded text-muted-foreground hover:text-[#0B1E3F] hover:bg-white/70 opacity-0 group-hover:opacity-100 focus:opacity-100 transition-opacity"
aria-label={t("executiveMeetings.rowColor.label")}
data-testid="em-row-color-trigger"
// NOTE: do NOT add onPointerDown stopPropagation here — Radix
// Popover uses pointerdown to open and our handler would
// swallow it. The host cell has no drag listeners (only the
// grip button does), so nothing else needs blocking.
aria-label={t("executiveMeetings.rowActions.label")}
title={t("executiveMeetings.rowActions.label")}
data-testid={`em-row-actions-${meetingId}`}
className="absolute top-1 inline-end-1 p-1 rounded text-muted-foreground hover:text-[#0B1E3F] hover:bg-white/80 opacity-0 group-hover:opacity-100 focus:opacity-100 [@media(hover:none)_and_(pointer:coarse)]:opacity-40 [@media(hover:none)_and_(pointer:coarse)]:p-1.5 transition-opacity print:hidden"
>
<Palette className="w-3.5 h-3.5" />
<MoreVertical className="w-3.5 h-3.5 [@media(hover:none)_and_(pointer:coarse)]:w-4 [@media(hover:none)_and_(pointer:coarse)]:h-4" />
</button>
</PopoverTrigger>
<PopoverContent className="w-auto p-2" align="end">
<RowColorPickerSwatches current={current} onPick={onPick} t={t} />
<PopoverContent className="w-56 p-1" align="end">
{view === "main" && (
<>
<button
type="button"
className={`${itemCls} text-destructive`}
disabled={isDeleting}
onClick={(e) => {
e.stopPropagation();
setOpen(false);
onDelete();
}}
data-testid={`em-delete-row-${meetingId}`}
>
<Trash2 className="w-3.5 h-3.5" aria-hidden="true" />
{t("executiveMeetings.schedule.deleteRow")}
</button>
<button
type="button"
className={itemCls}
onClick={() => setView("color")}
data-testid="em-row-color-trigger"
>
<Palette className="w-3.5 h-3.5" aria-hidden="true" />
{t("executiveMeetings.rowColor.label")}
</button>
<button
type="button"
className={itemCls}
onClick={() => setView("merge")}
data-testid={`em-merge-trigger-${meetingId}`}
>
<Combine className="w-3.5 h-3.5" aria-hidden="true" />
{t("executiveMeetings.merge.label")}
</button>
</>
)}
{view === "color" && (
<>
<BackToMainButton onClick={() => setView("main")} t={t} />
<div className="px-1 pb-1">
<RowColorPickerSwatches
current={rowColorKey}
onPick={(key) => {
onPickRowColor(key);
setOpen(false);
}}
t={t}
/>
</div>
</>
)}
{view === "merge" && (
<>
<BackToMainButton onClick={() => setView("main")} t={t} />
<button
type="button"
className="w-full text-start px-2 py-1.5 text-sm rounded hover:bg-accent"
data-testid={`em-merge-opt-meeting-attendees-${meetingId}`}
onClick={() => applyMerge("meeting", "attendees")}
>
{t("executiveMeetings.merge.meetingAttendees")}
</button>
<button
type="button"
className="w-full text-start px-2 py-1.5 text-sm rounded hover:bg-accent"
data-testid={`em-merge-opt-meeting-time-${meetingId}`}
onClick={() => applyMerge("meeting", "time")}
>
{t("executiveMeetings.merge.meetingTime")}
</button>
<button
type="button"
className="w-full text-start px-2 py-1.5 text-sm rounded hover:bg-accent"
data-testid={`em-merge-opt-all-${meetingId}`}
onClick={() => applyMerge("number", "time")}
>
{t("executiveMeetings.merge.all")}
</button>
{hasStoredMerge && (
<button
type="button"
className="w-full text-start px-2 py-1.5 text-sm rounded hover:bg-accent text-destructive"
data-testid={`em-merge-opt-unmerge-${meetingId}`}
onClick={() => {
setOpen(false);
onChangeMerge(null);
}}
>
{t("executiveMeetings.merge.unmerge")}
</button>
)}
</>
)}
</PopoverContent>
</Popover>
);
}
// Reusable row-level "delete entire meeting" trigger. Rendered as an
// absolutely-positioned overlay inside whichever cell is the row's
// current anchor (first visible cell on unmerged rows, the merged
// cell on merged rows) so it stays available regardless of which
// columns the user has hidden via the column customizer. The host
// <td> must carry `relative group` for the positioning + hover
// reveal to work, which every renderCell branch already does.
function DeleteRowButton({
meetingId,
isDeleting,
onDelete,
label,
// Small "Back" affordance shown at the top of the row-actions submenu
// views (Row color, Merge cells). RTL-aware: uses ChevronRight in
// Arabic so the arrow points back toward the main menu visually.
function BackToMainButton({
onClick,
t,
}: {
meetingId: number;
isDeleting: boolean;
onDelete: () => void;
label: string;
onClick: () => void;
t: (k: string) => string;
}) {
const isRtl =
typeof document !== "undefined" && document.documentElement.dir === "rtl";
const ArrowIcon = isRtl ? ChevronRight : ChevronLeft;
return (
<button
type="button"
// Pinned to the cell's leading-top corner. The color picker (on
// # cell only) sits at top-end, the merge trigger sits at
// bottom-end, so the three row-level overlays don't collide.
// Hover-reveal on desktop, ~40% on touch (same pattern as the
// merge trigger and grip handle), focusable for keyboard users,
// hidden in print. Stops propagation so the click never bubbles
// into row drag or inline-edit affordances.
onClick={(e) => {
e.stopPropagation();
onDelete();
}}
onPointerDown={(e) => e.stopPropagation()}
disabled={isDeleting}
className="absolute top-1 inline-start-1 p-1 rounded text-destructive/80 hover:text-destructive hover:bg-white/80 opacity-0 group-hover:opacity-100 focus:opacity-100 [@media(hover:none)_and_(pointer:coarse)]:opacity-40 [@media(hover:none)_and_(pointer:coarse)]:p-1.5 transition-opacity print:hidden disabled:opacity-50 disabled:cursor-not-allowed"
aria-label={label}
title={label}
data-testid={`em-delete-row-${meetingId}`}
className="w-full text-start px-2 py-1.5 text-xs rounded hover:bg-accent text-muted-foreground flex items-center gap-1 mb-0.5"
onClick={onClick}
data-testid="em-row-actions-back"
>
<Trash2 className="w-3.5 h-3.5 [@media(hover:none)_and_(pointer:coarse)]:w-4 [@media(hover:none)_and_(pointer:coarse)]:h-4" />
<ArrowIcon className="w-3.5 h-3.5" aria-hidden="true" />
{t("executiveMeetings.rowActions.back")}
</button>
);
}
@@ -2083,36 +2226,28 @@ function MeetingRow({
tintBg ? { ...cellStyle(col), backgroundColor: tintBg } : cellStyle(col);
const renderCell = (col: ColumnSetting, idx: number): ReactNode => {
// Anchor the merge-menu trigger to whichever cell is the first
// visible one on an unmerged row. Using "first visible cell"
// (instead of always the # cell) keeps the trigger reachable when
// editors hide the number column. The trigger is rendered as an
// Anchor the consolidated row-actions menu (delete + row color +
// merge) to whichever cell is the first visible one on an unmerged
// row. Using "first visible cell" (instead of always the # cell)
// keeps the trigger reachable when editors hide the number column
// via the column customizer. The button is rendered as an
// absolutely-positioned overlay inside the cell, so every cell type
// needs `relative group` on its <td> for the overlay to find it.
const showMergeTrigger = canMutate && !isMerged && idx === firstUnmergedIdx;
const mergeOverlay = showMergeTrigger ? (
<MergeMenu
// needs `relative group` on its <td> for it to find them.
const showActionsMenu = canMutate && !isMerged && idx === firstUnmergedIdx;
const actionsOverlay = showActionsMenu ? (
<RowActionsMenu
meetingId={meeting.id}
isDeleting={isDeleting}
onDelete={onDeleteMeeting}
rowColorKey={rowColorKey}
onPickRowColor={onPickRowColor}
// Offer Unmerge whenever the row has a stored merge in the DB,
// even if we're in degraded (non-contiguous) rendering — that's
// the only way the user can clear the stored state.
hasStoredMerge={hasStoredMerge}
existingMergeText={meeting.mergeText ?? null}
onChangeMerge={onSaveMerge}
t={t}
onChange={onSaveMerge}
/>
) : null;
// Delete trigger uses the same "first visible cell" anchoring as the
// merge trigger so the action stays available even when editors hide
// the # column via the column customizer. Only one button per row.
// The merged-cell branch (in renderCells below) renders its own copy.
const showDeleteTrigger = canMutate && !isMerged && idx === firstUnmergedIdx;
const deleteOverlay = showDeleteTrigger ? (
<DeleteRowButton
meetingId={meeting.id}
isDeleting={isDeleting}
onDelete={onDeleteMeeting}
label={t("executiveMeetings.schedule.deleteRow")}
/>
) : null;
switch (col.id) {
@@ -2157,11 +2292,7 @@ function MeetingRow({
)}
<span>{meeting.dailyNumber}</span>
</div>
{canMutate && (
<RowColorPicker current={rowColorKey} onPick={onPickRowColor} t={t} />
)}
{deleteOverlay}
{mergeOverlay}
{actionsOverlay}
</td>
);
case "meeting":
@@ -2187,8 +2318,7 @@ function MeetingRow({
{meeting.location}
</div>
)}
{deleteOverlay}
{mergeOverlay}
{actionsOverlay}
</td>
);
case "attendees":
@@ -2208,8 +2338,7 @@ function MeetingRow({
onCommitAddAttendee={onCommitAddAttendee}
onCancelAddAttendee={onCancelAddAttendee}
/>
{deleteOverlay}
{mergeOverlay}
{actionsOverlay}
</td>
);
case "time":
@@ -2226,8 +2355,7 @@ function MeetingRow({
canMutate={canMutate}
onSaveTimes={onSaveTimes}
/>
{deleteOverlay}
{mergeOverlay}
{actionsOverlay}
</td>
);
}
@@ -2235,7 +2363,7 @@ function MeetingRow({
// When a merge is active we render a single <td colSpan=N> with an
// EditableCell for `mergeText`, and skip the cells the merge covers
// after the first. The merged cell hosts its own MergeMenu so users
// after the first. The merged cell hosts its own RowActionsMenu so users
// can always unmerge or re-merge — even when the # column is hidden.
// Originals stay in the DB so unmerge restores them.
const renderCells = (): ReactNode[] => {
@@ -2282,21 +2410,16 @@ function MeetingRow({
testId={`em-merge-edit-${meeting.id}`}
/>
{canMutate && (
<DeleteRowButton
<RowActionsMenu
meetingId={meeting.id}
isDeleting={isDeleting}
onDelete={onDeleteMeeting}
label={t("executiveMeetings.schedule.deleteRow")}
/>
)}
{canMutate && (
<MergeMenu
meetingId={meeting.id}
isMerged
rowColorKey={rowColorKey}
onPickRowColor={onPickRowColor}
hasStoredMerge
existingMergeText={meeting.mergeText ?? null}
onChangeMerge={onSaveMerge}
t={t}
onChange={onSaveMerge}
/>
)}
</td>,
@@ -2375,98 +2498,6 @@ function hexToRgba(hex: string, alpha: number): string {
return `rgba(${r}, ${g}, ${b}, ${alpha})`;
}
// Small popover-driven control attached to a row (in the # cell when
// not merged, or to the merged cell itself) that lets editors pick a
// merge range or clear an existing one. Each option corresponds to a
// fixed [start..end] column range.
function MergeMenu({
meetingId,
// True iff the row is rendered as a merged cell *right now*. Used
// for testid wiring only.
isMerged = false,
// True iff the row has a stored merge in the DB (regardless of
// whether it's currently renderable as merged). Drives the
// "Unmerge" option so users can always clear stored merges, even
// when the visible columns aren't contiguous.
hasStoredMerge,
// Existing merge text from the DB, preserved when the user picks a
// new merge range so we never silently overwrite their content.
existingMergeText,
t,
onChange,
}: {
meetingId: number;
isMerged?: boolean;
hasStoredMerge: boolean;
existingMergeText: string | null;
t: (k: string) => string;
onChange: (
merge:
| { mergeStartColumn: ColumnId; mergeEndColumn: ColumnId; mergeText: string }
| null,
) => Promise<void>;
}) {
// Re-merging keeps the existing text so users don't accidentally lose
// what they had typed. New merges (no stored text) start empty.
const apply = async (start: ColumnId, end: ColumnId) => {
await onChange({
mergeStartColumn: start,
mergeEndColumn: end,
mergeText: existingMergeText ?? "",
});
};
return (
<Popover>
<PopoverTrigger asChild>
<button
type="button"
className="absolute bottom-1 inline-end-1 p-1 rounded text-muted-foreground hover:text-[#0B1E3F] hover:bg-white/70 opacity-0 group-hover:opacity-100 focus:opacity-100 [@media(hover:none)_and_(pointer:coarse)]:opacity-40 [@media(hover:none)_and_(pointer:coarse)]:p-1.5 transition-opacity print:hidden"
aria-label={t("executiveMeetings.merge.label")}
data-testid={`em-merge-trigger-${meetingId}`}
>
<Combine className="w-3.5 h-3.5" />
</button>
</PopoverTrigger>
<PopoverContent className="w-56 p-1" align="end">
<button
type="button"
className="w-full text-start px-2 py-1.5 text-sm rounded hover:bg-accent"
data-testid={`em-merge-opt-meeting-attendees-${meetingId}`}
onClick={() => apply("meeting", "attendees")}
>
{t("executiveMeetings.merge.meetingAttendees")}
</button>
<button
type="button"
className="w-full text-start px-2 py-1.5 text-sm rounded hover:bg-accent"
data-testid={`em-merge-opt-meeting-time-${meetingId}`}
onClick={() => apply("meeting", "time")}
>
{t("executiveMeetings.merge.meetingTime")}
</button>
<button
type="button"
className="w-full text-start px-2 py-1.5 text-sm rounded hover:bg-accent"
data-testid={`em-merge-opt-all-${meetingId}`}
onClick={() => apply("number", "time")}
>
{t("executiveMeetings.merge.all")}
</button>
{hasStoredMerge && (
<button
type="button"
className="w-full text-start px-2 py-1.5 text-sm rounded hover:bg-accent text-destructive"
data-testid={`em-merge-opt-unmerge-${meetingId}`}
onClick={() => onChange(null)}
>
{t("executiveMeetings.merge.unmerge")}
</button>
)}
</PopoverContent>
</Popover>
);
}
function TimeRangeCell({
meeting,
t,
@@ -0,0 +1,106 @@
// Verifies the consolidated row-actions kebab menu introduced as part
// of task #191 — the three previously-separate row overlays (delete,
// row color, merge cells) are now collapsed into a single menu so they
// no longer collide visually inside narrow cells. Each sub-action
// keeps its original testid, so the rest of the suite + any external
// tooling that targeted them keeps working without changes.
import { test, expect } from "@playwright/test";
async function loginViaUi(page, username, password) {
await page.goto("/login");
await page.locator("#username").fill(username);
await page.locator("#password").fill(password);
await Promise.all([
page.waitForURL((url) => !url.pathname.endsWith("/login"), {
timeout: 15_000,
}),
page.locator('form button[type="submit"]').click(),
]);
}
async function ensureEditModeOn(page) {
const toggle = page.getByTestId("em-edit-mode-toggle");
await expect(toggle).toBeVisible();
if ((await toggle.getAttribute("aria-pressed")) === "false") {
await toggle.click();
}
await expect(toggle).toHaveAttribute("aria-pressed", "true");
}
test("Schedule: row actions menu exposes delete + color + merge in a single popover and supports back navigation", async ({
page,
}) => {
await page.addInitScript(() => {
try {
window.localStorage.setItem("tx-lang", "en");
} catch {
/* localStorage may not be ready before navigation */
}
});
await loginViaUi(page, "admin", "admin123");
await page.goto("/executive-meetings");
await ensureEditModeOn(page);
// Exactly one consolidated kebab trigger per row, replacing what used
// to be three separate overlay icons (Trash, Palette, Combine).
const kebab = page.locator('[data-testid^="em-row-actions-"]').first();
await expect(kebab).toHaveCount(1);
// Closed state: none of the legacy testids are mounted yet.
await expect(page.locator('[data-testid^="em-delete-row-"]')).toHaveCount(0);
await expect(page.getByTestId("em-row-color-trigger")).toHaveCount(0);
await expect(page.locator('[data-testid^="em-merge-trigger-"]')).toHaveCount(
0,
);
// The kebab is opacity-0 until row hover, so force the click.
await kebab.click({ force: true });
// Main menu shows all three legacy testids — preserves selectors used
// elsewhere in the suite.
await expect(page.locator('[data-testid^="em-delete-row-"]')).toHaveCount(1);
await expect(page.getByTestId("em-row-color-trigger")).toHaveCount(1);
await expect(page.locator('[data-testid^="em-merge-trigger-"]')).toHaveCount(
1,
);
// Drill into the Row color sub-view: Delete + Merge get hidden,
// a Back affordance + the swatch row appear.
await page.getByTestId("em-row-color-trigger").click();
await expect(page.locator('[data-testid^="em-delete-row-"]')).toHaveCount(0);
await expect(page.locator('[data-testid^="em-merge-trigger-"]')).toHaveCount(
0,
);
await expect(page.getByTestId("em-row-actions-back")).toBeVisible();
// Back returns to the main 3-item view.
await page.getByTestId("em-row-actions-back").click();
await expect(page.locator('[data-testid^="em-delete-row-"]')).toHaveCount(1);
await expect(page.getByTestId("em-row-color-trigger")).toHaveCount(1);
await expect(page.locator('[data-testid^="em-merge-trigger-"]')).toHaveCount(
1,
);
// Drill into the Merge sub-view — three merge options appear and the
// other items disappear.
await page.locator('[data-testid^="em-merge-trigger-"]').click();
await expect(
page.locator('[data-testid^="em-merge-opt-meeting-attendees-"]'),
).toHaveCount(1);
await expect(
page.locator('[data-testid^="em-merge-opt-meeting-time-"]'),
).toHaveCount(1);
await expect(page.locator('[data-testid^="em-merge-opt-all-"]')).toHaveCount(
1,
);
await expect(page.locator('[data-testid^="em-delete-row-"]')).toHaveCount(0);
await expect(page.getByTestId("em-row-color-trigger")).toHaveCount(0);
// Escape closes the popover entirely.
await page.keyboard.press("Escape");
await expect(page.locator('[data-testid^="em-delete-row-"]')).toHaveCount(0);
await expect(page.getByTestId("em-row-color-trigger")).toHaveCount(0);
await expect(page.locator('[data-testid^="em-merge-trigger-"]')).toHaveCount(
0,
);
});