diff --git a/client/e2e/ipad-scroll-1432.spec.ts b/client/e2e/ipad-scroll-1432.spec.ts index 55631b4ce..be5431c9c 100644 --- a/client/e2e/ipad-scroll-1432.spec.ts +++ b/client/e2e/ipad-scroll-1432.spec.ts @@ -41,6 +41,10 @@ test('#1432 iPad: places list is scrollable, not draggable', async ({ page }) => } await page.reload() await expect(page.locator('.leaflet-container')).toBeVisible({ timeout: 20_000 }) + + // An iPad is inside the 768-1023px band, where only one side panel is open at a + // time so the map keeps a usable width (#2247). The places list is one tap away. + await page.getByRole('button', { name: 'Places' }).click() await expect(page.getByText('Place 1').first()).toBeVisible({ timeout: 20_000 }) // The context must really be the one from the bug report: coarse pointer, desktop @@ -74,6 +78,8 @@ test('#1432 iPad: places list is scrollable, not draggable', async ({ page }) => }) expect(['1', 'absent']).toContain(arrowOpacity) - // 4. The iPad must still get the desktop two-pane layout — isMobile stayed width-based. + // 4. The iPad must still get the desktop shell — isMobile stayed width-based. One + // panel at a time (#2247), so opening Places closed the day plan. await expect(page.locator('.leaflet-container')).toBeVisible() + await expect(page.getByRole('button', { name: 'Plan' })).toBeVisible() }) diff --git a/client/src/components/Planner/DayPlanSidebar.test.tsx b/client/src/components/Planner/DayPlanSidebar.test.tsx index 2ec0d95e0..ab0470dc1 100644 --- a/client/src/components/Planner/DayPlanSidebar.test.tsx +++ b/client/src/components/Planner/DayPlanSidebar.test.tsx @@ -1388,39 +1388,90 @@ describe('DayPlanSidebar', () => { expect(onDeletePlace).toHaveBeenCalledWith(42) }) - // ── Note card edit/delete buttons ───────────────────────────────────── + // ── Note card editing (#2249) ───────────────────────── - it('FE-PLANNER-DAYPLAN-064: note card edit button calls openEditNote', async () => { + it('FE-PLANNER-DAYPLAN-064: clicking a note row opens its edit modal', async () => { const user = userEvent.setup() const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) const note = buildDayNote({ id: 55, day_id: 10, text: 'My note' }) mockDayNotesState.dayNotes = { '10': [note] } render() - // Find note edit button (Pencil in note-edit-buttons) - const noteEditBtns = document.querySelectorAll('.note-edit-buttons button') - if (noteEditBtns.length > 0) { - await user.click(noteEditBtns[0] as HTMLElement) - expect(mockDayNotesState.openEditNote).toHaveBeenCalled() - } + await user.click(cardRow(screen.getByText('My note'))) + expect(mockDayNotesState.openEditNote).toHaveBeenCalledWith(10, expect.objectContaining({ id: 55 })) }) - it('FE-PLANNER-DAYPLAN-065: deleting a note asks for confirmation before calling deleteNote', async () => { + // The bug: an absolutely-positioned pencil/trash pill sat on the note's own + // reorder chevrons, and a coarse-pointer `opacity: 1 !important` in index.css + // made it permanent on tablets. Nothing may float over that column again. + it('FE-PLANNER-DAYPLAN-064b: the note row carries no floating action pill over its reorder buttons', () => { + const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) + mockDayNotesState.dayNotes = { '10': [buildDayNote({ id: 55, day_id: 10, text: 'My note' })] } + render() + const row = cardRow(screen.getByText('My note')) + expect(document.querySelector('.note-edit-buttons')).toBeNull() + expect(row.querySelectorAll('[style*="position: absolute"]')).toHaveLength(0) + expect(row.querySelectorAll('.reorder-buttons button')).toHaveLength(2) + // Positional, not just by class: the reorder column owns the row's right edge. + expect(row.lastElementChild).toHaveClass('reorder-buttons') + }) + + it('FE-PLANNER-DAYPLAN-064e: the keyboard reaches the note and opens it with Enter', () => { + const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) + mockDayNotesState.dayNotes = { '10': [buildDayNote({ id: 55, day_id: 10, text: 'My note' })] } + render() + const row = cardRow(screen.getByText('My note')) + expect(row).toHaveAttribute('tabindex', '0') + // A press-scale on a draggable row fights the drag, so the row opts out (#2158). + expect(row).toHaveAttribute('data-no-press') + fireEvent.keyDown(row, { key: 'Enter' }) + expect(mockDayNotesState.openEditNote).toHaveBeenCalledWith(10, expect.objectContaining({ id: 55 })) + }) + + it('FE-PLANNER-DAYPLAN-064c: a link inside the note body opens the link, not the edit modal', async () => { const user = userEvent.setup() const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) - const note = buildDayNote({ id: 55, day_id: 10, text: 'My note' }) - mockDayNotesState.dayNotes = { '10': [note] } + mockDayNotesState.dayNotes = { '10': [buildDayNote({ id: 55, day_id: 10, text: 'My note', time: '[docs](https://example.com)' })] } render() - // Find note delete button (Trash2 in note-edit-buttons) - const noteEditBtns = document.querySelectorAll('.note-edit-buttons button') - if (noteEditBtns.length > 1) { - await user.click(noteEditBtns[1] as HTMLElement) - // Clicking delete opens a confirmation dialog rather than deleting immediately. - expect(mockDayNotesState.deleteNote).not.toHaveBeenCalled() - expect(screen.getByText('Delete note?')).toBeInTheDocument() - // Confirming triggers the actual delete. - await user.click(screen.getByRole('button', { name: /^delete$/i })) - expect(mockDayNotesState.deleteNote).toHaveBeenCalled() - } + await user.click(screen.getByRole('link', { name: 'docs' })) + expect(mockDayNotesState.openEditNote).not.toHaveBeenCalled() + }) + + it('FE-PLANNER-DAYPLAN-064d: the reorder chevrons still move the note instead of editing it', async () => { + const user = userEvent.setup() + const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) + mockDayNotesState.dayNotes = { '10': [ + buildDayNote({ id: 55, day_id: 10, text: 'First note', sort_order: 0 }), + buildDayNote({ id: 56, day_id: 10, text: 'Second note', sort_order: 1 }), + ] } + render() + const row = cardRow(screen.getByText('Second note')) + await user.click(row.querySelectorAll('.reorder-buttons button')[0] as HTMLElement) + expect(mockDayNotesState.moveNote).toHaveBeenCalled() + expect(mockDayNotesState.openEditNote).not.toHaveBeenCalled() + }) + + it('FE-PLANNER-DAYPLAN-065: the note modal deletes only through the confirmation dialog', async () => { + const user = userEvent.setup() + const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) + mockDayNotesState.dayNotes = { '10': [buildDayNote({ id: 55, day_id: 10, text: 'My note' })] } + mockDayNotesState.noteUi = { '10': { mode: 'edit', noteId: 55, text: 'My note', time: '', icon: 'FileText', color: null } } + render() + await user.click(screen.getByRole('button', { name: /^delete$/i })) + expect(mockDayNotesState.deleteNote).not.toHaveBeenCalled() + expect(screen.getByText('Delete note?')).toBeInTheDocument() + const confirm = within(screen.getByText('Delete note?').closest('[class*="z-[10000]"]') as HTMLElement) + await user.click(confirm.getByRole('button', { name: /^delete$/i })) + expect(mockDayNotesState.deleteNote).toHaveBeenCalledWith(10, 55) + // Otherwise the edit modal is left open on a note that no longer exists. + expect(mockDayNotesState.cancelNote).toHaveBeenCalledWith(10) + }) + + it('FE-PLANNER-DAYPLAN-065b: a note being created offers no delete', () => { + const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) + mockDayNotesState.dayNotes = { '10': [] } + mockDayNotesState.noteUi = { '10': { mode: 'add', text: '', time: '', icon: 'FileText', color: null } } + render() + expect(screen.queryByRole('button', { name: /^delete$/i })).toBeNull() }) // ── Drop on assignment: same-day reorder ───────────────────────────── @@ -2940,6 +2991,10 @@ describe('DayPlanSidebar', () => { expect(screen.queryByLabelText('Add Note')).not.toBeInTheDocument() expect(dragRow(screen.getByText('Read only place'))).toBeNull() expect(screen.getByText('A note')).toBeInTheDocument() + // A viewer must not reach the edit modal they could never save (#2249). + const noteRow = cardRow(screen.getByText('A note')) + expect(noteRow).not.toHaveAttribute('role', 'button') + expect(noteRow.querySelectorAll('.reorder-buttons')).toHaveLength(0) }) it('FE-PLANNER-DAYPLAN-132: the read-only chevron still collapses and expands the day', async () => { diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index fe7b4bb6e..650a54d92 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -696,19 +696,13 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { }) } - const openEditNote = (dayId: number, note: DayNote, e?: React.MouseEvent) => { - e?.stopPropagation() - _openEditNote(dayId, note) - } + const openEditNote = (dayId: number, note: DayNote) => _openEditNote(dayId, note) // Deleting a note asks for confirmation first — the edit/delete icons sit close together and are // easy to mis-tap on touch devices, where an accidental delete was previously unrecoverable. const [pendingDeleteNote, setPendingDeleteNote] = useState<{ dayId: number; noteId: number } | null>(null) - const deleteNote = async (dayId: number, noteId: number, e?: React.MouseEvent) => { - e?.stopPropagation() - await _deleteNote(dayId, noteId) - } + const deleteNote = (dayId: number, noteId: number) => _deleteNote(dayId, noteId) // Unified reorder: assigns positions to ALL item types based on new visual order const applyMergedOrder = async (dayId: number, newOrder: { type: string; data: any }[]) => { @@ -2614,17 +2608,33 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP { divider: true }, { label: t('common.delete'), icon: Trash2, danger: true, onClick: () => setPendingDeleteNote({ dayId: day.id, noteId: note.id }) }, ]) : undefined} + {...(canEditDays ? { + role: 'button' as const, + // No press-scale: index.css shrinks every [role=button] on + // :active, which on a draggable row fights the drag (#2158). + 'data-no-press': '', + tabIndex: 0, + // The row is the note's edit affordance — the same gesture the + // phone shell already uses (#2249). Links inside the rendered + // body and the reorder buttons keep their own click. + onClick: (e: React.MouseEvent) => { + if ((e.target as HTMLElement).closest('a, button')) return + openEditNote(day.id, note) + }, + onKeyDown: (e: React.KeyboardEvent) => { + if (e.target !== e.currentTarget) return + if (e.key !== 'Enter' && e.key !== ' ') return + e.preventDefault() + openEditNote(day.id, note) + }, + } : {})} onMouseEnter={e => { const grip = e.currentTarget.querySelector('.dp-grip') as HTMLElement | null if (grip) grip.style.opacity = '1' - const editBtns = e.currentTarget.querySelector('.note-edit-buttons') as HTMLElement | null - if (editBtns) editBtns.style.opacity = '1' }} onMouseLeave={e => { const grip = e.currentTarget.querySelector('.dp-grip') as HTMLElement | null if (grip) grip.style.opacity = '0.3' - const editBtns = e.currentTarget.querySelector('.note-edit-buttons') as HTMLElement | null - if (editBtns) editBtns.style.opacity = '0' }} style={{ position: 'relative', @@ -2636,7 +2646,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP borderTop: showDropLine ? '2px solid var(--text-primary)' : undefined, background: noteSkin.background, opacity: draggingId === `note-${note.id}` ? 0.4 : 1, - transition: 'background 0.1s', cursor: 'grab', userSelect: 'none', + transition: 'background 0.1s', cursor: canEditDays ? 'pointer' : 'default', userSelect: 'none', }} > {canEditDays && !dragDisabled &&
@@ -2658,19 +2668,6 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP
)} - {/* Stacked in a capsule of their own, set off from the text: - a note card is as tall as its body, so the height is free - while the width next to the text is not, and a 10px icon - in a 14px box was barely a target. */} - {/* Floated over the text rather than sharing the row with - it: a note is mostly text, and reserving a column for - two buttons that are invisible until you hover costs - every note a shorter line for the whole time you are - not hovering. */} - {canEditDays &&
- - -
} {canEditDays &&
@@ -2877,6 +2874,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP noteInputRef={noteInputRef} cancelNote={cancelNote} saveNote={saveNote} + onRequestDelete={(dayId, noteId) => setPendingDeleteNote({ dayId, noteId })} t={t} /> @@ -2892,7 +2890,13 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP setPendingDeleteNote(null)} - onConfirm={() => { if (pendingDeleteNote) deleteNote(pendingDeleteNote.dayId, pendingDeleteNote.noteId) }} + onConfirm={() => { + if (!pendingDeleteNote) return + // Close the edit modal behind the confirm; leaving it open would go on + // editing a note that no longer exists. + cancelNote(pendingDeleteNote.dayId) + deleteNote(pendingDeleteNote.dayId, pendingDeleteNote.noteId) + }} title={t('dayplan.confirmDeleteNoteTitle')} message={t('dayplan.confirmDeleteNoteBody')} /> diff --git a/client/src/components/Planner/DayPlanSidebarNoteModal.tsx b/client/src/components/Planner/DayPlanSidebarNoteModal.tsx index 50a78e0f1..881c96298 100644 --- a/client/src/components/Planner/DayPlanSidebarNoteModal.tsx +++ b/client/src/components/Planner/DayPlanSidebarNoteModal.tsx @@ -11,6 +11,7 @@ import { markdownLinkComponents } from '../shared/markdownLink' interface NoteModalUi { mode: 'add' | 'edit' + noteId?: number icon: string text: string time: string @@ -25,6 +26,8 @@ interface DayPlanSidebarNoteModalProps { noteInputRef: React.RefObject cancelNote: (dayId: number) => void saveNote: (dayId: number) => void + /** Asks the planner to confirm and then delete the note being edited (#2249). */ + onRequestDelete: (dayId: number, noteId: number) => void t: (key: string, params?: Record) => string } @@ -38,19 +41,19 @@ const BODY_MAX = 2000 * right. The preview is the reason the colour picker is worth having — a swatch * row tells you nothing about what a tinted card looks like next to a place. */ -export function DayPlanSidebarNoteModal({ noteUi, setNoteUi, noteInputRef, cancelNote, saveNote, t }: DayPlanSidebarNoteModalProps) { +export function DayPlanSidebarNoteModal({ noteUi, setNoteUi, noteInputRef, cancelNote, saveNote, onRequestDelete, t }: DayPlanSidebarNoteModalProps) { return ( <> {Object.entries(noteUi).map(([dayId, ui]) => ui && createPortal( , + cancelNote={cancelNote} saveNote={saveNote} onRequestDelete={onRequestDelete} t={t} />, document.body ))} ) } -function NoteDialog({ dayId, ui, setNoteUi, noteInputRef, cancelNote, saveNote, t }: Omit & { +function NoteDialog({ dayId, ui, setNoteUi, noteInputRef, cancelNote, saveNote, onRequestDelete, t }: Omit & { dayId: string ui: NoteModalUi }) { @@ -174,7 +177,17 @@ function NoteDialog({ dayId, ui, setNoteUi, noteInputRef, cancelNote, saveNote,
-
+ {/* Delete lives here rather than on the row: the row's floating pencil/trash + pill covered the reorder chevrons and, on a coarse pointer, never went + away (#2249). The planner still confirms before it deletes. */} +
+ {ui.mode === 'edit' && ui.noteId != null && ( + + )} + {open && createPortal( +
+ {SUPPORTED_LANGUAGES.map(lang => ( + + ))} +
, + document.body, + )} +
+ ) +} diff --git a/client/src/hooks/useResizablePanels.ts b/client/src/hooks/useResizablePanels.ts index ba53a2b6c..ad0060b1b 100644 --- a/client/src/hooks/useResizablePanels.ts +++ b/client/src/hooks/useResizablePanels.ts @@ -1,7 +1,16 @@ -import { useState, useEffect, useRef } from 'react' +import { useState, useEffect, useRef, useCallback } from 'react' const MIN_SIDEBAR = 200 const MAX_SIDEBAR = 520 +/** + * The band where the desktop planner renders but two panels leave the map a + * useless strip. Below 768 the phone shell takes over, so the squeeze lives + * entirely between there and Tailwind's `lg`: a Pixel 10 Pro Fold unfolded is + * ~860 CSS px, where 340 + 300 of panel left the map 200 px (#2247). + */ +const NARROW_QUERY = '(min-width: 768px) and (max-width: 1023px)' +/** However wide a panel was dragged on a big screen, the map keeps this much. */ +const MIN_MAP = 360 export function useResizablePanels() { const [leftWidth, setLeftWidth] = useState(() => Number.parseInt(localStorage.getItem('sidebarLeftWidth') || '') || 340) @@ -11,6 +20,34 @@ export function useResizablePanels() { const isResizingLeft = useRef(false) const isResizingRight = useRef(false) + const [narrow, setNarrow] = useState( + () => typeof window !== 'undefined' && window.matchMedia(NARROW_QUERY).matches, + ) + // Which panel owns the screen while only one of them fits. The day plan is what + // the Plan tab is about, so it starts open and the places list is one tap away; + // null is map-only. Deliberately separate from the two collapse flags, which stay + // the wide-layout state — folding back to a wide window restores what was there. + const [narrowPanel, setNarrowPanel] = useState<'left' | 'right' | null>('left') + const [maxPanel, setMaxPanel] = useState(Number.POSITIVE_INFINITY) + + useEffect(() => { + const mq = window.matchMedia(NARROW_QUERY) + const handler = (e: MediaQueryListEvent) => setNarrow(e.matches) + setNarrow(mq.matches) + mq.addEventListener('change', handler) + return () => mq.removeEventListener('change', handler) + }, []) + + // Only measured inside the narrow band: that is the only place the clamp can + // bite, and a resize listener on every planner mount is not worth the rest. + useEffect(() => { + if (!narrow) { setMaxPanel(Number.POSITIVE_INFINITY); return } + const measure = () => setMaxPanel(Math.max(MIN_SIDEBAR, window.innerWidth - MIN_MAP - 20)) + measure() + window.addEventListener('resize', measure) + return () => window.removeEventListener('resize', measure) + }, [narrow]) + useEffect(() => { const onMove = (e: MouseEvent) => { if (isResizingLeft.current) { @@ -41,5 +78,27 @@ export function useResizablePanels() { const startResizeLeft = () => { isResizingLeft.current = true; document.body.style.cursor = 'col-resize'; document.body.style.userSelect = 'none' } const startResizeRight = () => { isResizingRight.current = true; document.body.style.cursor = 'col-resize'; document.body.style.userSelect = 'none' } - return { leftWidth, rightWidth, leftCollapsed, rightCollapsed, setLeftCollapsed, setRightCollapsed, startResizeLeft, startResizeRight } + // What the layout actually shows. The collapse flags stay the caller's intent — + // handlePlaceClick reopening "both" must not re-crowd a narrow screen. + const leftHidden = narrow ? narrowPanel !== 'left' : leftCollapsed + const rightHidden = narrow ? narrowPanel !== 'right' : rightCollapsed + + const toggleLeft = useCallback(() => { + if (narrow) setNarrowPanel(p => (p === 'left' ? null : 'left')) + else setLeftCollapsed(c => !c) + }, [narrow]) + const toggleRight = useCallback(() => { + if (narrow) setNarrowPanel(p => (p === 'right' ? null : 'right')) + else setRightCollapsed(c => !c) + }, [narrow]) + + return { + // Clamped for the layout; the stored width is left alone, so a 520 px panel + // dragged on a desktop comes back untouched once there is room for it again. + leftWidth: Math.min(leftWidth, maxPanel), + rightWidth: Math.min(rightWidth, maxPanel), + leftCollapsed, rightCollapsed, setLeftCollapsed, setRightCollapsed, + leftHidden, rightHidden, toggleLeft, toggleRight, narrow, + startResizeLeft, startResizeRight, + } } diff --git a/client/src/index.css b/client/src/index.css index 2f733bf2e..a15392236 100644 --- a/client/src/index.css +++ b/client/src/index.css @@ -232,8 +232,9 @@ html.dark .border-dashed.border-gray-300 { border-color: var(--border-primary) ! html.dark .bg-slate-50\/60, html.dark [class*="bg-slate-50/"] { background-color: transparent !important; } /* Reorder buttons: desktop = original style; touch = always visible, larger touch targets. - Keyed on pointer as well as width — a tablet is a wide touch device, and drag reorder is - off wherever the pointer is coarse (#1432), so these buttons are the only affordance left. */ + Keyed on pointer as well as width — a tablet is a wide touch device with no hover, so + without this the buttons would never appear there. (Drag reorder itself is no longer off + on a coarse pointer: #1616 gave tablets the long-press bridge.) */ .reorder-buttons { flex-direction: column; opacity: 0; @@ -270,9 +271,6 @@ html.dark .bg-slate-50\/60, html.dark [class*="bg-slate-50/"] { background-color align-items: center !important; justify-content: center !important; } - .note-edit-buttons { - opacity: 1 !important; - } } /* Packing list rows: on phones, keep the item name readable and move the dense @@ -1290,19 +1288,6 @@ img[alt="TREK"] { .collab-note-md-full th, .collab-note-md-full td { border: 1px solid var(--border-primary); padding: 4px 8px; font-size: 0.9em; } .collab-note-md-full hr { border: none; border-top: 1px solid var(--border-primary); margin: 0.8em 0; } -/* Note card actions: stacked beside the card, and big enough to aim at. */ -.note-edit-btn { - transition: background-color 0.12s ease, color 0.12s ease; -} -.note-edit-btn:hover { - background: var(--bg-hover); - color: var(--text-primary); -} -.note-edit-btn-danger:hover { - background: rgba(220, 38, 38, 0.13); - color: #dc2626; -} - /* Day-plan header action grid (edit / +transport / note / collapse) */ .dp-day-actions button { color: var(--text-faint); diff --git a/client/src/mobile/screens/trip/plan/MPlanTimelineRows.tsx b/client/src/mobile/screens/trip/plan/MPlanTimelineRows.tsx index 38659c6c4..f02f136af 100644 --- a/client/src/mobile/screens/trip/plan/MPlanTimelineRows.tsx +++ b/client/src/mobile/screens/trip/plan/MPlanTimelineRows.tsx @@ -547,7 +547,8 @@ export function NoteRow({ note, chrome, reorder, drag, onEdit }: { {...dragProps(drag)} role={chrome.editing ? 'button' : undefined} tabIndex={chrome.editing ? 0 : undefined} - onClick={chrome.editing ? onEdit : undefined} + // A link in the rendered body keeps its own tap; the rest of the row edits. + onClick={chrome.editing ? (e => { if (!(e.target as HTMLElement).closest('a')) onEdit() }) : undefined} onKeyDown={chrome.editing ? (e => { if (e.target === e.currentTarget && (e.key === 'Enter' || e.key === ' ')) { e.preventDefault(); onEdit() } }) : undefined} className={`my-[2px] flex items-center gap-2.5 ${chrome.editing ? 'cursor-pointer' : ''} ${dragClass(drag)}`} > diff --git a/client/src/pages/JourneyPublicPage.tsx b/client/src/pages/JourneyPublicPage.tsx index 42a119b8a..032ca45e3 100644 --- a/client/src/pages/JourneyPublicPage.tsx +++ b/client/src/pages/JourneyPublicPage.tsx @@ -28,8 +28,8 @@ import MobileEntryView from '../components/Journey/MobileEntryView'; import MobileMapTimeline from '../components/Journey/MobileMapTimeline'; import PhotoLightbox from '../components/Journey/PhotoLightbox'; import EmptyState from '../components/shared/EmptyState'; -import { SUPPORTED_LANGUAGES, useTranslation } from '../i18n'; -import { useSettingsStore } from '../store/settingsStore'; +import PublicLanguagePicker from '../components/shared/PublicLanguagePicker'; +import { useTranslation } from '../i18n'; import { formatLocationName } from '../utils/formatters'; import { useJourneyPublic } from './journeyPublic/useJourneyPublic'; @@ -616,70 +616,7 @@ export default function JourneyPublicPage() { }} /> - {/* Language picker */} -
- - {showLangPicker && ( -
- {SUPPORTED_LANGUAGES.map((lang) => ( - - ))} -
- )} -
+ {/* Logo */}
{ const { hook } = setup({ showLangPicker: true }); const german = screen.getByRole('button', { name: 'Deutsch' }); fireEvent.mouseEnter(german); - expect(german).toHaveStyle({ background: 'rgb(243, 244, 246)' }); + expect(german).toHaveStyle({ background: 'var(--bg-hover)' }); fireEvent.mouseLeave(german); fireEvent.click(german); diff --git a/client/src/pages/SharedTripPage.test.tsx b/client/src/pages/SharedTripPage.test.tsx index a1ed42467..6737651f0 100644 --- a/client/src/pages/SharedTripPage.test.tsx +++ b/client/src/pages/SharedTripPage.test.tsx @@ -1317,7 +1317,7 @@ describe('SharedTripPage', () => { const option = screen.getByRole('button', { name: /deutsch/i }); fireEvent.mouseEnter(option); - expect(option.style.background).toBe('rgb(243, 244, 246)'); + expect(option.style.background).toBe('var(--bg-hover)'); fireEvent.mouseLeave(option); expect(option.style.background).toBe('none'); }); diff --git a/client/src/pages/SharedTripPage.tsx b/client/src/pages/SharedTripPage.tsx index 912fb22ed..169e5d4db 100644 --- a/client/src/pages/SharedTripPage.tsx +++ b/client/src/pages/SharedTripPage.tsx @@ -19,10 +19,10 @@ import { createElement, useEffect, useRef } from 'react'; import { renderIconMarkup } from '../utils/iconMarkup'; import { MapContainer, Marker, Polyline, TileLayer, Tooltip, useMap } from 'react-leaflet'; import { getCategoryIcon } from '../components/shared/categoryIcons'; +import PublicLanguagePicker from '../components/shared/PublicLanguagePicker'; import { OFM_POSITRON, DEFAULT_MAP_CENTER, DEFAULT_MAP_ZOOM, MAP_MAX_ZOOM, attributionForTile } from '../constants/mapDefaults'; import VectorBasemap from '../components/Map/VectorBasemap'; -import { SUPPORTED_LANGUAGES, useTranslation } from '../i18n'; -import { useSettingsStore } from '../store/settingsStore'; +import { useTranslation } from '../i18n'; import { avatarSrc } from '../utils/avatarSrc'; import { safeHexColor } from '../utils/safeColor'; import { getMergedItems, getTransportForDay, hidesOnMiddleDay } from '../utils/dayMerge'; @@ -335,70 +335,7 @@ export default function SharedTripPage() { {t('shared.readOnly')}
- {/* Language picker - top right */} -
- - {showLangPicker && ( -
- {SUPPORTED_LANGUAGES.map((lang) => ( - - ))} -
- )} -
+
diff --git a/client/src/pages/TripPlannerPage.tsx b/client/src/pages/TripPlannerPage.tsx index 15c2629e2..29213dd1d 100644 --- a/client/src/pages/TripPlannerPage.tsx +++ b/client/src/pages/TripPlannerPage.tsx @@ -38,7 +38,6 @@ import { addonsApi, accommodationsApi, authApi, tripsApi, assignmentsApi, mapsAp import { accommodationRepo } from '../repo/accommodationRepo' import { useAuthStore } from '../store/authStore' import ConfirmDialog from '../components/shared/ConfirmDialog' -import { useResizablePanels } from '../hooks/useResizablePanels' import { useTripWebSocket } from '../hooks/useTripWebSocket' import { useRouteCalculation } from '../hooks/useRouteCalculation' import { usePlaceSelection } from '../hooks/usePlaceSelection' @@ -246,7 +245,9 @@ function TripPlannerPageDesktop(): React.ReactElement | null { enabledAddons, collabFeatures, tripAccommodations, setTripAccommodations, allowedFileTypes, tripMembers, setTripMembers, refreshMembers, loadAccommodations, TRANSPORT_TYPES, TRIP_TABS, activeTab, setActiveTab, handleTabChange, - leftWidth, rightWidth, leftCollapsed, rightCollapsed, setLeftCollapsed, setRightCollapsed, startResizeLeft, startResizeRight, + leftWidth, rightWidth, + leftHidden, rightHidden, toggleLeft, toggleRight, narrowPanels, + startResizeLeft, startResizeRight, selectedPlaceId, selectedAssignmentId, setSelectedPlaceId, selectAssignment, showDayDetail, setShowDayDetail, dayDetailCollapsed, setDayDetailCollapsed, showPlaceForm, setShowPlaceForm, editingPlace, setEditingPlace, @@ -327,6 +328,14 @@ function TripPlannerPageDesktop(): React.ReactElement | null { } if (!trip) return null + // What each panel actually occupies right now, and where that leaves the + // strip of map between them. The panels float 10px inside the map, so the + // corridor starts past that margin. + const leftPanelPx = leftHidden ? 0 : leftWidth + const rightPanelPx = rightHidden ? 0 : rightWidth + const mapInsetLeft = leftPanelPx ? leftPanelPx + 10 : 0 + const mapInsetRight = rightPanelPx ? rightPanelPx + 10 : 0 + return (
navigate('/dashboard')} onShare={() => setShowMembersModal(true)} /> @@ -381,8 +390,8 @@ function TripPlannerPageDesktop(): React.ReactElement | null { tileUrl={mapTileUrl} fitKey={fitKey} dayOrderMap={dayOrderMap} - leftWidth={leftCollapsed ? 0 : leftWidth} - rightWidth={rightCollapsed ? 0 : rightWidth} + leftWidth={leftPanelPx} + rightWidth={rightPanelPx} hasInspector={!!selectedPlace} hasDayDetail={!!showDayDetail && !selectedPlace} reservations={reservations} @@ -399,7 +408,14 @@ function TripPlannerPageDesktop(): React.ReactElement | null { /> {(poiPillEnabled || glMap) && ( -
+
{poiPillEnabled && ( )} @@ -425,30 +441,32 @@ function TripPlannerPageDesktop(): React.ReactElement | null { )}
-
{ if (selectedDayId) updateRouteForDay(selectedDayId) }} onAddBookingToAssignment={can('day_edit', trip) ? (dayId, assignmentId) => { tripActions.setSelectedDay(dayId); setBookingForAssignmentId(assignmentId); setEditingReservation(null); setShowReservationModal(true) } : undefined} /> - {!leftCollapsed && ( + {!leftHidden && !narrowPanels && (
-
- {!rightCollapsed && ( + {!rightHidden && !narrowPanels && (
{ setShowDayDetail(null); handleSelectDay(null) }} onAccommodationChange={loadAccommodations} - leftWidth={isMobile ? 0 : (leftCollapsed ? 0 : leftWidth)} - rightWidth={isMobile ? 0 : (rightCollapsed ? 0 : rightWidth)} + leftWidth={isMobile ? 0 : leftPanelPx} + rightWidth={isMobile ? 0 : rightPanelPx} collapsed={dayDetailCollapsed} onToggleCollapse={() => setDayDetailCollapsed(c => !c)} mobile={isMobile} @@ -659,8 +679,8 @@ function TripPlannerPageDesktop(): React.ReactElement | null { onUpdatePlace={async (placeId, data) => { try { await tripActions.updatePlace(tripId, placeId, data) } catch (err: unknown) { toast.error(err instanceof Error ? err.message : t('common.unknownError')) } }} onUploadImage={async (placeId, file) => { await tripActions.uploadPlaceImage(tripId, placeId, file) }} onRate={async (placeId, rating) => { try { await tripActions.ratePlace(tripId, placeId, rating) } catch (err: unknown) { toast.error(err instanceof Error ? err.message : t('common.unknownError')) } }} - leftWidth={(isMobile || window.innerWidth < 900) ? 0 : (leftCollapsed ? 0 : leftWidth)} - rightWidth={(isMobile || window.innerWidth < 900) ? 0 : (rightCollapsed ? 0 : rightWidth)} + leftWidth={isMobile ? 0 : leftPanelPx} + rightWidth={isMobile ? 0 : rightPanelPx} /> )} diff --git a/client/src/pages/TripPlannerPage.wiring.test.tsx b/client/src/pages/TripPlannerPage.wiring.test.tsx index 69861f580..6430106d4 100644 --- a/client/src/pages/TripPlannerPage.wiring.test.tsx +++ b/client/src/pages/TripPlannerPage.wiring.test.tsx @@ -165,6 +165,11 @@ function baseState(): HookState { rightCollapsed: false, setLeftCollapsed: vi.fn(), setRightCollapsed: vi.fn(), + leftHidden: false, + rightHidden: false, + toggleLeft: vi.fn(), + toggleRight: vi.fn(), + narrowPanels: false, startResizeLeft: vi.fn(), startResizeRight: vi.fn(), selectedPlaceId: null, @@ -371,7 +376,7 @@ describe('TripPlannerPage — plan tab', () => { }) it('FE-PAGE-TPW-009: collapsed panels report a zero width to the map', () => { - renderPage({ leftCollapsed: true, rightCollapsed: true }) + renderPage({ leftHidden: true, rightHidden: true }) expect(props('map').leftWidth).toBe(0) expect(props('map').rightWidth).toBe(0) @@ -410,19 +415,42 @@ describe('TripPlannerPage — plan tab', () => { const [leftBtn, rightBtn] = screen.getAllByRole('button') fireEvent.click(leftBtn) - expect(updaterOf(hookState.setLeftCollapsed)(false)).toBe(true) + expect(hookState.toggleLeft).toHaveBeenCalled() fireEvent.mouseEnter(leftBtn) fireEvent.mouseLeave(leftBtn) fireEvent.click(rightBtn) - expect(updaterOf(hookState.setRightCollapsed)(false)).toBe(true) + expect(hookState.toggleRight).toHaveBeenCalled() fireEvent.mouseEnter(rightBtn) fireEvent.mouseLeave(rightBtn) }) + // The tabs are the only way back to a panel on a touch device, and they used to + // announce themselves with nothing but a hover colour (#2247). + it('FE-PAGE-TPW-013b: each collapse tab is named for what it does', () => { + renderPage({ leftHidden: true, rightHidden: false }) + const [leftBtn, rightBtn] = screen.getAllByRole('button') + expect(leftBtn).toHaveAttribute('aria-label', 'trip.mobilePlan') + expect(rightBtn).toHaveAttribute('aria-label', 'common.collapse') + }) + + // At a foldable's ~860px the viewport centre sits under the Places panel, so the + // floating POI/compass cluster covered both tabs and the Add Place button (#2247). + it('FE-PAGE-TPW-013c: the floating map controls centre on the map corridor, not the viewport', () => { + const { container } = renderPage({ leftWidth: 340, rightWidth: 300 }) + const cluster = container.querySelector('div[style*="translateX(-50%)"][style*="z-index: 25"]') as HTMLElement + expect(cluster.style.left).toBe('calc(350px + 0.5 * (100% - 350px - 310px))') + }) + + it('FE-PAGE-TPW-013d: a hidden panel takes no corridor away from the map controls', () => { + const { container } = renderPage({ leftWidth: 340, rightWidth: 300, rightHidden: true }) + const cluster = container.querySelector('div[style*="translateX(-50%)"][style*="z-index: 25"]') as HTMLElement + expect(cluster.style.left).toBe('calc(350px + 0.5 * (100% - 350px - 0px))') + }) + it('FE-PAGE-TPW-014: a collapsed panel keeps its toggle but drops the hover highlight', () => { - renderPage({ leftCollapsed: true, rightCollapsed: true }) + renderPage({ leftHidden: true, rightHidden: true }) const [leftBtn, rightBtn] = screen.getAllByRole('button') fireEvent.mouseEnter(leftBtn) @@ -433,6 +461,13 @@ describe('TripPlannerPage — plan tab', () => { expect(screen.getByTestId('day-plan-sidebar')).toBeInTheDocument() }) + // Mouse-only handles (mousedown + document mousemove) are inert on a tablet, and + // a 4px invisible strip on the panel edge is a trap for a fat finger (#2247). + it('FE-PAGE-TPW-015b: the narrow layout drops the mouse-only resize handles', () => { + const { container } = renderPage({ narrowPanels: true }) + expect(container.querySelectorAll('div[style*="col-resize"]')).toHaveLength(0) + }) + it('FE-PAGE-TPW-015: the resize handles start a drag on mouse-down and highlight on hover', () => { const { container } = renderPage() const handles = container.querySelectorAll('div[style*="col-resize"]') diff --git a/client/src/pages/tripPlanner/useTripPlanner.ts b/client/src/pages/tripPlanner/useTripPlanner.ts index a6b538fad..4ed67dcad 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.ts +++ b/client/src/pages/tripPlanner/useTripPlanner.ts @@ -194,7 +194,11 @@ export function useTripPlanner() { if (activeTab === 'finanzplan') tripActions.loadBudgetItems?.(tripId) if (activeTab === 'dateien' && (!files || files.length === 0)) tripActions.loadFiles?.(tripId) }, [tripId]) - const { leftWidth, rightWidth, leftCollapsed, rightCollapsed, setLeftCollapsed, setRightCollapsed, startResizeLeft, startResizeRight } = useResizablePanels() + const { + leftWidth, rightWidth, leftCollapsed, rightCollapsed, setLeftCollapsed, setRightCollapsed, + leftHidden, rightHidden, toggleLeft, toggleRight, narrow: narrowPanels, + startResizeLeft, startResizeRight, + } = useResizablePanels() const { selectedPlaceId, selectedAssignmentId, setSelectedPlaceId, selectAssignment } = usePlaceSelection() const [showDayDetail, setShowDayDetail] = useState(null) const [dayDetailCollapsed, setDayDetailCollapsed] = useState(false) @@ -1081,7 +1085,9 @@ export function useTripPlanner() { enabledAddons, collabFeatures, tripAccommodations, setTripAccommodations, allowedFileTypes, tripMembers, setTripMembers, refreshMembers, loadAccommodations, TRANSPORT_TYPES, TRIP_TABS, activeTab, setActiveTab, handleTabChange, - leftWidth, rightWidth, leftCollapsed, rightCollapsed, setLeftCollapsed, setRightCollapsed, startResizeLeft, startResizeRight, + leftWidth, rightWidth, leftCollapsed, rightCollapsed, setLeftCollapsed, setRightCollapsed, + leftHidden, rightHidden, toggleLeft, toggleRight, narrowPanels, + startResizeLeft, startResizeRight, selectedPlaceId, selectedAssignmentId, setSelectedPlaceId, selectAssignment, showDayDetail, setShowDayDetail, dayDetailCollapsed, setDayDetailCollapsed, showPlaceForm, setShowPlaceForm, editingPlace, setEditingPlace, diff --git a/client/src/utils/mapViewport.test.ts b/client/src/utils/mapViewport.test.ts index bcd8d0b36..fab9a2cbb 100644 --- a/client/src/utils/mapViewport.test.ts +++ b/client/src/utils/mapViewport.test.ts @@ -223,4 +223,43 @@ describe('computeMapViewport', () => { expect(x).toBeLessThanOrEqual(VIEW.width) } }) + + // Two 300px planner panels on an 860px foldable left the framing window 140px + // wide, and the camera then opened on half of East Asia instead of the trip + // (#2247). The layout fix stops it happening; this stops it mattering. + it('never frames the camera for less than a third of the box, however wide the padding', () => { + const sane = computeMapViewport([PARIS, LYON], { + ...VIEW, + tileSize: TILE_SIZE_RASTER, + padding: { left: 380, right: 340 }, + })! + const absurd = computeMapViewport([PARIS, LYON], { + ...VIEW, + tileSize: TILE_SIZE_RASTER, + padding: { left: 480, right: 480 }, + })! + // 1000 - 960 = 40px of window would zoom ~2.5 levels further out than the + // third the clamp guarantees. + const floored = computeMapViewport([PARIS, LYON], { + width: VIEW.width / 3, + height: VIEW.height, + tileSize: TILE_SIZE_RASTER, + })! + expect(absurd.zoom).toBe(floored.zoom) + expect(absurd.zoom).toBeGreaterThan(sane.zoom - 3) + }) + + it('leaves a sane padding untouched', () => { + const withPadding = computeMapViewport([PARIS, LYON], { + ...VIEW, + tileSize: TILE_SIZE_RASTER, + padding: { left: 200, right: 100 }, + })! + const equivalent = computeMapViewport([PARIS, LYON], { + width: VIEW.width - 300, + height: VIEW.height, + tileSize: TILE_SIZE_RASTER, + })! + expect(withPadding.zoom).toBe(equivalent.zoom) + }) }) diff --git a/client/src/utils/mapViewport.ts b/client/src/utils/mapViewport.ts index 7041e5e8c..b0dce0835 100644 --- a/client/src/utils/mapViewport.ts +++ b/client/src/utils/mapViewport.ts @@ -215,8 +215,14 @@ export function computeMapViewport( const wrapsWorld = options.wrapsWorld ?? isGl const padding: ViewportPadding = { ...NO_PADDING, ...options.padding } - const width = Math.max(1, (options.width ?? defaultWidth()) - padding.left - padding.right) - const height = Math.max(1, (options.height ?? defaultHeight()) - padding.top - padding.bottom) + const rawWidth = options.width ?? defaultWidth() + const rawHeight = options.height ?? defaultHeight() + // Padding wider than the box leaves a sliver, and the camera is then framed for + // THAT — a continent instead of the trip. Two 300px planner panels on an 860px + // foldable did exactly this (#2247). Never surrender more than two thirds; the + // centre offset still uses the real padding, so the framing stays where it was. + const width = Math.max(1, rawWidth / 3, rawWidth - padding.left - padding.right) + const height = Math.max(1, rawHeight / 3, rawHeight - padding.top - padding.bottom) const lats = pts.map(p => p[0]) const south = Math.min(...lats) diff --git a/client/tests/integration/hooks/useResizablePanels.test.ts b/client/tests/integration/hooks/useResizablePanels.test.ts index b3b085336..89b1064f2 100644 --- a/client/tests/integration/hooks/useResizablePanels.test.ts +++ b/client/tests/integration/hooks/useResizablePanels.test.ts @@ -1,14 +1,38 @@ import { renderHook, act } from '@testing-library/react'; -import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { fireEvent } from '@testing-library/react'; import { useResizablePanels } from '../../../src/hooks/useResizablePanels'; +/** + * The suite's jsdom stub answers `matches: false` to everything, so the hook is + * in its wide layout unless a test says otherwise. `narrow()` puts it in the + * 768-1023 band the foldable lands in (#2247). + */ +function setViewport(width: number, narrow: boolean) { + Object.defineProperty(window, 'innerWidth', { writable: true, configurable: true, value: width }); + window.matchMedia = vi.fn().mockImplementation((query: string) => ({ + matches: query.includes('min-width: 768px') ? narrow : false, + media: query, + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + addListener: vi.fn(), + removeListener: vi.fn(), + dispatchEvent: vi.fn(), + })) as unknown as typeof window.matchMedia; +} + describe('useResizablePanels', () => { + const realMatchMedia = window.matchMedia; + beforeEach(() => { localStorage.clear(); vi.clearAllMocks(); }); + afterEach(() => { + window.matchMedia = realMatchMedia; + }); + it('FE-HOOK-PANELS-001: default leftWidth is 340 when localStorage is empty', () => { const { result } = renderHook(() => useResizablePanels()); expect(result.current.leftWidth).toBe(340); @@ -165,4 +189,85 @@ describe('useResizablePanels', () => { expect(result.current.setLeftCollapsed).toBeTypeOf('function'); expect(result.current.setRightCollapsed).toBeTypeOf('function'); }); + + // ── The narrow band, 768-1023px (#2247) ────────────────────────────────── + + it('FE-HOOK-PANELS-015: a wide layout shows whatever the collapse flags say', () => { + setViewport(1440, false); + const { result } = renderHook(() => useResizablePanels()); + expect(result.current.narrow).toBe(false); + expect(result.current.leftHidden).toBe(false); + expect(result.current.rightHidden).toBe(false); + + act(() => { result.current.toggleRight(); }); + expect(result.current.rightHidden).toBe(true); + expect(result.current.leftHidden).toBe(false); + expect(result.current.rightCollapsed).toBe(true); + }); + + it('FE-HOOK-PANELS-016: the narrow band opens the day plan and hides the places list', () => { + setViewport(860, true); + const { result } = renderHook(() => useResizablePanels()); + expect(result.current.narrow).toBe(true); + expect(result.current.leftHidden).toBe(false); + expect(result.current.rightHidden).toBe(true); + }); + + it('FE-HOOK-PANELS-017: opening one panel in the narrow band closes the other', () => { + setViewport(860, true); + const { result } = renderHook(() => useResizablePanels()); + + act(() => { result.current.toggleRight(); }); + expect(result.current.rightHidden).toBe(false); + expect(result.current.leftHidden).toBe(true); + + act(() => { result.current.toggleLeft(); }); + expect(result.current.leftHidden).toBe(false); + expect(result.current.rightHidden).toBe(true); + }); + + it('FE-HOOK-PANELS-018: tapping the open panel in the narrow band leaves the map alone', () => { + setViewport(860, true); + const { result } = renderHook(() => useResizablePanels()); + + act(() => { result.current.toggleLeft(); }); + expect(result.current.leftHidden).toBe(true); + expect(result.current.rightHidden).toBe(true); + }); + + // A marker tap calls both setters with false; in the narrow band that used to + // re-crowd the screen the user had just cleared. + it('FE-HOOK-PANELS-019: reopening both panels does not re-crowd the narrow layout', () => { + setViewport(860, true); + const { result } = renderHook(() => useResizablePanels()); + + act(() => { result.current.toggleRight(); }); + act(() => { result.current.setLeftCollapsed(false); result.current.setRightCollapsed(false); }); + expect(result.current.rightHidden).toBe(false); + expect(result.current.leftHidden).toBe(true); + }); + + it('FE-HOOK-PANELS-020: a panel dragged wide on a desktop is clamped so the map keeps 360px', () => { + localStorage.setItem('sidebarLeftWidth', '520'); + setViewport(800, true); + const { result } = renderHook(() => useResizablePanels()); + // 800 - 360 map - 20 margins = 420 + expect(result.current.leftWidth).toBe(420); + // The stored value is untouched, so a wide window gets its 520 back. + expect(localStorage.getItem('sidebarLeftWidth')).toBe('520'); + }); + + it('FE-HOOK-PANELS-021: the clamp never squeezes a panel below the drag minimum', () => { + localStorage.setItem('sidebarLeftWidth', '520'); + setViewport(500, true); + const { result } = renderHook(() => useResizablePanels()); + expect(result.current.leftWidth).toBe(200); + }); + + it('FE-HOOK-PANELS-022: a wide layout leaves the stored widths alone', () => { + localStorage.setItem('sidebarLeftWidth', '520'); + setViewport(1100, false); + const { result } = renderHook(() => useResizablePanels()); + expect(result.current.leftWidth).toBe(520); + }); }); diff --git a/client/tests/unit/mobile/trip/MPlanTimelineRows.test.tsx b/client/tests/unit/mobile/trip/MPlanTimelineRows.test.tsx index 7dfe25b34..551a2e5f0 100644 --- a/client/tests/unit/mobile/trip/MPlanTimelineRows.test.tsx +++ b/client/tests/unit/mobile/trip/MPlanTimelineRows.test.tsx @@ -519,4 +519,15 @@ describe('NoteRow', () => { expect(onEdit).toHaveBeenCalledTimes(1) expect(screen.getByTestId('reorder')).toBeInTheDocument() }) + + it('FE-MOB-PLROW-041: a link in the note body opens the link instead of the sheet', () => { + const onEdit = vi.fn() + render() + + fireEvent.click(screen.getByRole('link', { name: 'the map' })) + expect(onEdit).not.toHaveBeenCalled() + + fireEvent.click(screen.getByText('Buy museum tickets')) + expect(onEdit).toHaveBeenCalledTimes(1) + }) })