diff --git a/client/e2e/urlbar-scroll-1809.spec.ts b/client/e2e/urlbar-scroll-1809.spec.ts new file mode 100644 index 000000000..917748f76 --- /dev/null +++ b/client/e2e/urlbar-scroll-1809.spec.ts @@ -0,0 +1,105 @@ +import { test, expect, devices } from '@playwright/test' +import { dismissSystemNotices } from './helpers' + +// Phone regression guard for #1809: the dashboard must scroll the DOCUMENT. +// +// iOS Safari minimises its address bar only in reaction to the root scroller +// moving. V4 blocked that twice over: html was globally overflow:hidden and the +// mobile shell added a scroll container of its own, so the gesture never +// reached the viewport and the bar kept ~1/6th of the screen forever. The bar +// itself is not observable from page context, but the invariant it hangs off is: +// document.scrollingElement has to move. +// +// Needs WebKit (`npx playwright install webkit`). playwright.config.ts has no +// webkit project, the engine comes from this file-level device (defaultBrowserType +// 'webkit'), same as e2e/ipad-scroll-1432.spec.ts. WebKit is the point here, not +// a nicety: viewport propagation and the toolbar behaviour are Safari's. +// +// The file sorts last in the e2e run, so the trips it seeds cannot show up in +// another spec's dashboard assertions. +test.use({ ...devices['iPhone 14'] }) + +function readScroll() { + const el = document.scrollingElement as HTMLElement + return { + top: el.scrollTop, + scrollHeight: el.scrollHeight, + viewport: window.innerHeight, + htmlOverflow: getComputedStyle(document.documentElement).overflowY, + } +} + +test('#1809 iPhone: flow screens scroll the document, full-screen ones do not', async ({ page }) => { + await page.goto('/dashboard') + await dismissSystemNotices(page) + + // The context has to be the one from the report: phone width, coarse pointer. + // If either is wrong, nothing below proves anything. + const env = await page.evaluate(() => ({ + width: window.innerWidth, + coarse: window.matchMedia('(pointer: coarse)').matches, + })) + expect(env.width, 'iPhone sits below the 768px phone breakpoint').toBeLessThan(768) + expect(env.coarse, 'iPhone reports a coarse primary pointer').toBe(true) + + // Enough trips for the dashboard to overflow a phone screen. + const stamp = Date.now() + for (let i = 1; i <= 20; i++) { + const res = await page.request.post('/api/trips', { + data: { title: `URL bar ${stamp} ${i}`, start_date: '2099-06-01', end_date: '2099-06-10' }, + }) + expect(res.ok(), `seed trip ${i}`).toBeTruthy() + } + await page.reload() + await dismissSystemNotices(page) + await expect(page.getByText(`URL bar ${stamp} 1`).first()).toBeVisible({ timeout: 20_000 }) + + // 1. The root scroller is unlocked and the page is longer than the viewport. + const before = await page.evaluate(readScroll) + expect(before.htmlOverflow, 'html must not lock the viewport scroller').not.toBe('hidden') + expect(before.scrollHeight, 'dashboard is longer than one viewport').toBeGreaterThan(before.viewport) + expect(before.top).toBe(0) + + // 2. Scrolling moves the document, the signal Safari retracts its bar for. + await page.evaluate(() => window.scrollBy(0, 400)) + const scrolled = await page.evaluate(readScroll) + expect(scrolled.top, 'the document itself scrolled').toBeGreaterThan(0) + + // 3. The brand tile scrolls the page back up (it used to walk up to the shell + // container, which no longer exists). + await page.getByRole('button', { name: 'TREK' }).click() + await expect.poll(() => page.evaluate(() => document.scrollingElement!.scrollTop)).toBe(0) + + // 4. The shared body scroll lock: with the document as the scroller, an open + // sheet must freeze the page behind it and give the position back. + await page.evaluate(() => window.scrollBy(0, 300)) + const locked = await page.evaluate(readScroll) + expect(locked.top).toBeGreaterThan(0) + + await page.getByRole('button', { name: 'New Trip' }).click() + await expect(page.getByRole('dialog')).toBeVisible() + await page.evaluate(() => window.scrollBy(0, 300)) + expect(await page.evaluate(() => document.scrollingElement!.scrollTop)).toBe(locked.top) + + await page.keyboard.press('Escape') + await expect(page.getByRole('dialog')).toHaveCount(0) + expect(await page.evaluate(() => document.scrollingElement!.scrollTop)).toBe(locked.top) + await page.evaluate(() => window.scrollBy(0, 200)) + expect(await page.evaluate(() => document.scrollingElement!.scrollTop)).toBeGreaterThan(locked.top) + + // 5. Counter-probe: the map screens keep their own full-viewport layout. They + // deliberately do NOT scroll the document (and so keep the address bar), and + // the map must not collapse to zero height now that the shell has none. + await page.goto('/atlas') + await expect(page.locator('.leaflet-container')).toBeVisible({ timeout: 20_000 }) + const atlas = await page.evaluate(() => { + const map = document.querySelector('.leaflet-container') as HTMLElement + window.scrollBy(0, 400) + return { + top: document.scrollingElement!.scrollTop, + mapHeight: map.getBoundingClientRect().height, + } + }) + expect(atlas.top, 'the atlas does not move the document').toBe(0) + expect(atlas.mapHeight, 'the atlas map keeps a real height').toBeGreaterThan(200) +}) diff --git a/client/src/components/Budget/CostsPanel.helpers.test.ts b/client/src/components/Budget/CostsPanel.helpers.test.ts index ec2d15cb6..31c97c102 100644 --- a/client/src/components/Budget/CostsPanel.helpers.test.ts +++ b/client/src/components/Budget/CostsPanel.helpers.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest' -import { hasTicketSplit, payerSum, payersBalanced, readTicketItems, readUserNote, rebalancePayers, splitCents, writeTicketItems } from './CostsPanel.helpers' +import { calculateTicketShares, hasTicketSplit, payerSum, payersBalanced, readTicketItems, readUserNote, rebalancePayers, splitCents, writeTicketItems, type TicketItem } from './CostsPanel.helpers' describe('splitCents', () => { it('splits evenly when it divides cleanly', () => { @@ -132,3 +132,47 @@ describe('readUserNote', () => { expect(readUserNote(null)).toBe('') }) }) + +// ── Receipt splits have to reconcile (#1382) ───────────────────────────────── + +describe('calculateTicketShares', () => { + const line = (price: string, parts: number[]): TicketItem => + ({ id: price + parts.join(), name: 'Line', price, participants: new Set(parts) }) + const shareSum = (shares: Record) => + Object.values(shares).reduce((a, v) => a + Math.round(v * 100), 0) + + it('splits each line among the people on it', () => { + const { shares, total } = calculateTicketShares([line('10', [1, 2]), line('6', [2])]) + expect(shares).toEqual({ 1: 5, 2: 11 }) + expect(total).toBe(16) + }) + + it('spreads a line nobody was ticked for across everyone on the receipt', () => { + // The service charge used to count toward the total and toward nobody's share, + // so the expense carried a permanent difference no settle-up could clear. + const { shares, total } = calculateTicketShares([ + line('10', [1, 2]), + line('5', []), + line('x', [2]), + ]) + expect(shares).toEqual({ 1: 7.5, 2: 7.5 }) + expect(total).toBe(15) + expect(shareSum(shares)).toBe(Math.round(total * 100)) + }) + + it('hands out the remainder cents so the shares add back up to the total', () => { + const { shares, total } = calculateTicketShares([line('10', [1, 2, 3]), line('0.01', [])]) + expect(shareSum(shares)).toBe(Math.round(total * 100)) + expect(total).toBe(10.01) + }) + + it('splits nothing when the receipt has no participants anywhere', () => { + const { shares, total } = calculateTicketShares([line('10', []), line('5', [])]) + expect(shares).toEqual({}) + expect(total).toBe(15) + }) + + it('is empty for an empty receipt', () => { + expect(calculateTicketShares([])).toEqual({ shares: {}, total: 0 }) + }) +}) diff --git a/client/src/components/Budget/CostsPanel.helpers.ts b/client/src/components/Budget/CostsPanel.helpers.ts index 74e6d631e..8bc566144 100644 --- a/client/src/components/Budget/CostsPanel.helpers.ts +++ b/client/src/components/Budget/CostsPanel.helpers.ts @@ -135,24 +135,36 @@ export function readUserNote(item: { note?: string | null } | null | undefined): return note && !note.startsWith('TICKETJSON:') ? note : '' } -/** Per-person totals for a receipt split line by line, plus the receipt total. */ +/** + * Per-person totals for a receipt split line by line, plus the receipt total. + * + * Every cent on the receipt lands on somebody: a line nobody was ticked for is + * carried by everyone who appears on the receipt, so the shares add back up to + * the total. They used to count toward the total only, which left the expense + * with member debits that could never cancel the payer's credit — a permanent + * difference the settle-up view showed as debt but had no flow to clear (#1382). + * A receipt with no participants at all splits nothing; it is not saveable + * anyway, since every line needs someone on it. + */ export function calculateTicketShares(items: TicketItem[]): { shares: Record; total: number } { const shares: Record = {} let totalCents = 0 + const everyone = [...new Set(items.flatMap(i => [...i.participants]))].sort((a, b) => a - b) + for (const item of items) { const priceNum = parseFloat(item.price) || 0 const priceCents = Math.round(priceNum * 100) totalCents += priceCents - const partIds = [...item.participants] - const n = partIds.length + const sortedPartIds = item.participants.size > 0 + ? [...item.participants].sort((a, b) => a - b) + : everyone + const n = sortedPartIds.length if (n === 0) continue const baseCents = Math.floor(priceCents / n) - const remainder = priceCents % n - - const sortedPartIds = [...partIds].sort((a, b) => a - b) + const remainder = priceCents - baseCents * n for (let i = 0; i < n; i++) { const id = sortedPartIds[i] diff --git a/client/src/components/Budget/CostsPanel.test.tsx b/client/src/components/Budget/CostsPanel.test.tsx index 037d62a37..bba2289e5 100644 --- a/client/src/components/Budget/CostsPanel.test.tsx +++ b/client/src/components/Budget/CostsPanel.test.tsx @@ -1762,13 +1762,15 @@ describe('CostsPanel — split maths', () => { expect(splitEqualShares(10, [{ user_id: 1 }, { user_id: 2 }, { user_id: 3 }], 1)).toEqual({ 1: 3.33, 2: 3.34, 3: 3.33 }) }) - it('FE-W5COSTS-041: ticket items nobody is assigned to still count toward the total', () => { + it('FE-W5COSTS-041: a receipt line nobody is assigned to is carried by everyone on the receipt', () => { const items: TicketItem[] = [ { id: 'a', name: 'Apples', price: '10', participants: new Set([1, 2]) }, { id: 'b', name: 'Service', price: '5', participants: new Set() }, { id: 'c', name: 'Cake', price: 'x', participants: new Set([2]) }, ] - expect(calculateTicketShares(items)).toEqual({ shares: { 1: 5, 2: 5 }, total: 15 }) + // The service line used to count toward the total without landing on anyone, + // so the shares stayed a permanent 5.00 short of it (#1382). + expect(calculateTicketShares(items)).toEqual({ shares: { 1: 7.5, 2: 7.5 }, total: 15 }) }) }) diff --git a/client/src/components/Budget/CostsPanel.tsx b/client/src/components/Budget/CostsPanel.tsx index 2a210162a..4bca99248 100644 --- a/client/src/components/Budget/CostsPanel.tsx +++ b/client/src/components/Budget/CostsPanel.tsx @@ -1234,7 +1234,13 @@ export function ExpenseModal({ tripId, base, people, me, editing, prefill, onClo .map(id => ({ user_id: id, amount: parseFloat(payerAmounts[id]) || 0 })) .filter(p => p.amount > 0) : payerId > 0 ? [{ user_id: payerId, amount: totalNum }] : [] - const memberList = [...participants].map(id => ({ + // A receipt line can name somebody who is not ticked as a participant. Sending + // only the ticked set would drop their share, leaving the member sum short of + // total_price and handing the settlement a difference it can never clear (#1382). + const memberIds = splitMode === 'ticket' + ? [...new Set([...participants, ...Object.keys(ticketInfo.shares).map(Number)])].sort((a, b) => a - b) + : [...participants] + const memberList = memberIds.map(id => ({ user_id: id, amount: splitMode === 'custom' ? (parseFloat(customAmounts[id]) || 0) @@ -1248,7 +1254,7 @@ export function ExpenseModal({ tripId, base, people, me, editing, prefill, onClo currency, payers: payerList, members: memberList, - member_ids: [...participants], + member_ids: memberIds, expense_date: day || null, total_price: totalNum, note: note.trim() || null, diff --git a/client/src/components/Collections/AddPlaceToCollectionModal.test.tsx b/client/src/components/Collections/AddPlaceToCollectionModal.test.tsx index 7658da9d9..44282d5b3 100644 --- a/client/src/components/Collections/AddPlaceToCollectionModal.test.tsx +++ b/client/src/components/Collections/AddPlaceToCollectionModal.test.tsx @@ -1,4 +1,4 @@ -// FE-COMP-ADDPLACECOL-001 to FE-COMP-ADDPLACECOL-024 +// FE-COMP-ADDPLACECOL-001 to FE-COMP-ADDPLACECOL-028 import React from 'react' import type { Mock } from 'vitest' import { http, HttpResponse } from 'msw' @@ -163,13 +163,27 @@ describe('AddPlaceToCollectionModal', () => { await waitFor(() => expect(addToast).toHaveBeenCalledWith('Places API disabled', 'error', undefined)) }) - it('FE-COMP-ADDPLACECOL-009: a search response without places yields no dropdown', async () => { + it('FE-COMP-ADDPLACECOL-009: a search that finds nothing says so instead of staying silent', async () => { server.use(http.post('/api/maps/search', () => HttpResponse.json({ source: 'nominatim' }))) setup() fireEvent.change(screen.getByPlaceholderText('Search for a place…'), { target: { value: 'kissa' } }) fireEvent.click(screen.getByRole('button', { name: /Search/ })) - await waitFor(() => expect(screen.getByRole('button', { name: /Search/ })).not.toBeDisabled()) - expect(screen.queryByRole('button', { name: 'Close' })).not.toBeInTheDocument() + + expect(await screen.findByText('No places found')).toBeInTheDocument() + // Typing the next query clears it again. + fireEvent.change(screen.getByPlaceholderText('Search for a place…'), { target: { value: 'kissaten' } }) + expect(screen.queryByText('No places found')).not.toBeInTheDocument() + }) + + it('FE-COMP-ADDPLACECOL-009b: the empty search panel can be dismissed', async () => { + server.use(http.post('/api/maps/search', () => HttpResponse.json({ places: [], source: 'nominatim' }))) + setup() + fireEvent.change(screen.getByPlaceholderText('Search for a place…'), { target: { value: 'kissa' } }) + fireEvent.click(screen.getByRole('button', { name: /Search/ })) + await screen.findByText('No places found') + + fireEvent.click(screen.getByRole('button', { name: 'Close' })) + expect(screen.queryByText('No places found')).not.toBeInTheDocument() }) it('FE-COMP-ADDPLACECOL-010: saving posts the manually typed place with nulls for what is empty', async () => { @@ -360,4 +374,49 @@ describe('AddPlaceToCollectionModal', () => { expect(props.onClose).toHaveBeenCalledTimes(1) expect(savedBodies).toHaveLength(0) }) + + it('FE-COMP-ADDPLACECOL-025: the caret goes back to the search field after an add', async () => { + setup() + const nameField = screen.getByPlaceholderText('Name') + typeName('Manual spot') + // Where the focus sits in the browser once the add button disables itself. + nameField.focus() + fireEvent.click(screen.getByRole('button', { name: /Add$/ })) + + await waitFor(() => expect(nameField).toHaveValue('')) + expect(screen.getByPlaceholderText('Search for a place…')).toHaveFocus() + }) + + it('FE-COMP-ADDPLACECOL-026: the search still runs after a place was added', async () => { + mockSearch([mapsResult]) + setup() + typeName('Manual spot') + fireEvent.click(screen.getByRole('button', { name: /Add$/ })) + await waitFor(() => expect(screen.getByPlaceholderText('Name')).toHaveValue('')) + + const input = screen.getByPlaceholderText('Search for a place…') + expect(input).toHaveValue('') + fireEvent.change(input, { target: { value: 'kissa' } }) + expect(screen.getByRole('button', { name: /Search/ })).not.toBeDisabled() + fireEvent.keyDown(input, { key: 'Enter' }) + expect(await screen.findByText('Kissa Sakaiki')).toBeInTheDocument() + }) + + it('FE-COMP-ADDPLACECOL-027: the search row is pinned to the top of the scrolling body', () => { + setup() + // The dialog stays open after an add, so the row has to survive a scrolled form. + expect(screen.getByPlaceholderText('Search for a place…').closest('.sticky')).not.toBeNull() + }) + + it('FE-COMP-ADDPLACECOL-028: a failed save leaves the caret where the user had it', async () => { + server.use(http.post('/api/addons/collections/places', () => HttpResponse.json({ error: 'List is full' }, { status: 400 }))) + setup() + const nameField = screen.getByPlaceholderText('Name') + typeName('Manual spot') + nameField.focus() + fireEvent.click(screen.getByRole('button', { name: /Add$/ })) + + await waitFor(() => expect(addToast).toHaveBeenCalledWith('List is full', 'error', undefined)) + expect(nameField).toHaveFocus() + }) }) diff --git a/client/src/components/Collections/AddPlaceToCollectionModal.tsx b/client/src/components/Collections/AddPlaceToCollectionModal.tsx index 0e53dc78c..da2117dc8 100644 --- a/client/src/components/Collections/AddPlaceToCollectionModal.tsx +++ b/client/src/components/Collections/AddPlaceToCollectionModal.tsx @@ -42,6 +42,9 @@ export default function AddPlaceToCollectionModal({ isOpen, collectionId, collec const [query, setQuery] = useState('') const [results, setResults] = useState([]) const [searching, setSearching] = useState(false) + // A search that came back empty used to render nothing at all, which reads as + // "the dialog is dead". Say so instead (#1921). + const [noResults, setNoResults] = useState(false) // The picked location (address/coords/ids) plus the editable fields. const [picked, setPicked] = useState(null) const [name, setName] = useState('') @@ -56,16 +59,20 @@ export default function AddPlaceToCollectionModal({ isOpen, collectionId, collec const [status, setStatus] = useState('idea') const [saving, setSaving] = useState(false) const descRef = useRef(null) + const searchRef = useRef(null) - const reset = () => { setQuery(''); setResults([]); setPicked(null); setName(''); setAddress(''); setLat(''); setLng(''); setCategoryId(null); setDescription(''); setLinks([]); setStatus('idea') } + const reset = () => { setQuery(''); setResults([]); setNoResults(false); setPicked(null); setName(''); setAddress(''); setLat(''); setLng(''); setCategoryId(null); setDescription(''); setLinks([]); setStatus('idea') } useEffect(() => { if (!isOpen) reset() }, [isOpen]) const search = async () => { if (!query.trim()) return setSearching(true) + setNoResults(false) try { const res = await mapsApi.search(query, language) - setResults((res.places as MapsPlace[]) || []) + const places = (res.places as MapsPlace[]) || [] + setResults(places) + setNoResults(places.length === 0) } catch (err) { toast.error(getApiErrorMessage(err, t('places.mapsSearchError'))) } finally { @@ -73,6 +80,8 @@ export default function AddPlaceToCollectionModal({ isOpen, collectionId, collec } } + const dismissResults = () => { setResults([]); setNoResults(false) } + const pick = (r: MapsPlace) => { setPicked(r) setName(str(r.name) ?? '') @@ -80,7 +89,7 @@ export default function AddPlaceToCollectionModal({ isOpen, collectionId, collec const la = num(r.lat); const lo = num(r.lng) setLat(la != null ? String(la) : '') setLng(lo != null ? String(lo) : '') - setResults([]); setQuery(str(r.name) ?? query) + setResults([]); setNoResults(false); setQuery(str(r.name) ?? query) } const setLink = (i: number, patch: Partial) => setLinks(links.map((l, idx) => (idx === i ? { ...l, ...patch } : l))) @@ -112,6 +121,11 @@ export default function AddPlaceToCollectionModal({ isOpen, collectionId, collec if (res.duplicate) toast.info(t('collections.duplicateWarning')) else { toast.success(t('collections.addedToList', { name: collectionName })); onAdded() } reset() + // The dialog stays open for the next place, so hand the caret back to the + // search field. The add button the user just clicked goes disabled with the + // cleared name, which drops the focus to and leaves the dialog dead + // to the keyboard (#1921). + searchRef.current?.focus() } catch (err) { toast.error(getApiErrorMessage(err, t('common.error'))) } finally { @@ -142,42 +156,51 @@ export default function AddPlaceToCollectionModal({ isOpen, collectionId, collec } >
- {/* Search — picking a result fills the location below */} -
-
-
- - setQuery(e.target.value)} - onKeyDown={e => { if (e.key === 'Enter') { e.preventDefault(); search() } }} - placeholder={t('collections.addPlaceSearch')} - className="w-full pl-9 pr-3 py-2 rounded-lg border border-edge bg-surface-input text-content text-[14px] outline-none focus:border-accent" - /> -
- -
- {results.length > 0 && ( -
-
- {t('common.search')} - + {/* Search — picking a result fills the location below. Pinned to the top of + the scrolling dialog body: the dialog stays open after an add, and the + search row used to sit above the viewport once the form was scrolled, + so it looked like the button had vanished (#1921). The negative margins + let its background cover the body padding while it is stuck. */} +
+
+
+
+ + { setQuery(e.target.value); setNoResults(false) }} + onKeyDown={e => { if (e.key === 'Enter') { e.preventDefault(); search() } }} + placeholder={t('collections.addPlaceSearch')} + className="w-full pl-9 pr-3 py-2 rounded-lg border border-edge bg-surface-input text-content text-[14px] outline-none focus:border-accent" + />
- {results.map((r, i) => ( - - ))} +
- )} + {(results.length > 0 || noResults) && ( +
+
+ {t('common.search')} + +
+ {noResults ? ( +
{t('planner.noPlacesFound')}
+ ) : results.map((r, i) => ( + + ))} +
+ )} +
{/* Name */} diff --git a/client/src/components/Collections/CollectionPlaceDetail.test.tsx b/client/src/components/Collections/CollectionPlaceDetail.test.tsx index c57a2f933..a4ea825f8 100644 --- a/client/src/components/Collections/CollectionPlaceDetail.test.tsx +++ b/client/src/components/Collections/CollectionPlaceDetail.test.tsx @@ -1,4 +1,4 @@ -// FE-COMP-COLDETAIL-001 to FE-COMP-COLDETAIL-035 +// FE-COMP-COLDETAIL-001 to FE-COMP-COLDETAIL-043 import React from 'react'; import { render, screen, fireEvent, waitFor, within } from '../../../tests/helpers/render'; import userEvent from '@testing-library/user-event'; @@ -335,6 +335,8 @@ describe('CollectionPlaceDetail', () => { links: [{ label: undefined, url: 'https://x.com' }], category_id: 2, label_ids: [1, 2], + // Untouched in this case, but the address rides along on every save (#1870). + address: 'Somewhere', lat: 52.5, lng: 13.4, }); @@ -547,3 +549,56 @@ describe('CollectionPlaceDetail — open in maps', () => { expect(screen.queryByRole('button', { name: /navigation|google maps/i })).toBeNull(); }); }); + +// ── Editable address (#1870) ───────────────────────────────────────────────── + +describe('CollectionPlaceDetail: editable address', () => { + it('FE-COMP-COLDETAIL-040: edit mode offers an address field seeded from the place', async () => { + const user = userEvent.setup(); + renderDetail({ canEdit: true }); + await user.click(await screen.findByRole('button', { name: 'Edit' })); + + expect(screen.getByText('Address')).toBeInTheDocument(); + expect(screen.getByPlaceholderText('Street, City, Country')).toHaveValue('Somewhere'); + }); + + it('FE-COMP-COLDETAIL-041: a corrected address is trimmed into the patch', async () => { + const user = userEvent.setup(); + const props = renderDetail({ canEdit: true }); + await user.click(await screen.findByRole('button', { name: 'Edit' })); + + const addressInput = screen.getByPlaceholderText('Street, City, Country'); + await user.clear(addressInput); + await user.type(addressInput, ' Via Nuova 1 '); + await user.click(screen.getByRole('button', { name: /Save/ })); + + await waitFor(() => expect(props.onSave).toHaveBeenCalled()); + expect(props.onSave).toHaveBeenCalledWith(expect.objectContaining({ address: 'Via Nuova 1' })); + }); + + it('FE-COMP-COLDETAIL-042: an emptied address is sent as null', async () => { + const user = userEvent.setup(); + const props = renderDetail({ canEdit: true }); + await user.click(await screen.findByRole('button', { name: 'Edit' })); + + await user.clear(screen.getByPlaceholderText('Street, City, Country')); + await user.click(screen.getByRole('button', { name: /Save/ })); + + await waitFor(() => expect(props.onSave).toHaveBeenCalled()); + expect(props.onSave).toHaveBeenCalledWith(expect.objectContaining({ address: null })); + }); + + it('FE-COMP-COLDETAIL-043: Cancel restores the stored address', async () => { + const user = userEvent.setup(); + const props = renderDetail({ canEdit: true }); + await user.click(await screen.findByRole('button', { name: 'Edit' })); + + await user.type(screen.getByPlaceholderText('Street, City, Country'), ' 2'); + await user.click(screen.getByRole('button', { name: 'Cancel' })); + expect(props.onSave).not.toHaveBeenCalled(); + expect(screen.getByText('Somewhere')).toBeInTheDocument(); + + await user.click(screen.getByRole('button', { name: 'Edit' })); + expect(screen.getByPlaceholderText('Street, City, Country')).toHaveValue('Somewhere'); + }); +}); diff --git a/client/src/components/Collections/CollectionPlaceDetail.tsx b/client/src/components/Collections/CollectionPlaceDetail.tsx index 2684b5cc0..7a3166b50 100644 --- a/client/src/components/Collections/CollectionPlaceDetail.tsx +++ b/client/src/components/Collections/CollectionPlaceDetail.tsx @@ -35,7 +35,7 @@ interface CollectionPlaceDetailProps { anchorRect?: { left: number; width: number } | null onClose: () => void onSetStatus: (status: CollectionStatus) => void - onSave: (patch: { name?: string; description?: string | null; links?: CollectionLink[]; category_id?: number | null; label_ids?: number[]; image_url?: string | null; lat?: number | null; lng?: number | null }) => Promise + onSave: (patch: { name?: string; description?: string | null; links?: CollectionLink[]; category_id?: number | null; label_ids?: number[]; image_url?: string | null; lat?: number | null; lng?: number | null; address?: string | null }) => Promise /** Upload a custom cover image (#1136); enables the cover change/remove controls. */ onUploadImage?: (file: File) => Promise onCopyToTrip: () => void @@ -83,6 +83,7 @@ export default function CollectionPlaceDetail({ const [description, setDescription] = useState(place.description ?? '') const [links, setLinks] = useState(place.links ?? []) const [labelIds, setLabelIds] = useState(place.label_ids ?? []) + const [address, setAddress] = useState(place.address ?? '') const [lat, setLat] = useState(place.lat != null ? String(place.lat) : '') const [lng, setLng] = useState(place.lng != null ? String(place.lng) : '') const [saving, setSaving] = useState(false) @@ -99,6 +100,7 @@ export default function CollectionPlaceDetail({ setDescription(place.description ?? '') setLinks(place.links ?? []) setLabelIds(place.label_ids ?? []) + setAddress(place.address ?? '') setLat(place.lat != null ? String(place.lat) : '') setLng(place.lng != null ? String(place.lng) : '') // eslint-disable-next-line react-hooks/exhaustive-deps @@ -147,7 +149,7 @@ export default function CollectionPlaceDetail({ const setLink = (i: number, patch: Partial) => setLinks(links.map((l, idx) => (idx === i ? { ...l, ...patch } : l))) const toggleLabel = (id: number) => setLabelIds(labelIds.includes(id) ? labelIds.filter(x => x !== id) : [...labelIds, id]) - const resetForm = () => { setEditing(false); setName(place.name); setCategoryId(place.category_id ?? null); setDescription(place.description ?? ''); setLinks(place.links ?? []); setLabelIds(place.label_ids ?? []); setLat(place.lat != null ? String(place.lat) : ''); setLng(place.lng != null ? String(place.lng) : '') } + const resetForm = () => { setEditing(false); setName(place.name); setCategoryId(place.category_id ?? null); setDescription(place.description ?? ''); setLinks(place.links ?? []); setLabelIds(place.label_ids ?? []); setAddress(place.address ?? ''); setLat(place.lat != null ? String(place.lat) : ''); setLng(place.lng != null ? String(place.lng) : '') } const coordPaste = (e: React.ClipboardEvent) => { const text = e.clipboardData.getData('text').trim() const match = text.match(/^(-?\d+\.?\d*)\s*[,;\s]\s*(-?\d+\.?\d*)$/) @@ -161,7 +163,7 @@ export default function CollectionPlaceDetail({ const lngNum = lng.trim() ? Number(lng) : NaN setSaving(true) try { - await onSave({ name: name.trim() || place.name, description: description.trim() || null, links: cleanLinks, category_id: categoryId, label_ids: labelIds, lat: Number.isFinite(latNum) ? latNum : null, lng: Number.isFinite(lngNum) ? lngNum : null }) + await onSave({ name: name.trim() || place.name, description: description.trim() || null, links: cleanLinks, category_id: categoryId, label_ids: labelIds, address: address.trim() || null, lat: Number.isFinite(latNum) ? latNum : null, lng: Number.isFinite(lngNum) ? lngNum : null }) setEditing(false) } catch (err) { toast.error(getApiErrorMessage(err, t('common.error'))) @@ -280,6 +282,11 @@ export default function CollectionPlaceDetail({ })}
+ {/* Address (#1870): free text, the coordinates stay separate */} +
+
{t('places.formAddress')}
+ setAddress(e.target.value)} placeholder={t('places.formAddressPlaceholder')} className="col-detail-input" /> +
{/* Coordinates */}
{t('collections.coordinates')}
diff --git a/client/src/components/PDF/JourneyBookPDF.test.tsx b/client/src/components/PDF/JourneyBookPDF.test.tsx index 8d7a994d5..d71bbcbca 100644 --- a/client/src/components/PDF/JourneyBookPDF.test.tsx +++ b/client/src/components/PDF/JourneyBookPDF.test.tsx @@ -1,10 +1,11 @@ -// FE-COMP-JOURNEYPDF-001 to FE-COMP-JOURNEYPDF-006 +// FE-COMP-JOURNEYPDF-001 to FE-COMP-JOURNEYPDF-012 // -// JourneyBookPDF.tsx exports an async function `downloadJourneyBookPDF(journey)` -// that renders a PDF preview in an srcdoc iframe overlay (Safari-safe pattern). -// Tests verify the overlay DOM structure and HTML content. +// JourneyBookPDF.tsx exports an async function +// `downloadJourneyBookPDF(journey, { t, locale })` that renders a PDF preview in +// an srcdoc iframe overlay (Safari-safe pattern). Tests verify the overlay DOM +// structure, the HTML content and that the book follows the app language. -import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; +import { describe, it, expect, vi, afterEach } from 'vitest'; // Mock `marked` so we don't need the real markdown parser vi.mock('marked', () => ({ @@ -87,6 +88,18 @@ function getIframe(): HTMLIFrameElement | null { return getOverlay()?.querySelector('iframe') ?? null; } +// Echoes the key (plus any params) so the assertions read as "this string comes +// from i18n" instead of pinning English copy. +function t(key: string, params?: Record): string { + if (!params) return key; + return `${key}(${Object.entries(params).map(([k, v]) => `${k}=${v}`).join(',')})`; +} + +const longDate = (d: string, locale: string) => + new Date(d + 'T00:00:00').toLocaleDateString(locale, { + weekday: 'long', month: 'long', day: 'numeric', year: 'numeric', + }); + // ── Setup ──────────────────────────────────────────────────────────────────── afterEach(() => { @@ -142,9 +155,10 @@ describe('downloadJourneyBookPDF', () => { expect(getOverlay()).not.toBeNull(); const html = getIframe()!.srcdoc; expect(html).toContain('Iceland Ring Road'); - // No entry pages, but cover and closing page are still present - expect(html).toContain('Journey Book'); - expect(html).toContain('The End'); + // No entry pages, but cover and closing page are still present. Without a + // translator the chrome falls back to the raw keys instead of throwing. + expect(html).toContain('journey.pdf.journeyBook'); + expect(html).toContain('journey.pdf.theEnd'); }); it('FE-COMP-JOURNEYPDF-007: sanitises HTML injected via an entry story and keeps the iframe script-free', async () => { @@ -162,4 +176,72 @@ describe('downloadJourneyBookPDF', () => { expect(html).toContain('Hello'); expect(html).toContain('world'); }); + + it('FE-COMP-JOURNEYPDF-008: renders cover, day header and closing page through i18n', async () => { + await downloadJourneyBookPDF(buildJourney(), { t, locale: 'de-DE' }); + const html = getIframe()!.srcdoc; + + expect(html).toContain(''); + expect(html).toContain('journey.pdf.journeyBook'); + expect(html).toContain('journey.stats.days'); + expect(html).toContain('journey.stats.entries'); + expect(html).toContain('journey.stats.photos'); + expect(html).toContain('journey.pdf.madeWith'); + expect(html).toContain('journey.pdf.theEnd'); + // Day header is the parameterised key plus a date in the picked locale — + // the hardcoded 'en' formatting would not produce this string. + expect(html).toContain('journey.detail.day(number=1)'); + expect(html).toContain(longDate('2026-07-01', 'de-DE')); + // Pros/cons headings too. + expect(html).toContain('journey.verdict.lovedIt'); + expect(html).toContain('journey.verdict.couldBeBetter'); + }); + + it('FE-COMP-JOURNEYPDF-009: translates the preview bar above the book', async () => { + await downloadJourneyBookPDF(buildJourney(), { t, locale: 'de-DE' }); + const overlay = getOverlay()!; + + // cover + one entry page + closing page + expect(overlay.textContent).toContain('3 journey.pdf.pages'); + expect(overlay.querySelector('#journey-pdf-save')!.textContent).toBe('journey.pdf.saveAsPdf'); + expect(overlay.querySelector('#journey-pdf-close')!.textContent).toBe('common.close'); + }); + + it('FE-COMP-JOURNEYPDF-010: prints mood and weather next to the entry', async () => { + const journey = buildJourney(); + journey.entries[0].mood = 'good'; + journey.entries[0].weather = 'rainy'; + await downloadJourneyBookPDF(journey, { t, locale: 'en-US' }); + const html = getIframe()!.srcdoc; + + expect(html).toContain('journey.mood.good'); + expect(html).toContain('journey.weather.rainy'); + // Icons come along as inline SVG inside the chips. + expect(html).toContain('class="entry-chips"'); + expect(html).toContain(' { + // The fixture carries mood 'excited', which is not one of the four moods. + await downloadJourneyBookPDF(buildJourney(), { t, locale: 'en-US' }); + const html = getIframe()!.srcdoc; + // Scoped to the chip row: the page elsewhere is free to carry whatever the + // fixture happens to leave undefined, and that is not what this asserts. + const chips = html.match(/
[\s\S]*?<\/div>/)?.[0] ?? ''; + + expect(chips).not.toBe(''); + expect(chips).not.toContain('undefined'); + expect(chips).not.toContain('journey.mood.'); + // The known weather value still gets its chip. + expect(chips).toContain('journey.weather.sunny'); + }); + + it('FE-COMP-JOURNEYPDF-012: leaves out the chip row when an entry has neither', async () => { + const journey = buildJourney(); + journey.entries[0].mood = null; + journey.entries[0].weather = null; + await downloadJourneyBookPDF(journey, { t, locale: 'en-US' }); + + expect(getIframe()!.srcdoc).not.toContain('class="entry-chips"'); + }); }); diff --git a/client/src/components/PDF/JourneyBookPDF.tsx b/client/src/components/PDF/JourneyBookPDF.tsx index 9aae33fec..a7f267eb4 100644 --- a/client/src/components/PDF/JourneyBookPDF.tsx +++ b/client/src/components/PDF/JourneyBookPDF.tsx @@ -1,7 +1,13 @@ // Journey Photo Book PDF — Polarsteps-inspired, magazine-density +import { createElement } from 'react' +import type { LucideIcon } from 'lucide-react' import { marked } from 'marked' import { sanitizeRichTextHtml } from '@trek/shared' import type { JourneyDetail, JourneyEntry, JourneyPhoto } from '../../store/journeyStore' +import { MOOD_CONFIG, WEATHER_CONFIG } from '../../pages/journeyDetail/JourneyDetailPage.constants' +import { renderIconMarkup } from '../../utils/iconMarkup' + +type Translate = (key: string, params?: Record) => string function esc(str: string | null | undefined): string { if (!str) return '' @@ -25,13 +31,9 @@ function pSrc(p: JourneyPhoto): string { return abs(`/api/photos/${p.photo_id}/original`) } -function fmtDate(d: string): string { +function fmtDate(d: string, locale: string): string { const date = new Date(d + 'T00:00:00') - return date.toLocaleDateString('en', { weekday: 'long', month: 'long', day: 'numeric', year: 'numeric' }) -} - -function fmtShort(d: string): string { - return new Date(d + 'T00:00:00').toLocaleDateString('en', { month: 'short', day: 'numeric' }) + return date.toLocaleDateString(locale, { weekday: 'long', month: 'long', day: 'numeric', year: 'numeric' }) } function groupByDate(entries: JourneyEntry[]): Map { @@ -44,7 +46,7 @@ function groupByDate(entries: JourneyEntry[]): Map { return groups } -function renderProscons(entry: JourneyEntry): string { +function renderProscons(entry: JourneyEntry, tr: Translate): string { const pc = entry.pros_cons if (!pc) return '' const pros = pc.pros?.filter(p => p.trim()) || [] @@ -52,11 +54,31 @@ function renderProscons(entry: JourneyEntry): string { if (pros.length === 0 && cons.length === 0) return '' return `
- ${pros.length > 0 ? `
Loved it
    ${pros.map(p => `
  • ${esc(p)}
    • `).join('')}
` : ''} - ${cons.length > 0 ? `
Could be better
    ${cons.map(c => `
  • ${esc(c)}
    • `).join('')}
` : ''} + ${pros.length > 0 ? `
${esc(tr('journey.verdict.lovedIt'))}
    ${pros.map(p => `
  • ${esc(p)}
    • `).join('')}
` : ''} + ${cons.length > 0 ? `
${esc(tr('journey.verdict.couldBeBetter'))}
    ${cons.map(c => `
  • ${esc(c)}
    • `).join('')}
` : ''}
` } +function chip(icon: LucideIcon, label: string, bg: string, color: string): string { + const svg = renderIconMarkup(createElement(icon, { size: 12, strokeWidth: 2, color })) + return `${svg}${esc(label)}` +} + +/** + * Mood and weather were on screen but never in the book (#1848). Unknown values + * (older entries, a provider-set category we don't render) drop out silently, + * the same way MoodChip/WeatherChip return null for them. + */ +function renderMoodWeather(entry: JourneyEntry, tr: Translate): string { + const chips: string[] = [] + const mood = entry.mood ? MOOD_CONFIG[entry.mood] : undefined + if (mood) chips.push(chip(mood.icon, tr(mood.label), mood.bg, mood.text)) + const weather = entry.weather ? WEATHER_CONFIG[entry.weather] : undefined + if (weather) chips.push(chip(weather.icon, tr(weather.label), '#f4f4f5', '#3f3f46')) + if (chips.length === 0) return '' + return `
${chips.join('')}
` +} + function renderPhotoBlock(photos: JourneyPhoto[]): string { if (photos.length === 0) return '' if (photos.length === 1) { @@ -75,7 +97,16 @@ function renderPhotoBlock(photos: JourneyPhoto[]): string {
` } -export async function downloadJourneyBookPDF(journey: JourneyDetail) { +/** + * `opts` stays optional so the signature is additive, but every caller passes + * the page's `t`/`locale` — without them the book falls back to raw keys (#1848). + */ +export async function downloadJourneyBookPDF( + journey: JourneyDetail, + opts: { t?: Translate; locale?: string } = {}, +) { + const tr: Translate = opts.t || (key => key) + const loc = opts.locale || 'en' const entries = (journey.entries || []).filter(e => e.type !== 'skeleton') const allPhotos = entries.flatMap(e => e.photos || []) const coverUrl = journey.cover_image ? abs(`/uploads/${journey.cover_image}`) : (allPhotos[0] ? pSrc(allPhotos[0]) : '') @@ -96,14 +127,14 @@ export async function downloadJourneyBookPDF(journey: JourneyDetail) { // Day header (inline, only on first entry of day) const dayHeaderHtml = isFirstOfDay - ? `
Day ${di + 1} · ${fmtDate(date)}
` + ? `
${esc(tr('journey.detail.day', { number: di + 1 }))} · ${esc(fmtDate(date, loc))}
` : '' // Photo block const photoHtml = renderPhotoBlock(photos) // Pros/cons - const prosconsHtml = renderProscons(entry) + const prosconsHtml = renderProscons(entry, tr) // Story (markdown) const storyHtml = entry.story ? `
${md(entry.story)}
` : '' @@ -114,6 +145,7 @@ export async function downloadJourneyBookPDF(journey: JourneyDetail) { ${photoHtml}
${meta ? `
${esc(meta)}
` : ''} + ${renderMoodWeather(entry, tr)} ${entry.title ? `

${esc(entry.title)}

` : ''} ${storyHtml} ${prosconsHtml} @@ -126,11 +158,17 @@ export async function downloadJourneyBookPDF(journey: JourneyDetail) { const totalPages = pageNum + 1 // +1 for closing page const html = ` - + + + -${esc(journey.title)} — Journey Book +${esc(journey.title)} — ${esc(tr('journey.pdf.journeyBook'))}