From 047b334ca4bab3549885229fcfbe064e65f98e12 Mon Sep 17 00:00:00 2001 From: jubnl Date: Fri, 1 May 2026 00:51:54 +0200 Subject: [PATCH] fix: restore scroll position in mobile Plan and Places sidebars on reopen (issue #932) Both DayPlanSidebar and PlacesSidebar have their own internal scroll containers (overflowY: auto). Scroll events don't bubble, so previous attempts that tracked scrollTop on the outer portal div never fired. Each sidebar now accepts initialScrollTop and onScrollTopChange props. The internal scroll container saves its scrollTop via onScrollTopChange on every scroll event, and restores it via useLayoutEffect on mount (before the browser paints, so no visible flash). TripPlannerPage holds the saved values in refs (mobilePlanScrollTopRef, mobilePlacesScrollTopRef) and passes them through on each portal mount. --- .../src/components/Planner/DayPlanSidebar.tsx | 14 +++++- .../src/components/Planner/PlacesSidebar.tsx | 13 ++++- client/src/pages/TripPlannerPage.test.tsx | 50 +++++++++++++++++++ client/src/pages/TripPlannerPage.tsx | 6 ++- 4 files changed, 77 insertions(+), 6 deletions(-) diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index 3b7a5a22..14fcd498 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -2,7 +2,7 @@ interface DragDataPayload { placeId?: string; assignmentId?: string; noteId?: string; reservationId?: string; fromDayId?: string; phase?: 'single' | 'start' | 'middle' | 'end' } declare global { interface Window { __dragData: DragDataPayload | null } } -import React, { useState, useEffect, useRef, useMemo } from 'react' +import React, { useState, useEffect, useLayoutEffect, useRef, useMemo } from 'react' import ReactDOM from 'react-dom' import { ChevronDown, ChevronRight, ChevronUp, ChevronsDownUp, ChevronsUpDown, Navigation, RotateCcw, ExternalLink, Clock, Pencil, GripVertical, Ticket, Plus, FileText, Check, Trash2, Info, MapPin, Star, Heart, Camera, Lightbulb, Flag, Bookmark, Train, Bus, Plane, Car, Ship, Coffee, ShoppingBag, AlertTriangle, FileDown, Lock, Hotel, Utensils, Users, Undo2, X, Route as RouteIcon } from 'lucide-react' @@ -191,6 +191,8 @@ interface DayPlanSidebarProps { onEditTransport?: (reservation: Reservation) => void onEditReservation?: (reservation: Reservation) => void onAddBookingToAssignment?: (dayId: number, assignmentId: number) => void + initialScrollTop?: number + onScrollTopChange?: (top: number) => void } const DayPlanSidebar = React.memo(function DayPlanSidebar({ @@ -219,6 +221,8 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar({ onEditTransport, onEditReservation, onAddBookingToAssignment, + initialScrollTop, + onScrollTopChange, }: DayPlanSidebarProps) { const toast = useToast() const { t, language, locale } = useTranslation() @@ -271,6 +275,12 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar({ } | null>(null) const inputRef = useRef(null) const dragDataRef = useRef(null) + const scrollContainerRef = useRef(null) + useLayoutEffect(() => { + if (scrollContainerRef.current && initialScrollTop) { + scrollContainerRef.current.scrollTop = initialScrollTop + } + }, []) const initedTransportIds = useRef(new Set()) // Speichert Drag-Daten als Backup (dataTransfer geht bei Re-Render verloren) // Remember which assignment we last auto-scrolled into view so we don't // keep yanking the user back whenever they scroll away while the same @@ -1118,7 +1128,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar({ {/* Tagesliste */} -
+
onScrollTopChange?.((e.currentTarget as HTMLElement).scrollTop)}> {days.map((day, index) => { const isSelected = selectedDayId === day.id const isExpanded = expandedDays.has(day.id) diff --git a/client/src/components/Planner/PlacesSidebar.tsx b/client/src/components/Planner/PlacesSidebar.tsx index a3d45d63..50303768 100644 --- a/client/src/components/Planner/PlacesSidebar.tsx +++ b/client/src/components/Planner/PlacesSidebar.tsx @@ -1,6 +1,6 @@ import React from 'react' import ReactDOM from 'react-dom' -import { useState, useMemo, useEffect, useRef, useCallback } from 'react' +import { useState, useMemo, useEffect, useLayoutEffect, useRef, useCallback } from 'react' import { Search, Plus, X, CalendarDays, Pencil, Trash2, ExternalLink, Navigation, Upload, ChevronDown, Check, MapPin, Eye, Route } from 'lucide-react' import PlaceAvatar from '../shared/PlaceAvatar' import { getCategoryIcon } from '../shared/categoryIcons' @@ -34,6 +34,8 @@ interface PlacesSidebarProps { onCategoryFilterChange?: (categoryIds: Set) => void onPlacesFilterChange?: (filter: string) => void pushUndo?: (label: string, undoFn: () => Promise | void) => void + initialScrollTop?: number + onScrollTopChange?: (top: number) => void } interface MemoPlaceRowProps { @@ -145,6 +147,7 @@ const MemoPlaceRow = React.memo(function MemoPlaceRow({ const PlacesSidebar = React.memo(function PlacesSidebar({ tripId, places, categories, assignments, selectedDayId, selectedPlaceId, onPlaceClick, onAddPlace, onAssignToDay, onEditPlace, onDeletePlace, onBulkDeletePlaces, onBulkDeleteConfirm, days, isMobile, onCategoryFilterChange, onPlacesFilterChange, pushUndo, + initialScrollTop, onScrollTopChange, }: PlacesSidebarProps) { const { t } = useTranslation() const toast = useToast() @@ -159,6 +162,12 @@ const PlacesSidebar = React.memo(function PlacesSidebar({ const [sidebarDropFile, setSidebarDropFile] = useState(null) const [sidebarDragOver, setSidebarDragOver] = useState(false) const sidebarDragCounter = useRef(0) + const scrollContainerRef = useRef(null) + useLayoutEffect(() => { + if (scrollContainerRef.current && initialScrollTop) { + scrollContainerRef.current.scrollTop = initialScrollTop + } + }, []) const handleSidebarDragEnter = (e: React.DragEvent) => { if (!canEditPlaces) return @@ -636,7 +645,7 @@ const PlacesSidebar = React.memo(function PlacesSidebar({ )} {/* Liste */} -
+
onScrollTopChange?.((e.currentTarget as HTMLElement).scrollTop)}> {filtered.length === 0 ? (
diff --git a/client/src/pages/TripPlannerPage.test.tsx b/client/src/pages/TripPlannerPage.test.tsx index 78096249..3ea43503 100644 --- a/client/src/pages/TripPlannerPage.test.tsx +++ b/client/src/pages/TripPlannerPage.test.tsx @@ -1474,6 +1474,56 @@ describe('TripPlannerPage', () => { }); }); + describe('FE-PAGE-PLANNER-051: Mobile Plan sidebar stays mounted after onPlaceClick (issue #932)', () => { + it('does not unmount the mobile Plan portal when a place is tapped, preserving scroll position', async () => { + vi.useFakeTimers(); + Object.defineProperty(window, 'innerWidth', { writable: true, configurable: true, value: 375 }); + + const place = buildPlace({ id: 1, trip_id: 42, lat: 48.8566, lng: 2.3522 }); + const assignment = buildAssignment({ id: 10, day_id: 99, place, order_index: 0 }); + seedTripStore({ id: 42 }); + seedStore(useTripStore, { + places: [place], + assignments: { '99': [assignment] }, + } as any); + + renderPlannerPage(42); + act(() => { vi.runAllTimers(); }); + vi.useRealTimers(); + + await waitFor(() => { + expect(screen.getByTestId('day-plan-sidebar')).toBeInTheDocument(); + }); + + // Open the mobile Plan portal via the bottom-nav Plan button (selector mirrors FE-PAGE-PLANNER-049). + const mobilePlanBtn = Array.from(document.body.querySelectorAll('button')).find( + b => b.textContent === 'Plan' && !b.getAttribute('title'), + ); + expect(mobilePlanBtn).toBeTruthy(); + await act(async () => { fireEvent.click(mobilePlanBtn!); }); + + await waitFor(() => { + expect(screen.getAllByTestId('day-plan-sidebar').length).toBe(2); + }); + + // The mock factory overwrites capturedDayPlanSidebarProps on each mount, + // so current holds the mobile portal instance's props. + const mobileOnPlaceClick = capturedDayPlanSidebarProps.current.onPlaceClick; + expect(typeof mobileOnPlaceClick).toBe('function'); + + await act(async () => { + mobileOnPlaceClick(place.id, assignment.id); + }); + + // Invariant: portal must NOT unmount — both instances persist. + // Pre-fix: collapses to 1 (setMobileSidebarOpen(null) destroyed scroll container). + // Post-fix: stays at 2, browser preserves scrollTop on the living DOM node. + expect(screen.getAllByTestId('day-plan-sidebar').length).toBe(2); + + Object.defineProperty(window, 'innerWidth', { writable: true, configurable: true, value: 1024 }); + }); + }); + describe('FE-PAGE-PLANNER-037: onExpandedDaysChange covers mapPlaces hidden logic', () => { it('calls onExpandedDaysChange to trigger mapPlaces hidden set computation', async () => { vi.useFakeTimers(); diff --git a/client/src/pages/TripPlannerPage.tsx b/client/src/pages/TripPlannerPage.tsx index 54cc4726..8be7cd58 100644 --- a/client/src/pages/TripPlannerPage.tsx +++ b/client/src/pages/TripPlannerPage.tsx @@ -272,6 +272,8 @@ export default function TripPlannerPage(): React.ReactElement | null { const [fitKey, setFitKey] = useState(0) const initialFitTripId = useRef(null) const [mobileSidebarOpen, setMobileSidebarOpen] = useState<'left' | 'right' | null>(null) + const mobilePlanScrollTopRef = useRef(0) + const mobilePlacesScrollTopRef = useRef(0) const [deletePlaceId, setDeletePlaceId] = useState(null) const [deletePlaceIds, setDeletePlaceIds] = useState(null) @@ -1114,8 +1116,8 @@ export default function TripPlannerPage(): React.ReactElement | null {
{mobileSidebarOpen === 'left' - ? { handleSelectDay(id); setMobileSidebarOpen(null) }} onPlaceClick={(placeId, assignmentId) => { handlePlaceClick(placeId, assignmentId); setMobileSidebarOpen(null) }} onReorder={handleReorder} onUpdateDayTitle={handleUpdateDayTitle} onAssignToDay={handleAssignToDay} onRouteCalculated={(r) => { if (r) { setRoute(r.coordinates); setRouteInfo({ distance: r.distanceText, duration: r.durationText }) } }} reservations={reservations} visibleConnectionIds={visibleConnections} onToggleConnection={toggleConnection} onAddReservation={(dayId) => { setEditingReservation(null); tripActions.setSelectedDay(dayId); setShowReservationModal(true); setMobileSidebarOpen(null) }} onAddPlace={() => { setEditingPlace(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onDayDetail={(day) => { setShowDayDetail(day); setSelectedPlaceId(null); selectAssignment(null); setMobileSidebarOpen(null) }} accommodations={tripAccommodations} onNavigateToFiles={() => { setMobileSidebarOpen(null); handleTabChange('dateien') }} onExpandedDaysChange={setExpandedDayIds} pushUndo={pushUndo} canUndo={canUndo} lastActionLabel={lastActionLabel} onUndo={handleUndo} onEditTransport={can('day_edit', trip) ? (reservation) => { setEditingTransport(reservation); setTransportModalDayId(reservation.day_id ?? null); setShowTransportModal(true); setMobileSidebarOpen(null) } : undefined} onEditReservation={can('reservation_edit', trip) ? (r) => { setEditingReservation(r); setShowReservationModal(true); setMobileSidebarOpen(null) } : undefined} /> - : { handlePlaceClick(placeId); setMobileSidebarOpen(null) }} onAddPlace={() => { setEditingPlace(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onAssignToDay={handleAssignToDay} onEditPlace={(place) => { setEditingPlace(place); setEditingAssignmentId(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onDeletePlace={(placeId) => handleDeletePlace(placeId)} onBulkDeletePlaces={(ids) => setDeletePlaceIds(ids)} onBulkDeleteConfirm={(ids) => confirmDeletePlaces(ids)} days={days} isMobile onCategoryFilterChange={setMapCategoryFilter} onPlacesFilterChange={setMapPlacesFilter} pushUndo={pushUndo} /> + ? { handleSelectDay(id); setMobileSidebarOpen(null) }} onPlaceClick={(placeId, assignmentId) => { handlePlaceClick(placeId, assignmentId) }} onReorder={handleReorder} onUpdateDayTitle={handleUpdateDayTitle} onAssignToDay={handleAssignToDay} onRouteCalculated={(r) => { if (r) { setRoute(r.coordinates); setRouteInfo({ distance: r.distanceText, duration: r.durationText }) } }} reservations={reservations} visibleConnectionIds={visibleConnections} onToggleConnection={toggleConnection} onAddReservation={(dayId) => { setEditingReservation(null); tripActions.setSelectedDay(dayId); setShowReservationModal(true); setMobileSidebarOpen(null) }} onAddPlace={() => { setEditingPlace(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onDayDetail={(day) => { setShowDayDetail(day); setSelectedPlaceId(null); selectAssignment(null); setMobileSidebarOpen(null) }} accommodations={tripAccommodations} onNavigateToFiles={() => { setMobileSidebarOpen(null); handleTabChange('dateien') }} onExpandedDaysChange={setExpandedDayIds} pushUndo={pushUndo} canUndo={canUndo} lastActionLabel={lastActionLabel} onUndo={handleUndo} onEditTransport={can('day_edit', trip) ? (reservation) => { setEditingTransport(reservation); setTransportModalDayId(reservation.day_id ?? null); setShowTransportModal(true); setMobileSidebarOpen(null) } : undefined} onEditReservation={can('reservation_edit', trip) ? (r) => { setEditingReservation(r); setShowReservationModal(true); setMobileSidebarOpen(null) } : undefined} initialScrollTop={mobilePlanScrollTopRef.current} onScrollTopChange={(top) => { mobilePlanScrollTopRef.current = top }} /> + : { handlePlaceClick(placeId); setMobileSidebarOpen(null) }} onAddPlace={() => { setEditingPlace(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onAssignToDay={handleAssignToDay} onEditPlace={(place) => { setEditingPlace(place); setEditingAssignmentId(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onDeletePlace={(placeId) => handleDeletePlace(placeId)} onBulkDeletePlaces={(ids) => setDeletePlaceIds(ids)} onBulkDeleteConfirm={(ids) => confirmDeletePlaces(ids)} days={days} isMobile onCategoryFilterChange={setMapCategoryFilter} onPlacesFilterChange={setMapPlacesFilter} pushUndo={pushUndo} initialScrollTop={mobilePlacesScrollTopRef.current} onScrollTopChange={(top) => { mobilePlacesScrollTopRef.current = top }} /> }