mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-21 22:46:32 +00:00
fix(planner): keep the places filter applied and visible across tab switches
The category and all/unplanned/tracks filters lived twice: a local copy in the sidebar driving the checkboxes and the list, and a page-level copy driving the map markers, synced only when a checkbox was clicked. Switching planner tabs unmounts the sidebar, so the local copy reset to "All Categories" while the markers stayed filtered — and the only way out was toggling any category on and off again (#1541). The filter now lives once in the trip store, next to selectedDayId: it survives the Plan tab unmounting (and the mobile places sheet closing), keeps both sidebar instances in agreement, and resets when another trip loads.
This commit is contained in:
Maurice
@@ -336,6 +336,39 @@ describe('Category filter dropdown', () => {
|
||||
await user.click(parkOpts[parkOpts.length - 1]);
|
||||
expect(screen.getByText(/2 categories/i)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('FE-PLANNER-SIDEBAR-047: category filter survives unmount/remount (#1541)', async () => {
|
||||
const user = userEvent.setup();
|
||||
const cat = buildCategory({ name: 'Hotel', color: '#3b82f6' });
|
||||
const withCat = buildPlace({ name: 'Grand Palace', category_id: cat.id, address: 'Vienna' });
|
||||
const noCat = buildPlace({ name: 'Street Market', category_id: null, address: 'Lisbon' });
|
||||
const { unmount } = render(<PlacesSidebar {...defaultProps} places={[withCat, noCat]} categories={[cat]} />);
|
||||
await user.click(screen.getByText(/All Categories/i));
|
||||
await user.click(screen.getByText('Hotel'));
|
||||
expect(screen.queryByText('Street Market')).not.toBeInTheDocument();
|
||||
// Switching planner tabs unmounts the sidebar; the filter must come back
|
||||
// both applied and visible instead of silently sticking on the map only.
|
||||
unmount();
|
||||
render(<PlacesSidebar {...defaultProps} places={[withCat, noCat]} categories={[cat]} />);
|
||||
expect(screen.queryByText(/All Categories/i)).not.toBeInTheDocument();
|
||||
expect(screen.getByText('Hotel')).toBeInTheDocument();
|
||||
expect(screen.getByText('Grand Palace')).toBeInTheDocument();
|
||||
expect(screen.queryByText('Street Market')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('FE-PLANNER-SIDEBAR-048: filter tab survives unmount/remount (#1541)', async () => {
|
||||
const user = userEvent.setup();
|
||||
const planned = buildPlace({ name: 'Planned Place' });
|
||||
const unplanned = buildPlace({ name: 'Unplanned Place' });
|
||||
const assignments = { '1': [buildAssignment({ place: planned, day_id: 1 })] };
|
||||
const { unmount } = render(<PlacesSidebar {...defaultProps} places={[planned, unplanned]} assignments={assignments} />);
|
||||
await user.click(screen.getByRole('button', { name: /Unplanned/i }));
|
||||
expect(screen.queryByText('Planned Place')).not.toBeInTheDocument();
|
||||
unmount();
|
||||
render(<PlacesSidebar {...defaultProps} places={[planned, unplanned]} assignments={assignments} />);
|
||||
expect(screen.queryByText('Planned Place')).not.toBeInTheDocument();
|
||||
expect(screen.getByText('Unplanned Place')).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
// ── Place list interaction ─────────────────────────────────────────────────────
|
||||
|
||||
@@ -23,8 +23,8 @@ export function PlacesHeader(S: SidebarState) {
|
||||
const {
|
||||
canEditPlaces, onAddPlace, t, setFileImportOpen, setListImportOpen, hasMultipleListImportProviders,
|
||||
places, categories, categoryFilters, search, setSearch, plannedIds, hasTracks,
|
||||
filter, setFilter, onPlacesFilterChange, setSelectedIds, selectMode, setSelectMode,
|
||||
catDropOpen, setCatDropOpen, toggleCategoryFilter, setCategoryFiltersLocal, onCategoryFilterChange,
|
||||
filter, setFilter, setSelectedIds, selectMode, setSelectMode,
|
||||
catDropOpen, setCatDropOpen, toggleCategoryFilter, setCategoryFilters,
|
||||
} = S
|
||||
return (
|
||||
<div className="border-b border-edge-faint" style={{ padding: '14px 16px 10px', flexShrink: 0 }}>
|
||||
@@ -99,7 +99,7 @@ export function PlacesHeader(S: SidebarState) {
|
||||
return (
|
||||
<button
|
||||
key={f.id}
|
||||
onClick={() => { setFilter(f.id); onPlacesFilterChange?.(f.id); setSelectedIds(new Set()) }}
|
||||
onClick={() => { setFilter(f.id); setSelectedIds(new Set()) }}
|
||||
className={active ? 'bg-accent text-accent-text' : 'bg-surface-card text-content'}
|
||||
style={{
|
||||
appearance: 'none', border: 'none', cursor: 'pointer', fontFamily: 'inherit',
|
||||
@@ -251,7 +251,7 @@ export function PlacesHeader(S: SidebarState) {
|
||||
)
|
||||
})()}
|
||||
{categoryFilters.size > 0 && (
|
||||
<button onClick={() => { setCategoryFiltersLocal(new Set()); onCategoryFilterChange?.(new Set()) }} className="bg-transparent text-content-faint" style={{
|
||||
<button onClick={() => setCategoryFilters(new Set())} className="bg-transparent text-content-faint" style={{
|
||||
display: 'flex', alignItems: 'center', justifyContent: 'center', gap: 4,
|
||||
width: '100%', padding: '6px 10px', borderRadius: 6, border: 'none', cursor: 'pointer',
|
||||
fontFamily: 'inherit', fontSize: 'calc(11px * var(--fs-scale-caption, 1))',
|
||||
|
||||
@@ -33,8 +33,6 @@ export interface PlacesSidebarProps {
|
||||
isMobile: boolean
|
||||
/** Primary pointer is coarse — HTML5 drag would swallow the scroll gesture (#1432). */
|
||||
isTouch?: boolean
|
||||
onCategoryFilterChange?: (categoryIds: Set<string>) => void
|
||||
onPlacesFilterChange?: (filter: string) => void
|
||||
pushUndo?: (label: string, undoFn: () => Promise<void> | void) => void
|
||||
initialScrollTop?: number
|
||||
onScrollTopChange?: (top: number) => void
|
||||
@@ -48,7 +46,7 @@ export interface PlacesSidebarProps {
|
||||
export function usePlacesSidebar(props: PlacesSidebarProps) {
|
||||
const {
|
||||
tripId, places, assignments, selectedDayId,
|
||||
onCategoryFilterChange, onPlacesFilterChange, pushUndo, initialScrollTop, onScrollTopChange,
|
||||
pushUndo, initialScrollTop, onScrollTopChange,
|
||||
} = props
|
||||
const { t } = useTranslation()
|
||||
const toast = useToast()
|
||||
@@ -149,8 +147,13 @@ export function usePlacesSidebar(props: PlacesSidebarProps) {
|
||||
}
|
||||
|
||||
const [search, setSearch] = useState('')
|
||||
const [filter, setFilter] = useState('all')
|
||||
const [categoryFilters, setCategoryFiltersLocal] = useState<Set<string>>(new Set())
|
||||
// Filter state lives in the trip store so it survives the Plan tab
|
||||
// unmounting (tab switch, mobile sheet close) and stays in lockstep with the
|
||||
// map markers, which filter on the same values (#1541).
|
||||
const filter = useTripStore((s) => s.placesFilter)
|
||||
const setFilter = useTripStore((s) => s.setPlacesFilter)
|
||||
const categoryFilters = useTripStore((s) => s.placesCategoryFilter)
|
||||
const setCategoryFilters = useTripStore((s) => s.setPlacesCategoryFilter)
|
||||
const [selectMode, setSelectMode] = useState(false)
|
||||
const [selectedIds, setSelectedIds] = useState<Set<number>>(new Set())
|
||||
const [pendingDeleteIds, setPendingDeleteIds] = useState<number[] | null>(null)
|
||||
@@ -177,12 +180,9 @@ export function usePlacesSidebar(props: PlacesSidebarProps) {
|
||||
}), [])
|
||||
|
||||
const toggleCategoryFilter = (catId: string) => {
|
||||
setCategoryFiltersLocal(prev => {
|
||||
const next = new Set(prev)
|
||||
if (next.has(catId)) next.delete(catId); else next.add(catId)
|
||||
onCategoryFilterChange?.(next)
|
||||
return next
|
||||
})
|
||||
const next = new Set(categoryFilters)
|
||||
if (next.has(catId)) next.delete(catId); else next.add(catId)
|
||||
setCategoryFilters(next)
|
||||
}
|
||||
const [dayPickerPlace, setDayPickerPlace] = useState(null)
|
||||
const [catDropOpen, setCatDropOpen] = useState(false)
|
||||
@@ -266,7 +266,7 @@ export function usePlacesSidebar(props: PlacesSidebarProps) {
|
||||
listImportLoading, listImportProvider, setListImportProvider,
|
||||
listImportEnrich, setListImportEnrich, canEnrichImport,
|
||||
availableListImportProviders, hasMultipleListImportProviders, handleListImport,
|
||||
search, setSearch, filter, setFilter, categoryFilters, setCategoryFiltersLocal,
|
||||
search, setSearch, filter, setFilter, categoryFilters, setCategoryFilters,
|
||||
selectMode, setSelectMode, selectedIds, setSelectedIds, pendingDeleteIds, setPendingDeleteIds,
|
||||
categoryPickerOpen, setCategoryPickerOpen,
|
||||
saveToListOpen, setSaveToListOpen, collectionsEnabled, tripId,
|
||||
|
||||
@@ -203,7 +203,7 @@ export default function TripPlannerPage(): React.ReactElement | null {
|
||||
mobileSidebarOpen, setMobileSidebarOpen, mobilePlanScrollTopRef, mobilePlacesScrollTopRef,
|
||||
deletePlaceId, setDeletePlaceId, deletePlaceIds, setDeletePlaceIds,
|
||||
visibleConnections, setVisibleConnections, toggleConnection, mapTransportDetail, setMapTransportDetail,
|
||||
isMobile, isTouch, mapCategoryFilter, setMapCategoryFilter, mapPlacesFilter, setMapPlacesFilter,
|
||||
isMobile, isTouch,
|
||||
expandedDayIds, setExpandedDayIds, mapPlaces,
|
||||
route, routeSegments, routeInfo, setRoute, setRouteInfo, updateRouteForDay,
|
||||
handleSelectDay, handlePlaceClick, handleMarkerClick, handleMapClick, handleMapContextMenu, openAddPlaceFromPoi,
|
||||
@@ -494,8 +494,6 @@ export default function TripPlannerPage(): React.ReactElement | null {
|
||||
onDeletePlace={(placeId) => handleDeletePlace(placeId)}
|
||||
onBulkDeletePlaces={(ids) => setDeletePlaceIds(ids)}
|
||||
onBulkChangeCategory={(ids, catId) => confirmChangeCategory(ids, catId)}
|
||||
onCategoryFilterChange={setMapCategoryFilter}
|
||||
onPlacesFilterChange={setMapPlacesFilter}
|
||||
pushUndo={pushUndo}
|
||||
days={days}
|
||||
isMobile={false}
|
||||
@@ -638,7 +636,7 @@ export default function TripPlannerPage(): React.ReactElement | null {
|
||||
<div style={{ flex: 1, overflow: 'auto', paddingBottom: 'env(safe-area-inset-bottom, 0px)' }}>
|
||||
{mobileSidebarOpen === 'left'
|
||||
? <DayPlanSidebar tripId={tripId} trip={trip} days={days} places={places} categories={categories} assignments={assignments} selectedDayId={selectedDayId} selectedPlaceId={selectedPlaceId} selectedAssignmentId={selectedAssignmentId} onSelectDay={(id) => { handleSelectDay(id); setMobileSidebarOpen(null) }} onPlaceClick={(placeId, assignmentId) => { handlePlaceClick(placeId, assignmentId) }} onReorder={handleReorder} onReorderDays={handleReorderDays} onAddDay={handleAddDay} onUpdateDayTitle={handleUpdateDayTitle} onAssignToDay={handleAssignToDay} onRouteCalculated={(r) => { if (r) { setRoute([r.coordinates]); setRouteInfo(r) } }} reservations={reservations} visibleConnectionIds={visibleConnections} onToggleConnection={toggleConnection} onAddReservation={(dayId) => { setEditingReservation(null); tripActions.setSelectedDay(dayId); setShowReservationModal(true); setMobileSidebarOpen(null) }} onAddTransport={can('day_edit', trip) ? (dayId) => { setTransportModalDayId(dayId); setEditingTransport(null); setTransitPrefill(null); setTransportModalAutomated(false); setShowTransportModal(true); setMobileSidebarOpen(null) } : undefined} onOpenTransit={(r) => { setTransitJourney(r); setMobileSidebarOpen(null) }} onPlanTransit={can('day_edit', trip) && tripHasDates ? (dayId) => { setTransportModalDayId(dayId); setEditingTransport(null); setTransitPrefill(null); setTransportModalAutomated(true); setShowTransportModal(true); setMobileSidebarOpen(null) } : undefined} onAddPlace={() => { setEditingPlace(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onDayDetail={(day) => { setShowDayDetail(day); setSelectedPlaceId(null); selectAssignment(null) }} onRemoveAssignment={handleRemoveAssignment} onEditPlace={(place, assignmentId) => { setEditingPlace(place); setEditingAssignmentId(assignmentId || null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onDeletePlace={(placeId) => handleDeletePlace(placeId)} accommodations={tripAccommodations} routeShown={routeShown} routeProfile={routeProfile} onToggleRoute={() => setRouteShown(v => !v)} onSetRouteProfile={setRouteProfile} 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 }} showRouteToolsWhenExpanded isMobile />
|
||||
: <PlacesSidebar tripId={tripId} places={places} categories={categories} assignments={assignments} selectedDayId={selectedDayId} selectedPlaceId={selectedPlaceId} onPlaceClick={(placeId) => { handlePlaceClick(placeId); setMobileSidebarOpen(null) }} onAddPlace={() => { setEditingPlace(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onAssignToDay={handleAssignToDay} onEditPlace={(place) => { openPlaceEditor(place); setMobileSidebarOpen(null) }} onDeletePlace={(placeId) => handleDeletePlace(placeId)} onBulkDeletePlaces={(ids) => setDeletePlaceIds(ids)} onBulkDeleteConfirm={(ids) => confirmDeletePlaces(ids)} onBulkChangeCategory={(ids, catId) => confirmChangeCategory(ids, catId)} days={days} isMobile onCategoryFilterChange={setMapCategoryFilter} onPlacesFilterChange={setMapPlacesFilter} pushUndo={pushUndo} initialScrollTop={mobilePlacesScrollTopRef.current} onScrollTopChange={(top) => { mobilePlacesScrollTopRef.current = top }} />
|
||||
: <PlacesSidebar tripId={tripId} places={places} categories={categories} assignments={assignments} selectedDayId={selectedDayId} selectedPlaceId={selectedPlaceId} onPlaceClick={(placeId) => { handlePlaceClick(placeId); setMobileSidebarOpen(null) }} onAddPlace={() => { setEditingPlace(null); setShowPlaceForm(true); setMobileSidebarOpen(null) }} onAssignToDay={handleAssignToDay} onEditPlace={(place) => { openPlaceEditor(place); setMobileSidebarOpen(null) }} onDeletePlace={(placeId) => handleDeletePlace(placeId)} onBulkDeletePlaces={(ids) => setDeletePlaceIds(ids)} onBulkDeleteConfirm={(ids) => confirmDeletePlaces(ids)} onBulkChangeCategory={(ids, catId) => confirmChangeCategory(ids, catId)} days={days} isMobile pushUndo={pushUndo} initialScrollTop={mobilePlacesScrollTopRef.current} onScrollTopChange={(top) => { mobilePlacesScrollTopRef.current = top }} />
|
||||
}
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -308,8 +308,10 @@ export function useTripPlanner() {
|
||||
|
||||
useTripWebSocket(tripId)
|
||||
|
||||
const [mapCategoryFilter, setMapCategoryFilter] = useState<Set<string>>(new Set())
|
||||
const [mapPlacesFilter, setMapPlacesFilter] = useState<string>('all')
|
||||
// Same filter the places sidebar renders — shared via the store so tab
|
||||
// switches can't desync the marker set from the filter UI (#1541).
|
||||
const placesFilter = useTripStore((s) => s.placesFilter)
|
||||
const placesCategoryFilter = useTripStore((s) => s.placesCategoryFilter)
|
||||
|
||||
const [expandedDayIds, setExpandedDayIds] = useState<Set<number> | null>(null)
|
||||
|
||||
@@ -335,23 +337,23 @@ export function useTripPlanner() {
|
||||
}
|
||||
|
||||
// Build set of planned place IDs for unplanned filter
|
||||
const plannedIds = mapPlacesFilter === 'unplanned'
|
||||
const plannedIds = placesFilter === 'unplanned'
|
||||
? new Set(Object.values(assignments).flatMap(da => da.map(a => a.place?.id).filter(Boolean)))
|
||||
: null
|
||||
|
||||
return places.filter(p => {
|
||||
if (!p.lat || !p.lng) return false
|
||||
if (mapPlacesFilter === 'tracks' && !p.route_geometry) return false
|
||||
if (mapCategoryFilter.size > 0) {
|
||||
if (placesFilter === 'tracks' && !p.route_geometry) return false
|
||||
if (placesCategoryFilter.size > 0) {
|
||||
if (p.category_id == null) {
|
||||
if (!mapCategoryFilter.has('uncategorized')) return false
|
||||
} else if (!mapCategoryFilter.has(String(p.category_id))) return false
|
||||
if (!placesCategoryFilter.has('uncategorized')) return false
|
||||
} else if (!placesCategoryFilter.has(String(p.category_id))) return false
|
||||
}
|
||||
if (hiddenPlaceIds.has(p.id)) return false
|
||||
if (plannedIds && plannedIds.has(p.id)) return false
|
||||
return true
|
||||
})
|
||||
}, [places, mapCategoryFilter, mapPlacesFilter, assignments, expandedDayIds])
|
||||
}, [places, placesCategoryFilter, placesFilter, assignments, expandedDayIds])
|
||||
|
||||
const { route, routeSegments, routeInfo, setRoute, setRouteInfo, updateRouteForDay } = useRouteCalculation({ assignments } as any, selectedDayId, routeShown, routeProfile, tripAccommodations)
|
||||
|
||||
@@ -915,7 +917,7 @@ export function useTripPlanner() {
|
||||
mobileSidebarOpen, setMobileSidebarOpen, mobilePlanScrollTopRef, mobilePlacesScrollTopRef,
|
||||
deletePlaceId, setDeletePlaceId, deletePlaceIds, setDeletePlaceIds,
|
||||
visibleConnections, setVisibleConnections, toggleConnection, mapTransportDetail, setMapTransportDetail,
|
||||
isMobile, isTouch, mapCategoryFilter, setMapCategoryFilter, mapPlacesFilter, setMapPlacesFilter,
|
||||
isMobile, isTouch,
|
||||
expandedDayIds, setExpandedDayIds, mapPlaces,
|
||||
route, routeSegments, routeInfo, setRoute, setRouteInfo, updateRouteForDay,
|
||||
handleSelectDay, handlePlaceClick, handleMarkerClick, handleMapClick, handleMapContextMenu, openAddPlaceFromPoi,
|
||||
|
||||
@@ -60,10 +60,17 @@ export interface TripStoreState
|
||||
files: TripFile[]
|
||||
reservations: Reservation[]
|
||||
selectedDayId: number | null
|
||||
// Places filter (list + map markers). Lives here, not in the sidebar, so the
|
||||
// applied filter and the filter UI can never drift apart when the Plan tab
|
||||
// unmounts and remounts (#1541).
|
||||
placesFilter: string
|
||||
placesCategoryFilter: Set<string>
|
||||
isLoading: boolean
|
||||
error: string | null
|
||||
|
||||
setSelectedDay: (dayId: number | null) => void
|
||||
setPlacesFilter: (filter: string) => void
|
||||
setPlacesCategoryFilter: (categoryIds: Set<string>) => void
|
||||
handleRemoteEvent: (event: WebSocketEvent) => void
|
||||
resetTrip: () => void
|
||||
loadTrip: (tripId: number | string) => Promise<void>
|
||||
@@ -88,10 +95,14 @@ export const useTripStore = create<TripStoreState>((set, get) => ({
|
||||
files: [],
|
||||
reservations: [],
|
||||
selectedDayId: null,
|
||||
placesFilter: 'all',
|
||||
placesCategoryFilter: new Set<string>(),
|
||||
isLoading: false,
|
||||
error: null,
|
||||
|
||||
setSelectedDay: (dayId: number | null) => set({ selectedDayId: dayId }),
|
||||
setPlacesFilter: (filter: string) => set({ placesFilter: filter }),
|
||||
setPlacesCategoryFilter: (categoryIds: Set<string>) => set({ placesCategoryFilter: categoryIds }),
|
||||
|
||||
handleRemoteEvent: (event: WebSocketEvent) => handleRemoteEvent(set, get, event),
|
||||
|
||||
@@ -110,6 +121,8 @@ export const useTripStore = create<TripStoreState>((set, get) => ({
|
||||
files: [],
|
||||
reservations: [],
|
||||
selectedDayId: null,
|
||||
placesFilter: 'all',
|
||||
placesCategoryFilter: new Set<string>(),
|
||||
error: null,
|
||||
}),
|
||||
|
||||
|
||||
Reference in New Issue
Block a user