fix(collections): a marker click shows the place, every time

A marker click toggled the selection, so the second click on the marker of the
place that was open closed it again, which read as the details refusing to
open: every other click on the same marker showed nothing. A marker click now
selects and never clears; the map background and the close control do that,
and a list row still toggles as before.

Closes #2431
This commit is contained in:
Maurice
2026-09-21 17:54:22 +02:00
parent 903378f8ad
commit 0644106f7a
2 changed files with 30 additions and 6 deletions
+21 -1
View File
@@ -25,7 +25,13 @@ vi.mock('../components/Collections/CollectionFilterBar', () => ({
? <button type="button" onClick={props.onAddPlace}>collections.addPlace</button>
: <div />,
}))
vi.mock('../components/Collections/CollectionMapPanel', () => ({ default: () => <div data-testid="map" /> }))
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 <div data-testid="map" />
},
}))
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(<CollectionsPage />)
// 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)
})
})
+9 -5
View File
@@ -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)