From 43a8984e4c9e2a9ab4810a936c4ffbd31af433f4 Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 13 Nov 2026 10:48:59 +0100 Subject: [PATCH] feat(planner): a calmer place inspector footer, and a folded place opens in full again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The footer wrapped once a place had a day, a website and a collection. Now only adding to the day, navigation and edit carry words, website and collection are icons, and removing from the day and delete sit behind one "…". The words drop out when the panel is narrow (a tablet) instead of the row wrapping. Folding is kept for the place on show only (#3039). --- .../Planner/PlaceInspector.test.tsx | 59 ++++++++----- .../src/components/Planner/PlaceInspector.tsx | 86 +++++++++++-------- 2 files changed, 84 insertions(+), 61 deletions(-) diff --git a/client/src/components/Planner/PlaceInspector.test.tsx b/client/src/components/Planner/PlaceInspector.test.tsx index 31252640b..42a31b527 100644 --- a/client/src/components/Planner/PlaceInspector.test.tsx +++ b/client/src/components/Planner/PlaceInspector.test.tsx @@ -108,6 +108,14 @@ beforeEach(() => { // ── Tests ───────────────────────────────────────────────────────────────────── + +/** Opens the footer's "…" and returns the menu with the rare and destructive actions. */ +async function openMoreMenu() { + const footer = document.querySelector('footer') as HTMLElement; + fireEvent.click(within(footer).getByRole('button', { name: 'More options' })); + return await screen.findByRole('menu'); +} + describe('PlaceInspector', () => { // ── Rendering ────────────────────────────────────────────────────────────── @@ -211,11 +219,9 @@ describe('PlaceInspector', () => { const user = userEvent.setup(); const onDelete = vi.fn(); render(); - const allButtons = screen.getAllByRole('button'); - // The square delete button sits just before Edit, named by its tooltip. - const deleteBtn = screen.getByRole('button', { name: 'Delete' }); - expect(allButtons[allButtons.length - 2]).toBe(deleteBtn); - await user.click(deleteBtn); + // Delete waits in the footer's "…", beside Edit. + const menu = await openMoreMenu(); + await user.click(within(menu).getByRole('button', { name: 'Delete' })); expect(onDelete).toHaveBeenCalled(); }); @@ -270,9 +276,9 @@ describe('PlaceInspector', () => { onRemoveAssignment={onRemoveAssignment} /> ); - // Find the remove button — it carries the "Remove from Day" label - const removeBtn = screen.getByText('Remove from Day').closest('button')!; - await user.click(removeBtn); + // Taking it off the day waits in the footer's "…". + const menu = await openMoreMenu(); + await user.click(within(menu).getByRole('button', { name: 'Remove from Day' })); // Component calls onRemoveAssignment(selectedDayId, assignmentInDay.id) expect(onRemoveAssignment).toHaveBeenCalledWith(1, 99); }); @@ -1205,18 +1211,20 @@ describe('PlaceInspector', () => { await user.click(screen.getAllByRole('button').find(b => b.textContent?.includes('Navigation'))!); await user.click(await screen.findByRole('menuitem', { name: 'OpenStreetMap' })); - fireEvent.click(screen.getByText('Open Website').closest('button')!); + fireEvent.click(screen.getByRole('button', { name: 'Open Website' })); expect(open).toHaveBeenCalledTimes(2); expect(open.mock.calls[1]).toEqual(['https://example.org', '_blank', 'noopener,noreferrer']); vi.unstubAllGlobals(); }); - it('FE-PLANNER-INSPECTOR-070: the footer keeps the ways out on the left and delete + edit on the right', () => { + it('FE-PLANNER-INSPECTOR-070: the footer keeps the ways out on the left and the "…" + edit on the right', () => { const p = buildPlace({ id: 712, name: 'Footer Place', lat: 48.1, lng: 2.1, website: 'https://example.org' }); render(); const footer = document.querySelector('footer') as HTMLElement; const names = within(footer).getAllByRole('button').map(b => b.getAttribute('aria-label') || b.textContent); - expect(names).toEqual(['Add to Day', 'Navigation', 'Open Website', 'Delete', 'Edit']); + expect(names).toEqual(['Add to Day', 'Navigation', 'Open Website', 'More options', 'Edit']); + // Only the day action, navigation and edit carry words; the website is an icon named by its tooltip. + expect(within(footer).getByRole('button', { name: 'Open Website' })).toHaveTextContent(''); // Hover looks are classes on the tokens now, not inline styles swapped by handlers. const edit = within(footer).getByRole('button', { name: 'Edit' }); expect(edit.getAttribute('style') ?? '').not.toMatch(/background/); @@ -1349,7 +1357,7 @@ describe('PlaceInspector', () => { http.get('/api/addons/collections/membership', () => HttpResponse.json({ saved: true, collections: [] })), ); render(); - expect(await screen.findByText('Saved')).toBeTruthy(); + expect(await screen.findByRole('button', { name: 'Saved' })).toBeTruthy(); }); it('FE-PLANNER-INSPECTOR-082: clicking save hands the whole place to the collection picker', async () => { @@ -1358,7 +1366,7 @@ describe('PlaceInspector', () => { http.get('/api/addons/collections/membership', () => HttpResponse.json({ saved: false, collections: [] })), ); render(); - fireEvent.click((await screen.findByText('Save to Collection')).closest('button')!); + fireEvent.click(await screen.findByRole('button', { name: 'Save to Collection' })); expect(useSaveToCollectionStore.getState().target).toMatchObject({ name: 'Eiffel Tower', source_place_id: place.id, lat: 48.8584, lng: 2.2945, }); @@ -1370,7 +1378,7 @@ describe('PlaceInspector', () => { http.get('/api/addons/collections/membership', () => new HttpResponse(null, { status: 500 })), ); render(); - expect(await screen.findByText('Save to Collection')).toBeTruthy(); + expect(await screen.findByRole('button', { name: 'Save to Collection' })).toBeTruthy(); }); // ── Plugin contributions (#1429) ───────────────────────────────────────────── @@ -1647,7 +1655,7 @@ describe('PlaceInspector', () => { expect(screen.getByText('Museum Ticket').closest('[role="button"]')).toHaveAttribute('data-no-press'); }); - it('FE-PLANNER-INSPECTOR-102: a member without place_edit gets neither Edit nor Delete nor the inline rename (#2446)', () => { + it('FE-PLANNER-INSPECTOR-102: a member without place_edit gets neither Edit nor Delete nor the inline rename (#2446)', async () => { // buildUser() is a plain member and buildTrip({ id: 1 }) belongs to somebody else. usePermissionsStore.setState({ permissions: { place_edit: 'trip_owner' } }); const onEdit = vi.fn(); @@ -1655,7 +1663,8 @@ describe('PlaceInspector', () => { const onUpdatePlace = vi.fn(); const member = render(); expect(screen.queryByText('Edit')).not.toBeInTheDocument(); - expect(screen.queryByRole('button', { name: 'Delete' })).not.toBeInTheDocument(); + // With nothing to offer, the "…" is not there either. + expect(screen.queryByRole('button', { name: 'More options' })).not.toBeInTheDocument(); fireEvent.doubleClick(screen.getByText(place.name)); expect(screen.queryByRole('textbox')).not.toBeInTheDocument(); member.unmount(); @@ -1663,11 +1672,11 @@ describe('PlaceInspector', () => { seedStore(useTripStore, { trip: buildTrip({ id: 1, user_id: useAuthStore.getState().user!.id }) }); render(); expect(screen.getByText('Edit')).toBeInTheDocument(); - expect(screen.getByRole('button', { name: 'Delete' })).toBeInTheDocument(); + expect(within(await openMoreMenu()).getByRole('button', { name: 'Delete' })).toBeInTheDocument(); }); - it('FE-PLANNER-INSPECTOR-017b: a stop a booked night wrote is neither removed nor doubled here', () => { + it('FE-PLANNER-INSPECTOR-017b: a stop a booked night wrote is neither removed nor doubled here', async () => { // Taking it off the day would leave the booking behind with nothing on the drive and // no way back short of saving it again; offering to add it would put a second copy // of the same hotel beside it. The night is removed where it is made: the day's @@ -1683,7 +1692,7 @@ describe('PlaceInspector', () => { /> ); - expect(screen.queryByRole('button', { name: /remove from day/i })).toBeNull(); + expect(within(await openMoreMenu()).queryByRole('button', { name: /remove from day/i })).toBeNull(); expect(screen.queryByRole('button', { name: /add to day/i })).toBeNull(); }); @@ -1699,7 +1708,7 @@ describe('PlaceInspector', () => { expect(screen.queryByText('Remove from Day')).toBeNull(); }); - it('FE-PLANNER-INSPECTOR-017d: a stop the traveller placed at the booked hotel can still be removed', () => { + it('FE-PLANNER-INSPECTOR-017d: a stop the traveller placed at the booked hotel can still be removed', async () => { // A second copy that did get onto the day is the traveller's own row and shows // in the list. That one keeps its "Remove from day", which is the way back. const onRemoveAssignment = vi.fn(); @@ -1715,12 +1724,12 @@ describe('PlaceInspector', () => { /> ); - fireEvent.click(screen.getByText('Remove from Day').closest('button')!); + fireEvent.click(within(await openMoreMenu()).getByRole('button', { name: 'Remove from Day' })); expect(onRemoveAssignment).toHaveBeenCalledWith(1, 100); expect(screen.queryByText('Add to Day')).toBeNull(); }); - it('FE-PLANNER-INSPECTOR-017e: any booked stop at the place keeps the buttons off, whichever row the store lists first', () => { + it('FE-PLANNER-INSPECTOR-017e: any booked stop at the place keeps the buttons off, whichever row the store lists first', async () => { // With the service stops switched off the day list can hide a hotel the traveller // placed as well, so the store may list that row ahead of the booked one. The // question is whether a booking holds this place on this day, not which row comes first. @@ -1730,7 +1739,7 @@ describe('PlaceInspector', () => { render(); expect(screen.queryByText('Add to Day')).toBeNull(); - expect(screen.queryByText('Remove from Day')).toBeNull(); + expect(within(await openMoreMenu()).queryByRole('button', { name: 'Remove from Day' })).toBeNull(); }); // ── Head band (booking-detail language) ────────────────────────────────────── @@ -1867,6 +1876,10 @@ describe('PlaceInspector folded to its head (#3039)', () => { expect(screen.getByText('Rue de Rivoli, Paris')).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Collapse' })).toHaveAttribute('aria-expanded', 'true'); + // Back on the first place it is open in full too: the fold went with the selection. + rerender(); + expect(screen.getByRole('button', { name: 'Collapse' })).toHaveAttribute('aria-expanded', 'true'); + rerender(); expect(screen.queryByRole('button', { name: 'Collapse' })).toBeNull(); }); diff --git a/client/src/components/Planner/PlaceInspector.tsx b/client/src/components/Planner/PlaceInspector.tsx index ec8e340e6..7d49f4aa9 100644 --- a/client/src/components/Planner/PlaceInspector.tsx +++ b/client/src/components/Planner/PlaceInspector.tsx @@ -6,7 +6,7 @@ import Markdown from 'react-markdown' import remarkGfm from 'remark-gfm' import remarkBreaks from 'remark-breaks' import { markdownLinkComponents } from '../shared/markdownLink' -import { Clock, MapPin, ExternalLink, Pencil, Plus, Minus, ChevronDown, ChevronUp, ChevronRight, FileText, Upload, File, FileImage, Navigation, Mountain, Bookmark, BookmarkCheck, Copy } from 'lucide-react' +import { Clock, MapPin, ExternalLink, Pencil, Plus, Minus, ChevronDown, ChevronUp, ChevronRight, FileText, Upload, File, FileImage, Navigation, Mountain, Bookmark, BookmarkCheck, Copy, Trash2 } from 'lucide-react' import { hoursLines, periodsFromWeek } from './placeHours' import { BlurredCode } from '../shared/BookingCode' import PlaceRating from '../shared/StarRating' @@ -40,10 +40,10 @@ import { resolveOpenNow, resolvePlaceTimeZone, getWeekdayIndex, type OpeningPeri import { convertHoursLine } from './placeHoursFormat' import type { EndDayControlProps } from '../Roadtrip/EndDayControl' import VisitControls, { type RoadtripStayControl } from '../Roadtrip/VisitControls' -import { DialogButton, DialogSection, DeleteButton, FooterSpacer, fs } from '../shared/DialogShell' +import { DialogButton, DialogIconButton, DialogSection, DeleteButton, FooterSpacer, fs } from '../shared/DialogShell' import { BOX, Field, TypeTile, toneOf, toneTint, useOpenFile } from './bookings/bookingParts' import { parseMeta } from './bookings/bookingsModel' -import { SoftPill } from './planParts' +import { MoreButton, SoftPill } from './planParts' import { InspectorHead } from './PlaceInspectorHead' import { usePlaceLanguage } from '../../hooks/usePlaceLanguage' import { participantsWith, participantsWithout, splitParticipants, usePlaceFileUpload } from './usePlaceActions' @@ -252,8 +252,14 @@ export default function PlaceInspector({ const saveVersion = useSaveToCollectionStore(s => s.version) const [savedInCollection, setSavedInCollection] = useState(false) const [hoursExpanded, setHoursExpanded] = useState(false) - // Folded by place, so the next place selected opens in full. - const [collapsedPlaceId, setCollapsedPlaceId] = useState(null) + // Folded for the place on show only: any other place selected, coming back + // included, opens in full. + const [collapsed, setCollapsed] = useState(false) + const [foldedFor, setFoldedFor] = useState(place.id) + if (foldedFor !== place.id) { + setFoldedFor(place.id) + setCollapsed(false) + } const [editingName, setEditingName] = useState(false) const [nameValue, setNameValue] = useState('') const nameInputRef = useRef(null) @@ -362,7 +368,6 @@ export default function PlaceInspector({ const description = place.description || googleDetails?.summary const showRating = (mode === 'trip' && !!onRate) || !!shortReview const mapCard = centredMapCard(leftWidth, rightWidth, { clearPill: clearMapPill }) - const collapsed = collapsedPlaceId === place.id return (
} onUpdatePlace={onUpdatePlace} collapsed={collapsed} - onToggleCollapsed={collapsible ? () => setCollapsedPlaceId(collapsed ? null : place.id) : undefined} + onToggleCollapsed={collapsible ? () => setCollapsed(c => !c) : undefined} onClose={onClose} /> {/* The body scrolls inside the capped card; each block keeps its natural height (#1195). */} @@ -475,8 +480,10 @@ export default function PlaceInspector({ )}
- {/* Footer: the day action and the ways out on the left, delete and edit on the right. */} -