diff --git a/client/src/pages/SharedTripPage.test.tsx b/client/src/pages/SharedTripPage.test.tsx index 8a96ca8cb..5fd60f6f7 100644 --- a/client/src/pages/SharedTripPage.test.tsx +++ b/client/src/pages/SharedTripPage.test.tsx @@ -7,6 +7,7 @@ import { resetAllStores, seedStore } from '../../tests/helpers/store'; import { buildSettings } from '../../tests/helpers/factories'; import { useSettingsStore } from '../store/settingsStore'; import SharedTripPage from './SharedTripPage'; +import L from 'leaflet'; // Mock react-leaflet (SharedTripPage renders a map) vi.mock('react-leaflet', () => ({ @@ -15,6 +16,7 @@ vi.mock('react-leaflet', () => ({ ), TileLayer: () => null, Marker: ({ children }: { children?: React.ReactNode }) =>
{children}
, + Polyline: () =>
, Tooltip: ({ children }: { children?: React.ReactNode }) =>
{children}
, useMap: () => ({ fitBounds: vi.fn(), @@ -500,8 +502,11 @@ describe('SharedTripPage', () => { ); renderSharedTrip('test-token'); // The untitled day shows the German label "Tag 1", proving the hardcoded English - // "Day 1" was replaced by the i18n key t('dayplan.dayN'). - await waitFor(() => expect(screen.getByText('Tag 1')).toBeInTheDocument()); + // "Day 1" was replaced by the i18n key t('dayplan.dayN'). It appears twice since + // the day picker above the map was added (#1962): once as a chip, once on the card. + await waitFor(() => expect(screen.getAllByText('Tag 1')).toHaveLength(2)); + const labels = screen.getAllByText('Tag 1'); + expect(labels.filter((el) => el.closest('button'))).toHaveLength(1); }); }); @@ -701,7 +706,10 @@ describe('SharedTripPage', () => { accommodations: [{ id: 3, place_name: 'Hotel Lutetia', start_day_id: 5, end_day_id: 5 }], })); - expect(screen.getByText('Day 1')).toBeInTheDocument(); + // Twice since the day picker landed (#1962): the chip above the map and the card. + const dayLabels = screen.getAllByText('Day 1'); + expect(dayLabels).toHaveLength(2); + expect(dayLabels.filter((el) => el.closest('button'))).toHaveLength(1); expect(screen.getByText('Hotel Lutetia')).toBeInTheDocument(); expect(screen.getByText('0 places')).toBeInTheDocument(); // No date row for an undated day. @@ -785,6 +793,158 @@ describe('SharedTripPage', () => { }); }); + // ── Day order, route line and the day picker at the map (#1962) ──────────── + describe('FE-PAGE-SHARED-038: day markers carry their order and a connecting line', () => { + const day = { id: 7, trip_id: 1, day_number: 1, date: '2026-07-02', title: 'Day One' }; + const louvre = { id: 201, name: 'Louvre', lat: 48.86, lng: 2.33, category: { id: 1, name: 'Sight', color: '#ff0000', icon: 'landmark' } }; + const orsay = { id: 202, name: 'Orsay', lat: 48.85, lng: 2.32, category: null }; + const notre = { id: 203, name: 'Notre-Dame', lat: 48.853, lng: 2.35, category: null }; + + // Every divIcon call the last render produced, as raw html strings. + const iconHtml = () => (L.divIcon as unknown as ReturnType).mock.calls.map((c: any[]) => String(c[0].html)); + + beforeEach(() => (L.divIcon as unknown as ReturnType).mockClear()); + + it('numbers the stops by order_index, not by payload order', async () => { + await open('order-map-token', payload({ + days: [day], + places: [louvre, orsay, notre], + assignments: { + '7': [ + { id: 302, day_id: 7, place_id: 202, order_index: 1, place: orsay }, + { id: 303, day_id: 7, place_id: 203, order_index: 2, place: notre }, + { id: 301, day_id: 7, place_id: 201, order_index: 0, place: louvre }, + ], + }, + })); + + fireEvent.click(screen.getByRole('button', { name: 'Day 1' })); + await waitFor(() => expect(iconHtml().some((h: string) => h.includes('>1<'))).toBe(true)); + + const html = iconHtml(); + // Louvre is order_index 0 and therefore 1, regardless of where it sat in the payload. + expect(html.filter((h: string) => h.includes('>1<'))).toHaveLength(1); + expect(html.filter((h: string) => h.includes('>2<'))).toHaveLength(1); + expect(html.filter((h: string) => h.includes('>3<'))).toHaveLength(1); + }); + + it('shows both positions once for a stop the day visits twice, and renders it once', async () => { + await open('twice-token', payload({ + days: [day], + places: [louvre, orsay], + assignments: { + '7': [ + { id: 301, day_id: 7, place_id: 201, order_index: 0, place: louvre }, + { id: 302, day_id: 7, place_id: 202, order_index: 1, place: orsay }, + { id: 303, day_id: 7, place_id: 201, order_index: 2, place: louvre }, + ], + }, + })); + + fireEvent.click(screen.getByRole('button', { name: 'Day 1' })); + await waitFor(() => expect(iconHtml().some((h: string) => h.includes('1 \u00b7 3'))).toBe(true)); + // One marker for the repeated place, not two with the same React key. + expect(iconHtml().filter((h: string) => h.includes('1 \u00b7 3'))).toHaveLength(1); + }); + + it('leaves the trip-wide pool unnumbered, since it has no order to show', async () => { + await open('nonum-token', payload({ + days: [day], + places: [louvre, orsay], + assignments: { '7': [{ id: 301, day_id: 7, place_id: 201, order_index: 0, place: louvre }] }, + })); + + await waitFor(() => expect(iconHtml().length).toBeGreaterThan(0)); + expect(iconHtml().some((h: string) => h.includes('border-radius:8px'))).toBe(false); + }); + + it('draws the connecting line only for a day with more than one stop', async () => { + await open('line-token', payload({ + days: [day], + places: [louvre, orsay], + assignments: { + '7': [ + { id: 301, day_id: 7, place_id: 201, order_index: 0, place: louvre }, + { id: 302, day_id: 7, place_id: 202, order_index: 1, place: orsay }, + ], + }, + })); + + // No day selected: no line, even though the trip has two geocoded places. + expect(screen.queryByTestId('route-line')).toBeNull(); + + fireEvent.click(screen.getByRole('button', { name: 'Day 1' })); + await waitFor(() => expect(screen.getByTestId('route-line')).toBeInTheDocument()); + + fireEvent.click(screen.getByRole('button', { name: 'All' })); + await waitFor(() => expect(screen.queryByTestId('route-line')).toBeNull()); + }); + + it('does not draw a line for a day with a single stop', async () => { + await open('single-token', payload({ + days: [day], + places: [louvre], + assignments: { '7': [{ id: 301, day_id: 7, place_id: 201, order_index: 0, place: louvre }] }, + })); + + fireEvent.click(screen.getByRole('button', { name: 'Day 1' })); + await waitFor(() => expect(screen.getAllByText('Louvre').length).toBeGreaterThan(1)); + expect(screen.queryByTestId('route-line')).toBeNull(); + }); + }); + + describe('FE-PAGE-SHARED-039: the day picker sits at the map and agrees with the day card', () => { + const day = { id: 7, trip_id: 1, day_number: 1, date: '2026-07-02', title: 'Day One' }; + const louvre = { id: 201, name: 'Louvre', lat: 48.86, lng: 2.33, category: null }; + const orsay = { id: 202, name: 'Orsay', lat: 48.85, lng: 2.32, category: null }; + + it('narrows the markers to one day and back again', async () => { + await open('picker-token', payload({ + days: [day], + places: [louvre, orsay], + assignments: { '7': [{ id: 301, day_id: 7, place_id: 201, order_index: 0, place: louvre }] }, + })); + + // All: both geocoded places are on the map, so Orsay's tooltip is present. + expect(screen.getByText('Orsay')).toBeInTheDocument(); + + fireEvent.click(screen.getByRole('button', { name: 'Day 1' })); + await waitFor(() => expect(screen.queryByText('Orsay')).toBeNull()); + + fireEvent.click(screen.getByRole('button', { name: 'All' })); + await waitFor(() => expect(screen.getByText('Orsay')).toBeInTheDocument()); + }); + + it('marks the active chip for assistive tech', async () => { + await open('pressed-token', payload({ days: [day], places: [louvre], assignments: {} })); + + expect(screen.getByRole('button', { name: 'All' })).toHaveAttribute('aria-pressed', 'true'); + fireEvent.click(screen.getByRole('button', { name: 'Day 1' })); + await waitFor(() => + expect(screen.getByRole('button', { name: 'Day 1' })).toHaveAttribute('aria-pressed', 'true'), + ); + expect(screen.getByRole('button', { name: 'All' })).toHaveAttribute('aria-pressed', 'false'); + }); + }); + + describe('FE-PAGE-SHARED-040: trip-wide markers keep their category colour', () => { + // The payload nests the category on a day's assignments but sends it flat on the + // trip-wide pool, so reading only the nested shape painted every marker indigo. + it('reads the flat category_color the trip pool sends', async () => { + (L.divIcon as unknown as ReturnType).mockClear(); + await open('flatcat-token', payload({ + days: [], + places: [{ id: 201, name: 'Louvre', lat: 48.86, lng: 2.33, category_color: '#ff8800', category_icon: 'landmark' }], + assignments: {}, + })); + + await waitFor(() => expect((L.divIcon as unknown as ReturnType).mock.calls.length).toBeGreaterThan(0)); + const html = (L.divIcon as unknown as ReturnType).mock.calls.map((c: any[]) => String(c[0].html)); + expect(html.some((h: string) => h.includes('#ff8800'))).toBe(true); + expect(html.some((h: string) => h.includes('#6366f1'))).toBe(false); + }); + }); + describe('FE-PAGE-SHARED-026: expanded day renders notes and single-leg transports', () => { const day = { id: 9, trip_id: 1, day_number: 1, date: '2026-07-02', title: 'Day One' }; diff --git a/client/src/pages/SharedTripPage.tsx b/client/src/pages/SharedTripPage.tsx index 153a0a754..6c8cbf235 100644 --- a/client/src/pages/SharedTripPage.tsx +++ b/client/src/pages/SharedTripPage.tsx @@ -17,7 +17,7 @@ import { } from 'lucide-react'; import { createElement, useEffect, useRef } from 'react'; import { renderIconMarkup } from '../utils/iconMarkup'; -import { MapContainer, Marker, TileLayer, Tooltip, useMap } from 'react-leaflet'; +import { MapContainer, Marker, Polyline, TileLayer, Tooltip, useMap } from 'react-leaflet'; import { getCategoryIcon } from '../components/shared/categoryIcons'; import { DEFAULT_MAP_CENTER, DEFAULT_MAP_ZOOM } from '../constants/mapDefaults'; import { SUPPORTED_LANGUAGES, useTranslation } from '../i18n'; @@ -33,24 +33,41 @@ import { useSharedTrip } from './sharedTrip/useSharedTrip'; const TRANSPORT_ICONS = { flight: Plane, train: Train, bus: Bus, car: Car, cruise: Ship }; -function createMarkerIcon(place: any) { +// Injected into Leaflet's marker HTML, where CSS variables cannot reach - the same +// reason MapView.tsx is exempt from theme:lint outright. +const ORDER_BADGE_STYLE = 'position:absolute;bottom:-4px;right:-4px;min-width:16px;height:16px;border-radius:8px;padding:0 3px;background:rgba(255,255,255,0.94);border:1.5px solid rgba(0,0,0,0.15);box-shadow:0 1px 4px rgba(0,0,0,0.18);display:flex;align-items:center;justify-content:center;font-weight:800;color:#111827;line-height:1;box-sizing:border-box;white-space:nowrap;'; // theme-lint-disable + +function createMarkerIcon(place: any, orderNumbers?: number[] | null) { const cat = place.category; // This page answers without a guard, so an unescaped colour here reaches // people who have no account on the instance at all. - const color = safeHexColor(cat?.color, '#6366f1'); - const CatIcon = getCategoryIcon(cat?.icon); + // The payload carries the category in two shapes: nested on a day's assignments, + // flat on the trip-wide pool. Reading only the nested one dropped every marker + // outside a day selection to the placeholder colour. + const color = safeHexColor(cat?.color ?? place.category_color, '#6366f1'); + const CatIcon = getCategoryIcon(cat?.icon ?? place.category_icon); const iconSvg = renderIconMarkup(createElement(CatIcon, { size: 14, strokeWidth: 2, color: 'white' })); + // Position in the day's order, same contract as the planner: a stop the day + // visits twice shows both, e.g. "1 . 3". The values are array indices, never payload. + const badge = orderNumbers?.length + ? `${orderNumbers.join(' \u00b7 ')}` + : ''; return L.divIcon({ className: '', iconSize: [28, 28], iconAnchor: [14, 14], - html: `
${iconSvg}
`, + html: `
${iconSvg}${badge}
`, }); } function FitBoundsToPlaces({ places, framedOnMount }: { places: any[]; framedOnMount: boolean }) { const map = useMap(); const fitRan = useRef(false); + // The page rebuilds this array on every render (a Page body may not memoise), so + // keying the effect on the coordinates rather than the array identity is what stops + // an unrelated re-render - the language picker, a late FX response - from throwing + // the viewer's pan and zoom away. + const fitKey = places.map((p) => `${p.lat},${p.lng}`).join('|'); useEffect(() => { if (places.length === 0) return; // The map already opened framed on these places; fitting again would only re-do it. @@ -62,7 +79,7 @@ function FitBoundsToPlaces({ places, framedOnMount }: { places: any[]; framedOnM fitRan.current = true; const bounds = L.latLngBounds(places.map((p) => [p.lat, p.lng])); map.fitBounds(bounds, { padding: [40, 40], maxZoom: 14 }); - }, [places, map]); // eslint-disable-line react-hooks/exhaustive-deps + }, [fitKey, map]); // eslint-disable-line react-hooks/exhaustive-deps return null; } @@ -136,10 +153,27 @@ export default function SharedTripPage() { } = data; const sortedDays = [...(days || [])].sort((a: any, b: any) => a.day_number - b.day_number); - // Map places - const mapPlaces = selectedDay - ? (assignments[String(selectedDay)] || []).map((a: any) => a.place).filter((p: any) => p?.lat && p?.lng) - : (places || []).filter((p: any) => p?.lat && p?.lng); + // Map places. In day mode each stop carries its position in the day's order; the + // trip-wide pool has none (it arrives by created_at). The index runs over the full + // sorted assignment list, like the planner does, so a stop without coordinates still + // consumes a number and the app and the share link agree on what "3" means. + const dayAssignments = selectedDay + ? [...(assignments[String(selectedDay)] || [])].sort((a: any, b: any) => a.order_index - b.order_index) + : []; + const dayOrderMap: Record = {}; + dayAssignments.forEach((a: any, i: number) => { + if (!a.place?.id) return; + (dayOrderMap[a.place.id] ||= []).push(i + 1); + }); + const dayPlaces: any[] = []; + const seenPlaceIds = new Set(); + for (const a of dayAssignments as any[]) { + const p = a.place; + if (!p?.lat || !p?.lng || seenPlaceIds.has(p.id)) continue; + seenPlaceIds.add(p.id); + dayPlaces.push(p); + } + const mapPlaces = selectedDay ? dayPlaces : (places || []).filter((p: any) => p?.lat && p?.lng); // Open framed on the trip's places instead of on Paris. MapContainer only reads center/zoom // at mount, so recomputing this per render is free — and the fit below takes over from there. @@ -396,6 +430,41 @@ export default function SharedTripPage() { {/* Map */} {activeTab === 'plan' && ( <> + {/* Day picker at the map. Same setter as the day card below, so the map and + the expanded day can never disagree. Without it the only way to narrow the + map down was a control 300px further down the page. */} + {sortedDays.length > 0 && ( +
+ {[null, ...sortedDays.map((d: any) => d.id)].map((id: number | null, i: number) => { + const active = selectedDay === id; + return ( + + ); + })} +
+ )}
+ {selectedDay && mapPlaces.length > 1 && ( + [p.lat, p.lng])} + // Dashed and straight on purpose: it shows the order of the day's stops, + // not the roads between them. A real route would mean sending the + // itinerary to a third party for every anonymous visitor of a shared + // link, with no way for the trip's owner to opt out. + pathOptions={{ color: '#0a84ff', weight: 3, opacity: 0.8, dashArray: '6 8', lineCap: 'round' }} // theme-lint-disable + interactive={false} + /> + )} {mapPlaces.map((p: any) => ( - + {p.name} ))}