From 3c575a06f470a842cd577f4d4e72f8576d317a95 Mon Sep 17 00:00:00 2001 From: Maurice Date: Mon, 27 Jul 2026 17:35:54 +0200 Subject: [PATCH] Add the track colour picker to the place inspector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New shared component rather than a ninth copy of the swatch grid that is already inline in eight places. Selection is drawn with var(--accent) so it follows whatever accent the user runs. The custom-colour cell listens for the native change event instead of React's onChange: for a colour input React maps onChange to input, which fires on every drag inside the OS picker — dozens of writes, each one a 409 candidate against the base-version header. The auto cell previews the colour you would get back, not the one currently set, which is a different value as soon as anything is picked. The row sits next to the track stats block, not inside it: that block returns null on unparsable geometry and the colour control has no business disappearing with it. The elevation chart and the distance pin follow the track colour too, otherwise the panel and the map read as two unrelated objects. In the places list the track icon becomes a stroke in the colour the line is drawn in. The map has no legend of its own, so that stroke is the only thing tying a line back to a row. Undo after deleting a track now restores its geometry and colour instead of bringing back a bare point. --- .../Planner/PlaceInspector.test.tsx | 31 ++++++ .../src/components/Planner/PlaceInspector.tsx | 63 +++++++++-- .../components/Planner/PlacesSidebar.test.tsx | 20 ++++ .../components/Planner/PlacesSidebarRow.tsx | 13 ++- .../shared/TrackColorPicker.test.tsx | 56 ++++++++++ .../components/shared/TrackColorPicker.tsx | 104 ++++++++++++++++++ .../src/pages/tripPlanner/useTripPlanner.ts | 4 + 7 files changed, 282 insertions(+), 9 deletions(-) create mode 100644 client/src/components/shared/TrackColorPicker.test.tsx create mode 100644 client/src/components/shared/TrackColorPicker.tsx diff --git a/client/src/components/Planner/PlaceInspector.test.tsx b/client/src/components/Planner/PlaceInspector.test.tsx index 26b32575e..c5bccd3a5 100644 --- a/client/src/components/Planner/PlaceInspector.test.tsx +++ b/client/src/components/Planner/PlaceInspector.test.tsx @@ -732,4 +732,35 @@ describe('PlaceInspector', () => { expect(screen.queryByRole('button', { name: 'Change image' })).toBeNull(); }); +// ── Track colour (#776) ────────────────────────────────────────────────────── + + it('FE-PLANNER-INSPECTOR-051: the colour row only exists for a place with geometry', () => { + const { rerender } = render(); + expect(screen.queryByText('Track color')).toBeNull(); + + rerender(); + expect(screen.getAllByText('Track color').length).toBeGreaterThan(0); + }); + + it('FE-PLANNER-INSPECTOR-052: picking a swatch saves that colour', () => { + const onUpdatePlace = vi.fn(); + const track = { ...place, route_geometry: '[[48.0,2.0],[49.0,3.0]]' }; + render(); + + fireEvent.click(screen.getAllByText('Track color')[0]); + fireEvent.click(screen.getByRole('button', { name: '#059669' })); + expect(onUpdatePlace).toHaveBeenCalledWith(track.id, { route_color: '#059669' }); + }); + + it('FE-PLANNER-INSPECTOR-053: the auto cell saves null, not undefined', () => { + const onUpdatePlace = vi.fn(); + const track = { ...place, route_geometry: '[[48.0,2.0],[49.0,3.0]]', route_color: '#059669' }; + render(); + + fireEvent.click(screen.getAllByText('Track color')[0]); + fireEvent.click(screen.getByRole('button', { name: 'Automatic color' })); + // null is what clears the column; undefined would leave the colour in place. + expect(onUpdatePlace).toHaveBeenCalledWith(track.id, { route_color: null }); + }); + }); diff --git a/client/src/components/Planner/PlaceInspector.tsx b/client/src/components/Planner/PlaceInspector.tsx index eab584625..191601a42 100644 --- a/client/src/components/Planner/PlaceInspector.tsx +++ b/client/src/components/Planner/PlaceInspector.tsx @@ -4,10 +4,12 @@ import { openFile } from '../../utils/fileDownload' import Markdown from 'react-markdown' import remarkGfm from 'remark-gfm' import remarkBreaks from 'remark-breaks' -import { X, Clock, MapPin, ExternalLink, Phone, Banknote, Edit2, Trash2, Plus, Minus, ChevronDown, ChevronUp, FileText, Upload, File, FileImage, Star, Navigation, Map as MapIcon, Users, Mountain, TrendingUp, Bookmark, BookmarkCheck, Copy } from 'lucide-react' +import { X, Clock, MapPin, ExternalLink, Phone, Banknote, Edit2, Trash2, Plus, Minus, ChevronDown, ChevronUp, FileText, Upload, File, FileImage, Star, Navigation, Map as MapIcon, Users, Mountain, TrendingUp, Bookmark, BookmarkCheck, Copy, Route } from 'lucide-react' import PlaceAvatar from '../shared/PlaceAvatar' import PlaceAvatarUpload from '../shared/PlaceAvatarUpload' import PlaceRating from '../shared/StarRating' +import TrackColorPicker from '../shared/TrackColorPicker' +import { resolveTrackColor, inheritedTrackColor } from '../Map/trackColors' import GuestBadge from '../shared/GuestBadge' import StatusBadge from '../Collections/StatusBadge' import { mapsApi, pluginsApi } from '../../api/client' @@ -392,7 +394,7 @@ export default function PlaceInspector({ setHoursExpanded={setHoursExpanded} timeFormat={timeFormat} t={t} place={place} placeFiles={placeFiles} onFileUpload={onFileUpload} filesExpanded={filesExpanded} setFilesExpanded={setFilesExpanded} fileInputRef={fileInputRef} handleFileUpload={handleFileUpload} isUploading={isUploading} - distanceUnit={distanceUnit} /> + distanceUnit={distanceUnit} onUpdatePlace={onUpdatePlace} /> {/* Extra native rows from placeDetailProvider plugins (#1429). */} {mode === 'trip' && providerDetails.length > 0 && ( @@ -856,8 +858,51 @@ function PlaceReservationParticipants({ selectedAssignmentId, reservations, assi ) } +/** + * Track colour row (#776) — only for places that carry GPX geometry. Sits next + * to the stats block rather than inside it: that block bails out on unparsable + * geometry, and the colour control has no business disappearing with it. + */ +function TrackColorRow({ place, trackColor, onUpdatePlace, t }: any) { + const [open, setOpen] = useState(false) + return ( +
+ + {open && ( +
+ onUpdatePlace?.(place.id, { route_color: color })} + /> +
+ )} +
+ ) +} + function PlaceExtras({ openingHours, weekdayIndex, hoursExpanded, setHoursExpanded, timeFormat, t, place, - placeFiles, onFileUpload, filesExpanded, setFilesExpanded, fileInputRef, handleFileUpload, isUploading, distanceUnit }: any) { + placeFiles, onFileUpload, filesExpanded, setFilesExpanded, fileInputRef, handleFileUpload, isUploading, distanceUnit, + onUpdatePlace }: any) { + const trackColor = resolveTrackColor(place) return (
0 ? 'sm:grid-cols-2' : ''} gap-2`}> {openingHours && openingHours.length > 0 && ( @@ -893,6 +938,10 @@ function PlaceExtras({ openingHours, weekdayIndex, hoursExpanded, setHoursExpand )} + {place.route_geometry && ( + + )} + {/* GPX Track stats */} {place.route_geometry && (() => { try { @@ -949,7 +998,7 @@ function PlaceExtras({ openingHours, weekdayIndex, hoursExpanded, setHoursExpand
- + {formatDistance(distKm, distanceUnit)}
{hasEle && ( @@ -972,12 +1021,12 @@ function PlaceExtras({ openingHours, weekdayIndex, hoursExpanded, setHoursExpand - - + + - + )}
diff --git a/client/src/components/Planner/PlacesSidebar.test.tsx b/client/src/components/Planner/PlacesSidebar.test.tsx index 3e3118c91..0cb1d8e4a 100644 --- a/client/src/components/Planner/PlacesSidebar.test.tsx +++ b/client/src/components/Planner/PlacesSidebar.test.tsx @@ -650,3 +650,23 @@ describe('touch device at desktop width (#1432)', () => { expect(screen.getByText('Drop to import')).toBeInTheDocument(); }); }); + +// The map draws no legend of its own, so the stroke in the row is the only +// thing tying a coloured line back to a place (#776). +describe('track colour legend (#776)', () => { + it('FE-PLANNER-SIDEBAR-049: a track row carries a stroke in the colour the map draws', () => { + const track = buildPlace({ id: 11, name: 'Coloured Track', route_geometry: '[[48.0,2.0],[49.0,3.0]]', route_color: '#e11d48' }); + render(); + const row = screen.getByText('Coloured Track').closest('div[draggable]')!; + const strokes = Array.from(row.querySelectorAll('span')).filter(el => (el as HTMLElement).style.borderRadius === '999px'); + expect(strokes.some(el => (el as HTMLElement).style.background.includes('225, 29, 72') || (el as HTMLElement).style.background.includes('#e11d48'))).toBe(true); + }); + + it('FE-PLANNER-SIDEBAR-050: a place without geometry gets no stroke at all', () => { + const plain = buildPlace({ id: 12, name: 'Plain Place' }); + render(); + const row = screen.getByText('Plain Place').closest('div[draggable]')!; + const strokes = Array.from(row.querySelectorAll('span')).filter(el => (el as HTMLElement).style.borderRadius === '999px'); + expect(strokes).toHaveLength(0); + }); +}); diff --git a/client/src/components/Planner/PlacesSidebarRow.tsx b/client/src/components/Planner/PlacesSidebarRow.tsx index f047ff7d4..936c3dac0 100644 --- a/client/src/components/Planner/PlacesSidebarRow.tsx +++ b/client/src/components/Planner/PlacesSidebarRow.tsx @@ -1,7 +1,8 @@ import React from 'react' -import { Plus, Check, Route, Star } from 'lucide-react' +import { Plus, Check, Star } from 'lucide-react' import PlaceAvatar from '../shared/PlaceAvatar' import { getCategoryIcon } from '../shared/categoryIcons' +import { resolveTrackColor } from '../Map/trackColors' import type { Place, Category } from '../../types' interface MemoPlaceRowProps { @@ -81,7 +82,15 @@ export const MemoPlaceRow = React.memo(function MemoPlaceRow({
- {hasGeometry && } + {/* A stroke of the colour the track is drawn in — the map has no legend + of its own, so this is what tells you which line is this row (#776). + A line rather than another 11px icon, so it doesn't read as a second + category glyph next to the one below. */} + {hasGeometry && ( + + + + )} {cat && (() => { const CatIcon = getCategoryIcon(cat.icon) return diff --git a/client/src/components/shared/TrackColorPicker.test.tsx b/client/src/components/shared/TrackColorPicker.test.tsx new file mode 100644 index 000000000..92b2123d8 --- /dev/null +++ b/client/src/components/shared/TrackColorPicker.test.tsx @@ -0,0 +1,56 @@ +import { describe, it, expect, vi, afterEach } from 'vitest' +import { render, screen } from '../../../tests/helpers/render' +import { fireEvent } from '@testing-library/react' +import { resetAllStores } from '../../../tests/helpers/store' +import TrackColorPicker from './TrackColorPicker' + +afterEach(() => { + vi.clearAllMocks() + resetAllStores() +}) + +function setup(props: Partial> = {}) { + const onChange = vi.fn() + render() + return { onChange } +} + +describe('TrackColorPicker (#776)', () => { + it('FE-COMP-TRACKCOLOR-001: a swatch commits its colour', () => { + const { onChange } = setup() + fireEvent.click(screen.getByRole('button', { name: '#059669' })) + expect(onChange).toHaveBeenCalledWith('#059669') + }) + + it('FE-COMP-TRACKCOLOR-002: the auto cell commits null, not undefined', () => { + const { onChange } = setup({ value: '#059669' }) + fireEvent.click(screen.getByRole('button', { name: 'Automatic color' })) + expect(onChange).toHaveBeenCalledWith(null) + }) + + it('FE-COMP-TRACKCOLOR-003: the auto cell previews what clearing would give, not the current colour', () => { + setup({ value: '#e11d48' }) + const auto = screen.getByRole('button', { name: 'Automatic color' }) + // jsdom rewrites the hex to rgb(). #16a34a is the inherited colour; showing + // the picked #e11d48 here would promise the opposite of what the button does. + expect(auto.getAttribute('style')).toContain('rgb(22, 163, 74)') + expect(auto.getAttribute('style')).not.toContain('rgb(225, 29, 72)') + }) + + it('FE-COMP-TRACKCOLOR-004: dragging inside the native picker does not commit', () => { + const { onChange } = setup() + const input = document.querySelector('input[type="color"]') as HTMLInputElement + expect(input).toBeTruthy() + + // React maps onChange on a colour input to the native `input` event, which + // fires on every drag — each one would be a PUT and a 409 candidate. + input.value = '#123456' + fireEvent.input(input) + expect(onChange).not.toHaveBeenCalled() + + // The native `change` fires once, when the picker closes. + fireEvent.change(input) + expect(onChange).toHaveBeenCalledTimes(1) + expect(onChange).toHaveBeenCalledWith('#123456') + }) +}) diff --git a/client/src/components/shared/TrackColorPicker.tsx b/client/src/components/shared/TrackColorPicker.tsx new file mode 100644 index 000000000..fa7518761 --- /dev/null +++ b/client/src/components/shared/TrackColorPicker.tsx @@ -0,0 +1,104 @@ +import { useEffect, useRef } from 'react' +import { Pipette, RotateCcw } from 'lucide-react' +import { TRACK_COLORS } from '@trek/shared' +import { useTranslation } from '../../i18n' + +interface TrackColorPickerProps { + /** The picked colour, or null while the track still inherits one. */ + value: string | null + /** What the track is drawn in when nothing is picked — shown in the auto cell. */ + inheritedColor: string + onChange: (color: string | null) => void +} + +/** + * Swatch row for a GPX track's line colour (#776). + * + * First cell resets to the inherited colour, then the shared palette, then a + * native picker for anything else. Selection is drawn with `--accent` so the + * row follows whatever accent the user runs rather than pinning one hue. + */ +export default function TrackColorPicker({ value, inheritedColor, onChange }: TrackColorPickerProps) { + const { t } = useTranslation() + const customInputRef = useRef(null) + const isPreset = value !== null && (TRACK_COLORS as readonly string[]).includes(value) + const selected = 'outline outline-2 outline-[var(--accent)] outline-offset-2 scale-110' + + const onChangeRef = useRef(onChange) + onChangeRef.current = onChange + + // Native `change`, deliberately not React's onChange — see the input below. + useEffect(() => { + const input = customInputRef.current + if (!input) return + const commit = () => onChangeRef.current(input.value) + input.addEventListener('change', commit) + return () => input.removeEventListener('change', commit) + }, []) + + return ( +
+ + + {TRACK_COLORS.map(color => ( + +
+ ) +} diff --git a/client/src/pages/tripPlanner/useTripPlanner.ts b/client/src/pages/tripPlanner/useTripPlanner.ts index 485244edc..ff0a356ce 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.ts +++ b/client/src/pages/tripPlanner/useTripPlanner.ts @@ -565,6 +565,9 @@ export function useTripPlanner() { address: capturedPlace.address, category_id: capturedPlace.category_id, price: capturedPlace.price, + // An undone track has to come back as a track, not a bare point. + route_geometry: capturedPlace.route_geometry, + route_color: capturedPlace.route_color, }) for (const { dayId, orderIndex } of capturedAssignments) { await tripActions.assignPlaceToDay(tripId, dayId, newPlace.id, orderIndex) @@ -595,6 +598,7 @@ export function useTripPlanner() { name: place.name, description: place.description, lat: place.lat, lng: place.lng, address: place.address, category_id: place.category_id, price: place.price, + route_geometry: place.route_geometry, route_color: place.route_color, }) for (const a of capturedAssignments.filter(x => x.placeId === place.id)) { await tripActions.assignPlaceToDay(tripId, a.dayId, newPlace.id, a.orderIndex)