Task #489 fix: decouple row drag from edit mode
Previous review rejected because row drag was gated on
`effectiveCanMutate = canMutate && editMode`, forcing users to
enter Edit mode to drag. Spec requires drag to work in view mode
for any user with mutate permission.
Changes:
- MeetingRow: new `dragEnabled` prop separate from `canMutate`.
Used by useSortable.disabled, safeRowDragListeners gate,
attributes spread, and cursor-grab class.
- Parent passes `dragEnabled={canMutate}` (raw, not gated by
editMode); `canMutate={effectiveCanMutate}` retained for inline
cell editing.
- handleRowClick: scope interactive `closest()` to descendants
(interactive !== e.currentTarget). Required because the row now
carries dnd-kit's role="button" in view mode and an unscoped
closest() matched the row itself, swallowing every popover click.
- e2e: removed edit-toggle click from row-drag spec — drag now
passes in view mode.
Tests: row-drag spec (view mode), popover spec (4/4), schedule-
features drag tests (290, 834), backend rotate-content (6/6) all
pass. 3 unrelated schedule-features failures reference a missing
em-nav-settings testid that does not exist in source — pre-existing
broken tests, not caused by this task.
This commit is contained in:
@@ -2874,6 +2874,9 @@ function ScheduleSection({
|
||||
rowColorKey={rowColors[m.id] ?? "default"}
|
||||
onPickRowColor={(key) => setRowColor(m.id, key)}
|
||||
canMutate={effectiveCanMutate}
|
||||
// #489: drag must work for mutators regardless
|
||||
// of edit mode — pass raw mutate permission.
|
||||
dragEnabled={canMutate}
|
||||
onSaveTitle={(html) => saveTitle(m, html)}
|
||||
onSaveAttendeeName={(idx, html) =>
|
||||
saveAttendeeName(m, idx, html)
|
||||
@@ -3696,6 +3699,7 @@ function MeetingRow({
|
||||
bulkSelected,
|
||||
onBulkSelectChange,
|
||||
displayNumber,
|
||||
dragEnabled,
|
||||
quickActionsCanMutate,
|
||||
onQuickPostpone,
|
||||
}: {
|
||||
@@ -3707,6 +3711,13 @@ function MeetingRow({
|
||||
rowColorKey: string;
|
||||
onPickRowColor: (key: string) => void;
|
||||
canMutate: boolean;
|
||||
// #489: raw (un-gated by editMode) mutate permission used to enable
|
||||
// ROW DRAG. The task spec explicitly requires that any user with
|
||||
// mutate permission can drag a row to a new position regardless of
|
||||
// edit mode, so drag must NOT be gated on `effectiveCanMutate`
|
||||
// (which equals canMutate && editMode and powers inline-editing
|
||||
// affordances on cells).
|
||||
dragEnabled: boolean;
|
||||
// #486: raw (un-gated by editMode) mutate permission — clicking a
|
||||
// row anywhere outside an interactive control opens the quick-
|
||||
// actions popover. The popover itself respects the same MUTATE_ROLES
|
||||
@@ -3779,7 +3790,7 @@ function MeetingRow({
|
||||
transform,
|
||||
transition,
|
||||
isDragging,
|
||||
} = useSortable({ id: meeting.id, disabled: !canMutate });
|
||||
} = useSortable({ id: meeting.id, disabled: !dragEnabled });
|
||||
const isSkipDragTarget = useCallback(
|
||||
(target: EventTarget | null): boolean => {
|
||||
const el = target as HTMLElement | null;
|
||||
@@ -3809,7 +3820,7 @@ function MeetingRow({
|
||||
[meeting.id],
|
||||
);
|
||||
const safeRowDragListeners = useMemo(() => {
|
||||
if (!listeners || !canMutate) return undefined;
|
||||
if (!listeners || !dragEnabled) return undefined;
|
||||
const out: Record<string, (e: { target: EventTarget | null }) => void> = {};
|
||||
for (const k of Object.keys(listeners)) {
|
||||
const handler = (listeners as Record<string, (e: unknown) => void>)[k];
|
||||
@@ -3820,7 +3831,7 @@ function MeetingRow({
|
||||
};
|
||||
}
|
||||
return out;
|
||||
}, [listeners, canMutate, isSkipDragTarget]);
|
||||
}, [listeners, dragEnabled, isSkipDragTarget]);
|
||||
|
||||
const numberCellCls =
|
||||
"border border-gray-300 px-2 py-2 text-center align-middle font-semibold " +
|
||||
@@ -4210,11 +4221,14 @@ function MeetingRow({
|
||||
// pure tag-name filter would let those clicks fall through to
|
||||
// the row handler and pop the quick-actions menu underneath the
|
||||
// inline editor the user just opened.
|
||||
if (
|
||||
target.closest(
|
||||
"button, a, input, textarea, select, [contenteditable='true'], [contenteditable=''], [role='menuitem'], [role='button'], [role='checkbox'], [role='switch'], [role='combobox'], [role='dialog']",
|
||||
)
|
||||
) {
|
||||
// #489: scope to descendants — the row itself now carries
|
||||
// role="button" + tabIndex from dnd-kit's `attributes` (because
|
||||
// dragEnabled is no longer gated by edit mode), so an unscoped
|
||||
// closest() would match the row itself and swallow every click.
|
||||
const interactive = target.closest(
|
||||
"button, a, input, textarea, select, [contenteditable='true'], [contenteditable=''], [role='menuitem'], [role='button'], [role='checkbox'], [role='switch'], [role='combobox'], [role='dialog']",
|
||||
);
|
||||
if (interactive && interactive !== e.currentTarget) {
|
||||
return;
|
||||
}
|
||||
// Skip clicks on row affordances by data-testid prefix so we
|
||||
@@ -4249,13 +4263,13 @@ function MeetingRow({
|
||||
<PopoverAnchor asChild>
|
||||
<tr
|
||||
ref={setNodeRef}
|
||||
className={`group${canMutate ? " cursor-grab active:cursor-grabbing" : ""}`}
|
||||
className={`group${dragEnabled ? " cursor-grab active:cursor-grabbing" : ""}`}
|
||||
style={trStyle}
|
||||
data-testid={`em-row-${meeting.id}`}
|
||||
data-current-meeting={isCurrent ? "true" : undefined}
|
||||
data-bulk-selected={bulkSelected ? "true" : undefined}
|
||||
onClick={quickActionsCanMutate ? handleRowClick : undefined}
|
||||
{...(canMutate ? attributes : {})}
|
||||
{...(dragEnabled ? attributes : {})}
|
||||
{...(safeRowDragListeners ?? {})}
|
||||
>
|
||||
{renderCells()}
|
||||
|
||||
@@ -140,14 +140,11 @@ test("dragging a row from position 1 to position 3 rotates content but anchors t
|
||||
await expect(alphaRow).toBeVisible({ timeout: 10_000 });
|
||||
await expect(charlieRow).toBeVisible();
|
||||
|
||||
// The schedule's row drag is gated on `effectiveCanMutate =
|
||||
// canMutate && editMode`, so the row's useSortable is disabled
|
||||
// (and `safeRowDragListeners` returns undefined) until edit mode
|
||||
// is toggled on. Without this click, no pointer event reaches
|
||||
// dnd-kit's PointerSensor.
|
||||
const editToggle = page.getByTestId("em-edit-mode-toggle");
|
||||
await editToggle.click();
|
||||
await expect(editToggle).toHaveAttribute("aria-pressed", "true");
|
||||
// #489: row drag must work for any user with mutate permission
|
||||
// even when edit mode is OFF — we deliberately do NOT toggle edit
|
||||
// mode here. The row's useSortable is gated on `dragEnabled`
|
||||
// (raw canMutate), independent of `effectiveCanMutate` which gates
|
||||
// inline-cell editing.
|
||||
|
||||
// The whole <tr> is the drag handle now (the dedicated grip is
|
||||
// gone). dnd-kit's PointerSensor activates after 6px of movement.
|
||||
|
||||
Reference in New Issue
Block a user