diff --git a/client/src/pages/CollectionsPage.test.tsx b/client/src/pages/CollectionsPage.test.tsx index 1e7634f63..09509527b 100644 --- a/client/src/pages/CollectionsPage.test.tsx +++ b/client/src/pages/CollectionsPage.test.tsx @@ -25,7 +25,13 @@ vi.mock('../components/Collections/CollectionFilterBar', () => ({ ? :
, })) -vi.mock('../components/Collections/CollectionMapPanel', () => ({ default: () =>
})) +const mapPanelProps = vi.hoisted(() => ({ current: null as null | { onSelect: (id: number) => void; onDeselect: () => void } })) +vi.mock('../components/Collections/CollectionMapPanel', () => ({ + default: (props: { onSelect: (id: number) => void; onDeselect: () => void }) => { + mapPanelProps.current = props + return
+ }, +})) vi.mock('../components/Collections/CopyToTripModal', () => ({ default: () => null })) vi.mock('../components/Collections/MoveToListModal', () => ({ default: () => null })) vi.mock('../components/Collections/ShareCollectionModal', () => ({ default: () => null })) @@ -147,3 +153,17 @@ describe('CollectionsPage — the map survives a filter that matches nothing', ( expect(screen.queryByTestId('map')).toBeNull() }) }) + +describe('CollectionsPage: a marker click shows the place (#2431)', () => { + it('selects the place a marker stands for, even when that place is already selected', () => { + const setSelectedPlaceId = vi.fn() + mockUseCollections.mockReturnValue(makeHook({ selectedPlaceId: 10, setSelectedPlaceId })) + render() + // A second click on the marker of the open place used to toggle it away, which + // read as the details refusing to open. The map background is what clears. + mapPanelProps.current!.onSelect(10) + expect(setSelectedPlaceId).toHaveBeenLastCalledWith(10) + mapPanelProps.current!.onDeselect() + expect(setSelectedPlaceId).toHaveBeenLastCalledWith(null) + }) +}) diff --git a/client/src/pages/CollectionsPage.tsx b/client/src/pages/CollectionsPage.tsx index c2e21eb42..21ceecfac 100644 --- a/client/src/pages/CollectionsPage.tsx +++ b/client/src/pages/CollectionsPage.tsx @@ -45,12 +45,16 @@ function CollectionsPageDesktop(): React.ReactElement { const isRealList = !c.isAllSaved && typeof c.activeId === 'number' const canManageLabels = isRealList && c.canEdit - // Selecting a place toggles it, so clicking it again — or the map background — - // clears it. Below the desktop breakpoint the list and map are separate views; - // above it the list view is a split with a persistent map that pans to the - // selection (the map stays mounted across the list↔map toggle so it animates). + // Selecting a place from the list toggles it, so clicking the row again clears + // it, as does the map background. A marker click always shows the place: a second + // click on the marker of the place that is open read as "the details do not open" + // rather than as "close them" (#2431). Below the desktop breakpoint the list and + // map are separate views; above it the list view is a split with a persistent map + // that pans to the selection (the map stays mounted across the list↔map toggle so + // it animates). const mappable = c.mappable const openPlace = (id: number) => c.setSelectedPlaceId(c.selectedPlaceId === id ? null : id) + const showPlace = (id: number) => c.setSelectedPlaceId(id) const deselect = () => c.setSelectedPlaceId(null) const toggleView = () => { // Going to the full-map view closes the (list-docked) detail sheet. @@ -59,7 +63,7 @@ function CollectionsPageDesktop(): React.ReactElement { } // Clicking a marker in the full-map view drops back to the split so the list // + detail come into view alongside the map. - const onMapSelect = (id: number) => { openPlace(id); if (c.view === 'map') c.setView('list') } + const onMapSelect = (id: number) => { showPlace(id); if (c.view === 'map') c.setView('list') } const desktopSplit = c.isWide && c.hasMappable const mapShown = c.hasMappable && (c.view === 'map' || c.isWide)