diff --git a/client/src/components/Planner/DayPlanSidebar.test.tsx b/client/src/components/Planner/DayPlanSidebar.test.tsx index db6aab5c9..99bbc6669 100644 --- a/client/src/components/Planner/DayPlanSidebar.test.tsx +++ b/client/src/components/Planner/DayPlanSidebar.test.tsx @@ -8,6 +8,7 @@ import { useAuthStore } from '../../store/authStore' import { useTripStore, type TripStoreState } from '../../store/tripStore' import { useSettingsStore } from '../../store/settingsStore' import { usePluginStore } from '../../store/pluginStore' +import { installTouchDragBridge } from '../../utils/touchDragBridge' import { resetAllStores, seedStore } from '../../../tests/helpers/store' import { buildUser, buildTrip, buildDay, buildPlace, buildCategory, buildAssignment, buildDayNote, buildReservation, @@ -2953,18 +2954,31 @@ describe('DayPlanSidebar', () => { expect(document.querySelectorAll('.dp-row')).toHaveLength(0) }) - it('FE-PLANNER-DAYPLAN-155: on a touch device place rows lose their drag handle', () => { - const place = buildPlace({ id: 1, name: 'Touch place' }) + it('FE-PLANNER-DAYPLAN-155: below lg place rows lose their drag handle', () => { + const place = buildPlace({ id: 1, name: 'Narrow place' }) const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) const a = buildAssignment({ id: 11, day_id: 10, order_index: 0, place }) - render() - const row = screen.getByText('Touch place').closest('.dp-row') as HTMLElement + render() + const row = screen.getByText('Narrow place').closest('.dp-row') as HTMLElement expect(row.getAttribute('draggable')).toBe('false') expect(row.querySelector('.dp-grip')).toBeNull() fireEvent.dragStart(row, { dataTransfer: emptyDataTransfer }) expect(row.style.opacity).toBe('1') }) + // #1616: a tablet is a coarse pointer at a desktop width. It gets the same rows + // the mouse does — the long press in touchDragBridge is what starts the drag. + it('FE-PLANNER-DAYPLAN-155b: at desktop width place rows keep grip and drag', () => { + const place = buildPlace({ id: 1, name: 'Tablet place' }) + const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) + const a = buildAssignment({ id: 11, day_id: 10, order_index: 0, place }) + render() + const row = screen.getByText('Tablet place').closest('.dp-row') as HTMLElement + expect(row.getAttribute('draggable')).toBe('true') + expect(row.querySelector('.dp-grip')).not.toBeNull() + expect(row.closest('[data-touch-drag]')).not.toBeNull() + }) + // ── Booking rows ───────────────────────────────────────────────────────── const withEndpoints = (r: Reservation): Reservation => ({ @@ -3655,10 +3669,10 @@ describe('DayPlanSidebar', () => { await waitFor(() => expect(updateDayNote).toHaveBeenCalledWith(1, 10, 70, { sort_order: expect.any(Number) })) }) - it('FE-PLANNER-DAYPLAN-188: on a touch device note rows lose their grip and cannot be dragged', () => { + it('FE-PLANNER-DAYPLAN-188: below lg note rows lose their grip and cannot be dragged', () => { const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) mockDayNotesState.dayNotes = { '10': [buildDayNote({ id: 70, day_id: 10, text: 'A note' })] } - render() + render() const row = cardRow(screen.getByText('A note')) expect(row.getAttribute('draggable')).toBe('false') expect(row.querySelector('.dp-grip')).toBeNull() @@ -3758,3 +3772,68 @@ describe('DayPlanSidebar remaining branches', () => { expect(body.indexOf('Departing Inn')).toBeLessThan(body.indexOf('Arriving Inn')) }) }) + +// #1616 — the reporter's iPad sees both panes but could not pick a row up at all. +// This drives the real bridge rather than a synthetic dragstart, so it fails if the +// long press, the hit test or the opt-in container ever stop lining up. +describe('reordering the day plan with a finger (#1616)', () => { + /** Presses a row for longer than the bridge's long press. */ + async function longPress(row: Element) { + fireEvent.touchStart(row, { touches: [{ identifier: 1, clientX: 20, clientY: 40 }] }) + await new Promise(resolve => setTimeout(resolve, 400)) + } + + function dragTo(target: Element) { + document.elementFromPoint = () => target as Element + fireEvent.touchMove(document, { touches: [{ identifier: 1, clientX: 20, clientY: 180 }] }) + const end = new Event('touchend', { bubbles: true, cancelable: true }) + Object.defineProperty(end, 'touches', { value: [] }) + fireEvent(document, end) + } + + it('FE-PLANNER-DAYPLAN-189: a long press drags one place row onto another and reorders the day', async () => { + const onReorder = vi.fn().mockResolvedValue(undefined) + const place1 = buildPlace({ id: 1, name: 'First Place' }) + const place2 = buildPlace({ id: 2, name: 'Second Place' }) + const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) + const a1 = buildAssignment({ id: 11, day_id: 10, order_index: 0, place: place1 }) + const a2 = buildAssignment({ id: 12, day_id: 10, order_index: 1, place: place2 }) + render() + const teardown = installTouchDragBridge() + try { + const from = screen.getByText('First Place').closest('[draggable="true"]')! + const to = screen.getByText('Second Place').closest('[draggable="true"]')! + await longPress(from) + dragTo(to) + await waitFor(() => expect(onReorder).toHaveBeenCalledWith(10, expect.any(Array))) + } finally { + teardown() + } + }) + + it('FE-PLANNER-DAYPLAN-190: a swipe across the same row scrolls instead of reordering', async () => { + const onReorder = vi.fn().mockResolvedValue(undefined) + const place1 = buildPlace({ id: 1, name: 'First Place' }) + const place2 = buildPlace({ id: 2, name: 'Second Place' }) + const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) + const a1 = buildAssignment({ id: 11, day_id: 10, order_index: 0, place: place1 }) + const a2 = buildAssignment({ id: 12, day_id: 10, order_index: 1, place: place2 }) + render() + const teardown = installTouchDragBridge() + try { + const from = screen.getByText('First Place').closest('[draggable="true"]')! + fireEvent.touchStart(from, { touches: [{ identifier: 1, clientX: 20, clientY: 40 }] }) + // The finger travels before the press lands, so the browser keeps the gesture. + const moved = fireEvent.touchMove(document, { touches: [{ identifier: 1, clientX: 20, clientY: 140 }] }) + expect(moved).toBe(true) + await new Promise(resolve => setTimeout(resolve, 400)) + expect(onReorder).not.toHaveBeenCalled() + } finally { + teardown() + } + }) +}) diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index 35aceafd7..a52d3afbb 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -104,11 +104,6 @@ interface DayPlanSidebarProps { /** Mobile: show the route tools footer (Route toggle / Optimize / travel profile) on expanded days, since selecting a day closes the sheet */ showRouteToolsWhenExpanded?: boolean isMobile?: boolean - /** Coarse primary pointer. Drag & drop reorder is disabled here (it hijacks the - * touch-scroll gesture, #1432); the grip handle is hidden and the arrow reorder - * buttons take over instead. Distinct from isMobile, which is width-based — a - * tablet is a wide touch device and needs both. */ - isTouch?: boolean } /** @@ -156,9 +151,11 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { onScrollTopChange, showRouteToolsWhenExpanded = false, isMobile = false, - isTouch = false, } = props - const dragDisabled = isMobile || isTouch + // Below lg the plan and the places sit in separate tabs, so there is nothing + // to drag between. A coarse pointer is no longer a reason of its own: tablets + // reach the same drag through a long press (#1616). + const dragDisabled = isMobile const toast = useToast() const { t, language, locale } = useTranslation() const ctxMenu = useContextMenu() @@ -1254,7 +1251,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP ]) } return ( -
+
{/* Toolbar */} { }); -// #1432: a tablet is a touch device at a desktop width. Before the fix, isTouch didn't -// exist and drag was gated on width alone, so on an iPad the rows stayed draggable and a -// scroll swipe started an HTML5 drag, which raised the drop-to-import overlay instead of -// scrolling. These cases pin the desktop-width + coarse-pointer combination. -describe('touch device at desktop width (#1432)', () => { - const touchProps = { ...defaultProps, isMobile: false, isTouch: true }; +// #1616: a tablet is a coarse pointer at a desktop width, and it sees both panes, so +// it has somewhere to drag a place to. A coarse pointer used to switch the drag off by +// itself, which left the reporter's iPad selecting text instead of picking up a row. +// Width is the only gate now: below lg the places live in their own tab. +describe('touch device at desktop width (#1616)', () => { + const tabletProps = { ...defaultProps, isMobile: false }; - it('FE-PLANNER-SIDEBAR-044: place rows are not draggable', () => { + it('FE-PLANNER-SIDEBAR-044: place rows are draggable and opt into the touch bridge', () => { const place = buildPlace({ id: 7, name: 'Tablet Place' }); - render(); + const { container } = render(); const placeRow = screen.getByText('Tablet Place').closest('div[draggable]')!; - expect(placeRow.getAttribute('draggable')).toBe('false'); - }); - - it('FE-PLANNER-SIDEBAR-045: dragging over the sidebar does not raise the drop-to-import overlay', () => { - const place = buildPlace({ id: 7, name: 'Tablet Place' }); - const { container } = render(); - fireEvent.dragEnter(container.firstChild as HTMLElement); - expect(screen.queryByText('Drop to import')).not.toBeInTheDocument(); - }); - - it('FE-PLANNER-SIDEBAR-046: a mouse-driven desktop keeps drag and the drop-to-import overlay', () => { - const place = buildPlace({ id: 7, name: 'Desktop Place' }); - const { container } = render(); - const placeRow = screen.getByText('Desktop Place').closest('div[draggable]')!; expect(placeRow.getAttribute('draggable')).toBe('true'); + expect((container.firstChild as HTMLElement).hasAttribute('data-touch-drag')).toBe(true); + }); + + it('FE-PLANNER-SIDEBAR-045: dragging over the sidebar raises the drop-to-import overlay', () => { + const place = buildPlace({ id: 7, name: 'Tablet Place' }); + const { container } = render(); fireEvent.dragEnter(container.firstChild as HTMLElement); expect(screen.getByText('Drop to import')).toBeInTheDocument(); }); + + it('FE-PLANNER-SIDEBAR-046: below lg the rows stay undraggable and the bridge stays out', () => { + const place = buildPlace({ id: 7, name: 'Narrow Place' }); + const { container } = render(); + const placeRow = screen.getByText('Narrow Place').closest('div[draggable]')!; + expect(placeRow.getAttribute('draggable')).toBe('false'); + expect((container.firstChild as HTMLElement).hasAttribute('data-touch-drag')).toBe(false); + fireEvent.dragEnter(container.firstChild as HTMLElement); + expect(screen.queryByText('Drop to import')).not.toBeInTheDocument(); + }); }); // The map draws no legend of its own, so the stroke in the row is the only @@ -670,3 +673,40 @@ describe('track colour legend (#776)', () => { expect(strokes).toHaveLength(0); }); }); + +// #1616 — the other half of the reporter's gesture: the pickup. A tablet cannot +// start an HTML5 drag with a finger, so the bridge's long press has to do it, and +// the row has to hand over the placeId the day plan reads back on drop. +describe('picking a place up with a finger (#1616)', () => { + it('FE-PLANNER-SIDEBAR-047: a long press on a place row starts a drag carrying its id', async () => { + const place = buildPlace({ id: 42, name: 'Tablet Place' }); + render(); + const teardown = installTouchDragBridge(); + try { + const row = screen.getByText('Tablet Place').closest('[draggable="true"]')!; + fireEvent.touchStart(row, { touches: [{ identifier: 1, clientX: 20, clientY: 40 }] }); + await new Promise(resolve => setTimeout(resolve, 400)); + expect(window.__dragData).toEqual({ placeId: '42' }); + } finally { + teardown(); + window.__dragData = null; + } + }); + + it('FE-PLANNER-SIDEBAR-048: a swipe down the list scrolls instead of picking the row up', async () => { + const place = buildPlace({ id: 42, name: 'Tablet Place' }); + render(); + const teardown = installTouchDragBridge(); + try { + const row = screen.getByText('Tablet Place').closest('[draggable="true"]')!; + fireEvent.touchStart(row, { touches: [{ identifier: 1, clientX: 20, clientY: 40 }] }); + const moved = fireEvent.touchMove(document, { touches: [{ identifier: 1, clientX: 20, clientY: 140 }] }); + await new Promise(resolve => setTimeout(resolve, 400)); + expect(moved).toBe(true); + expect(window.__dragData).toBeFalsy(); + } finally { + teardown(); + window.__dragData = null; + } + }); +}); diff --git a/client/src/components/Planner/PlacesSidebar.tsx b/client/src/components/Planner/PlacesSidebar.tsx index 5cf637627..7ef660976 100644 --- a/client/src/components/Planner/PlacesSidebar.tsx +++ b/client/src/components/Planner/PlacesSidebar.tsx @@ -17,13 +17,17 @@ const PlacesSidebar = React.memo(function PlacesSidebar(props: PlacesSidebarProp sidebarDragOver, handleSidebarDragEnter, handleSidebarDragOver, handleSidebarDragLeave, handleSidebarDrop, selectMode, filtered, t, dayPickerPlace, listImportOpen, fileImportOpen, setFileImportOpen, sidebarDropFile, setSidebarDropFile, tripId, pushUndo, - ctxMenu, isMobile, isTouch, pendingDeleteIds, setPendingDeleteIds, onBulkDeleteConfirm, + ctxMenu, isMobile, pendingDeleteIds, setPendingDeleteIds, onBulkDeleteConfirm, categories, selectedIds, exitSelectMode, onBulkChangeCategory, categoryPickerOpen, setCategoryPickerOpen, collectionsEnabled, saveToListOpen, setSaveToListOpen, } = S - const dragDisabled = isMobile || isTouch + // Below lg the places sit in their own tab with no plan beside them to drag + // into. A coarse pointer no longer disables the drag on its own — tablets + // reach it through a long press (#1616). + const dragDisabled = isMobile return (
) => string onPlaceClick: (id: number | null) => void onContextMenu: (e: React.MouseEvent, place: Place) => void @@ -29,11 +27,12 @@ interface MemoPlaceRowProps { export const MemoPlaceRow = React.memo(function MemoPlaceRow({ place, category: cat, isSelected, isPlanned, inDay, isChecked, - selectMode, selectedDayId, canEditPlaces, isMobile, isTouch, t, + selectMode, selectedDayId, canEditPlaces, isMobile, t, onPlaceClick, onContextMenu, onAssignToDay, toggleSelected, setDayPickerPlace, registerPlaceRow, }: MemoPlaceRowProps) { const hasGeometry = Boolean(place.route_geometry) - const dragDisabled = isMobile || isTouch + // Touch is reached through a long press instead of being locked out (#1616). + const dragDisabled = isMobile return (
void days: Day[] isMobile: boolean - /** Primary pointer is coarse — HTML5 drag would swallow the scroll gesture (#1432). */ - isTouch?: boolean pushUndo?: (label: string, undoFn: () => Promise | void) => void initialScrollTop?: number onScrollTopChange?: (top: number) => void diff --git a/client/src/hooks/useTouchDragBridge.test.ts b/client/src/hooks/useTouchDragBridge.test.ts new file mode 100644 index 000000000..d60f30e57 --- /dev/null +++ b/client/src/hooks/useTouchDragBridge.test.ts @@ -0,0 +1,57 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' +import { renderHook } from '@testing-library/react' +import { fireEvent } from '@testing-library/dom' +import { useTouchDragBridge } from './useTouchDragBridge' + +describe('useTouchDragBridge (#1616)', () => { + let source: HTMLElement + let started: number + + beforeEach(() => { + vi.useFakeTimers() + document.body.innerHTML = '
Place
' + source = document.getElementById('source')! + started = 0 + document.addEventListener('dragstart', () => { started++ }) + document.elementFromPoint = () => source + }) + + afterEach(() => { + vi.useRealTimers() + document.body.innerHTML = '' + }) + + /** Presses the row for longer than the bridge's long press. */ + function longPress() { + fireEvent.touchStart(source, { touches: [{ identifier: 1, clientX: 10, clientY: 10 }] }) + vi.advanceTimersByTime(400) + } + + it('FE-TOUCHDRAGHOOK-001: while enabled a long press starts a drag', () => { + renderHook(() => useTouchDragBridge(true)) + longPress() + expect(started).toBe(1) + }) + + it('FE-TOUCHDRAGHOOK-002: a mouse-driven desktop is left to the browser', () => { + renderHook(() => useTouchDragBridge(false)) + longPress() + expect(started).toBe(0) + }) + + it('FE-TOUCHDRAGHOOK-003: leaving the planner stops the bridge watching', () => { + const { unmount } = renderHook(() => useTouchDragBridge(true)) + unmount() + longPress() + expect(started).toBe(0) + }) + + it('FE-TOUCHDRAGHOOK-004: turning it off mid-session stops the bridge watching', () => { + const { rerender } = renderHook(({ on }) => useTouchDragBridge(on), { + initialProps: { on: true }, + }) + rerender({ on: false }) + longPress() + expect(started).toBe(0) + }) +}) diff --git a/client/src/hooks/useTouchDragBridge.ts b/client/src/hooks/useTouchDragBridge.ts new file mode 100644 index 000000000..84a603e66 --- /dev/null +++ b/client/src/hooks/useTouchDragBridge.ts @@ -0,0 +1,17 @@ +import { useEffect } from 'react' +import { installTouchDragBridge } from '../utils/touchDragBridge' + +/** + * Arms the planner's long-press touch drag while `enabled` (#1616). + * + * Deliberately not armed on hybrid laptops: their primary pointer is fine, so + * `drag-drop-touch` is loaded there instead (see utils/touchDragPolyfill) and + * two bridges would fight over the same gesture. The two conditions cannot + * both hold — a pointer is either coarse or fine. + */ +export function useTouchDragBridge(enabled: boolean): void { + useEffect(() => { + if (!enabled) return + return installTouchDragBridge() + }, [enabled]) +} diff --git a/client/src/index.css b/client/src/index.css index e420a06af..be779a914 100644 --- a/client/src/index.css +++ b/client/src/index.css @@ -1165,6 +1165,18 @@ img[alt="TREK"] { .dp-day-header[data-selected="true"] .dp-day-actions { opacity: 1; } } +/* Planner rows a finger can pick up (#1616). A long press is what arms the drag, + and that is the same gesture the browser answers with a text selection and a + callout — "the action is interpreted as a text selection" in the report. Only + on coarse pointers, and only inside the two panes that opt into the bridge. */ +@media (pointer: coarse) { + [data-touch-drag] [draggable='true'] { + -webkit-touch-callout: none; + -webkit-user-select: none; + user-select: none; + } +} + /* ── TREK range slider — thin filled track + soft round thumb ─────────────── */ .trek-range { -webkit-appearance: none; diff --git a/client/src/pages/TripPlannerPage.tsx b/client/src/pages/TripPlannerPage.tsx index 087c21191..ddc462cce 100644 --- a/client/src/pages/TripPlannerPage.tsx +++ b/client/src/pages/TripPlannerPage.tsx @@ -52,6 +52,7 @@ import { useTripPlanner } from './tripPlanner/useTripPlanner' import { usePoiExplore } from '../components/Map/usePoiExplore' import PoiCategoryPill from '../components/Map/PoiCategoryPill' import { useIsPhone } from '../mobile/useIsPhone' +import { useTouchDragBridge } from '../hooks/useTouchDragBridge' import MTripShell from '../mobile/screens/trip/MTripShell' function ListsContainer({ tripId, packingItems, todoItems }: { tripId: number; packingItems: PackingItem[]; todoItems: TodoItem[] }) { @@ -227,6 +228,11 @@ function TripPlannerPageDesktop(): React.ReactElement | null { mapTileUrl, fontStyle, splashDone, } = useTripPlanner() + // Tablets run this very layout but cannot start an HTML5 drag with a finger, + // so a long press stands in for one (#1616). Only where the pointer is + // coarse — a hybrid laptop loads drag-drop-touch instead. + useTouchDragBridge(isTouch && !isMobile) + const poi = usePoiExplore() const [glMap, setGlMap] = useState(null) const poiPillEnabled = useSettingsStore(s => s.settings.map_poi_pill_enabled) !== false @@ -379,7 +385,6 @@ function TripPlannerPageDesktop(): React.ReactElement | null { }}>
diff --git a/client/src/pages/tripPlanner/useTripPlanner.ts b/client/src/pages/tripPlanner/useTripPlanner.ts index f3d8a0834..0fbb06e6f 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.ts +++ b/client/src/pages/tripPlanner/useTripPlanner.ts @@ -298,7 +298,7 @@ export function useTripPlanner() { mq.addEventListener('change', handler) return () => mq.removeEventListener('change', handler) }, []) - // Layout is width-driven (isMobile); drag affordances are pointer-driven (isTouch). + // Layout is width-driven (isMobile); the drag bridge is pointer-driven (isTouch). // Conflating them is what left a tablet's places list undraggable-but-unscrollable (#1432). const isTouch = useIsTouch() diff --git a/client/src/utils/touchDragPolyfill.ts b/client/src/utils/touchDragPolyfill.ts index ead905a51..47b34dcb8 100644 --- a/client/src/utils/touchDragPolyfill.ts +++ b/client/src/utils/touchDragPolyfill.ts @@ -6,11 +6,11 @@ * dblclick fires the default double-click-zoom, so two quick one-finger pans zoomed * instead of panning (#1440). * - * Reorder DnD is disabled wherever the primary pointer is coarse (#1432), so the only - * device class left with a job for the polyfill is the hybrid laptop: a mouse as the - * primary pointer, with a touchscreen also available. Loading it anywhere else — notably - * on tablets, which a width check misclassifies as desktop — re-arms the very gesture - * hijack #1432 removed, and drags the #1440 phantom-dblclick along with it. + * The hybrid laptop is the only device class left with a job for it: a mouse as the + * primary pointer, with a touchscreen also available. Loading it anywhere else re-arms + * the very gesture hijack #1432 removed, and drags the #1440 phantom-dblclick along + * with it. Coarse-pointer devices get touchDragBridge instead, which waits out a long + * press before it claims a gesture and so leaves scrolling alone (#1616). */ export function maybeInstallTouchDragPolyfill(): Promise | void { if (typeof window === 'undefined') return