Task #499: fix Quick Actions dialog X overlap + iPad long-press drag feedback
Two scoped UX fixes on Executive Meetings schedule, both confined to
artifacts/tx-os/src/pages/executive-meetings.tsx (shared
src/components/ui/dialog.tsx untouched per task spec).
1) Quick Actions dialog header overlap
- Radix DialogContent renders the Close (X) at physical
`right-4 top-4` regardless of `dir`. In AR (RTL) the
`text-right`-aligned title "إجراءات سريعة" pinned to the same
edge and visibly overlapped the X.
- Fix: added `pr-8` to the per-row Quick Actions DialogHeader so
the title's right edge always clears the close button. Works
for AR (overlap eliminated) and EN (just adds trailing slack).
2) iPad long-press drag feedback
- dnd-kit's TouchSensor has a 200ms activation delay; previously
the row gave no cue during the hold window so users lifted
early thinking the schedule didn't register.
- Fix: added `isPressing` state toggled via capture-phase
onPointerDown/Up/Cancel handlers on the row tr, gated on
`pointerType === "touch"` && `effectiveDragEnabled`. Capture
phase chosen so handlers don't collide with safeRowDragListeners
(which spreads dnd-kit's bubble-phase onPointerDown).
- Visual cue: subtle `inset 0 0 0 2px hexToRgba(highlightColor,
0.45)` ring composed AFTER quickOpen/selection/current rings so
it never outranks a real selection state. Per spec, the cue
appears during the 200ms hold AND continues throughout the
active drag (we deliberately do NOT clear isPressing on the
rising edge of isDragging). Two defensive useEffects clear
`isPressing`: one when drag becomes disabled (capability flip /
edit-mode toggle), one tied to the FALLING edge of isDragging
(drag just ended) as a belt-and-suspenders guard against stale
visual state when dnd-kit absorbs the touch sequence and our
row's own pointerup never fires.
- data-pressing="true" attribute exposed on tr for tests.
- TouchSensor delay (200ms) and tolerance (8px) unchanged.
Tests added:
- executive-meetings-quick-actions-dialog-layout.spec.mjs: asserts
close button + title bounding boxes don't overlap in AR + EN.
- executive-meetings-row-touch-press-feedback.spec.mjs: uses
hasTouch+isMobile + CDP Input.dispatchTouchEvent, asserts
data-pressing flips true on touchstart, clears on touchend, and
tap-to-open quick-actions dialog still works for short taps;
second test verifies the cue persists past the 200ms window.
Verification: pnpm tsc --noEmit clean. Pre-existing `test` workflow
failures (rotate-content/font-settings/notes-share) are unrelated to
this change.
This commit is contained in:
@@ -4300,20 +4300,23 @@ function MeetingRow({
|
||||
useEffect(() => {
|
||||
if (!effectiveDragEnabled && isPressing) setIsPressing(false);
|
||||
}, [effectiveDragEnabled, isPressing]);
|
||||
// Defensive cleanup tied to the drag lifecycle: dnd-kit owns the
|
||||
// touch sequence once a drag activates, and on certain release
|
||||
// paths (drop outside the row, sensor-managed cancel) the row's
|
||||
// own `pointerup` / `pointercancel` listeners are not guaranteed
|
||||
// to fire. We watch `isDragging` for its rising edge (drag just
|
||||
// activated) AND falling edge (drag just ended) and force-clear
|
||||
// `isPressing` either way — once the drag itself is the active
|
||||
// visual signal (opacity 0.5 + dnd-kit overlay), the press ring
|
||||
// would be redundant; once the drag has ended, any leftover ring
|
||||
// is stale by definition.
|
||||
// Per task spec, the cue must appear during the 200ms hold AND
|
||||
// continue while the drag is active — so we DO NOT clear
|
||||
// `isPressing` on the rising edge of `isDragging`. We only clear
|
||||
// on the falling edge (drag just ended) as defensive cleanup for
|
||||
// release paths where dnd-kit absorbs the touch sequence and our
|
||||
// row's own `pointerup` / `pointercancel` capture listeners never
|
||||
// fire (e.g. drop outside the row, sensor-managed cancel). In
|
||||
// the normal release path our pointerup handler clears `isPressing`
|
||||
// first; this effect is a belt-and-suspenders guard against stale
|
||||
// visual state.
|
||||
const prevDraggingRef = useRef(false);
|
||||
useEffect(() => {
|
||||
if (!isDragging && !isPressing) return;
|
||||
if (isPressing) setIsPressing(false);
|
||||
}, [isDragging]); // eslint-disable-line react-hooks/exhaustive-deps
|
||||
if (prevDraggingRef.current && !isDragging && isPressing) {
|
||||
setIsPressing(false);
|
||||
}
|
||||
prevDraggingRef.current = isDragging;
|
||||
}, [isDragging, isPressing]);
|
||||
const pressShadow = isPressing
|
||||
? `inset 0 0 0 2px ${hexToRgba(highlightColor, 0.45)}`
|
||||
: null;
|
||||
|
||||
@@ -221,6 +221,10 @@ test("Touch (iPad): drag activation + drop reliably clears data-pressing (no sta
|
||||
});
|
||||
// Hold past 200ms TouchSensor activation delay.
|
||||
await page.waitForTimeout(350);
|
||||
// Per task spec, the press cue must remain visible AFTER drag
|
||||
// activates (not just during the pre-drag hold). Sanity-check the
|
||||
// attribute is still set right before we start dragging.
|
||||
await expect(rowA).toHaveAttribute("data-pressing", "true");
|
||||
// Drag down toward rowB.
|
||||
const steps = 12;
|
||||
for (let i = 1; i <= steps; i++) {
|
||||
@@ -231,6 +235,11 @@ test("Touch (iPad): drag activation + drop reliably clears data-pressing (no sta
|
||||
});
|
||||
await page.waitForTimeout(20);
|
||||
}
|
||||
// Mid-drag: cue must STILL be visible — feedback continues
|
||||
// throughout the active drag, not just during the activation
|
||||
// delay. This is the explicit "continues while drag is active"
|
||||
// requirement from #499.
|
||||
await expect(rowA).toHaveAttribute("data-pressing", "true");
|
||||
// Release — drag ends. Whether the row's own pointerup fires or
|
||||
// dnd-kit cancels it, the `isDragging` cleanup effect must clear
|
||||
// `data-pressing`.
|
||||
|
||||
Reference in New Issue
Block a user