From 9dd0a851cab6688c8d2f4d5a10750bfd0084c638 Mon Sep 17 00:00:00 2001 From: Maurice <61554723+mauriceboe@users.noreply.github.com> Date: Thu, 13 Aug 2026 14:06:35 +0200 Subject: [PATCH] fix: eleven reported bugs across atlas, budget, collections, journey, vacay and the LLM settings (#1923) * fix(mcp): accept per-leg times on create_transport and update_transport metadata was pinned to Record, so a legs array could not get through and a multi-leg flight built over MCP lost every intermediate time. The booking ended up with correct endpoints and no legs, which every reader then treats as a single segment. legs is now a real, typed parameter with its contract in @trek/shared, shaped exactly the way the planner form and the importers write metadata.legs. The folding checks what the readers depend on: one leg fewer than endpoints (the AirTrail dedupe builds its signatures on that), day ids that belong to the trip, airport codes that agree with their endpoint, and the times, where a stopover endpoint carries the departure and the last one the arrival. metadata keeps its type and its replace semantics. Segments smuggled through metadata.legs as a JSON string used to be stored and then ignored by everything that read them; they are refused now and point at the legs parameter instead. Closes #1914 * fix(atlas): refuse a bucket list entry that is already on the list Nothing gave a bucket entry an identity, so the same country could be added as often as it was clicked. A wish is now identified by name, country, target date and, where it has them, coordinates. A different target date is a different wish and still lands, which is what the report asked for; two real places sharing a name stay separate because their coordinates differ. Empty string and NULL are normalised on both sides of the comparison, otherwise the dateless case, which is the one that was reported, would slip through. Deliberately no unique index and no migration. better-sqlite3 is synchronous and the server is a single process, so the race an index would guard against cannot happen here, and an index would have needed a destructive dedupe of existing rows first. create_bucket_list_item had no target_date parameter at all, so over MCP the allowed "same place, other date" case was unreachable. It takes one now. The plugin RPC reports the duplicate as bad params instead of letting it surface as an internal error. Closes #1898 * fix(settings): keep a free-form LLM endpoint in admin hands Any account could name the address the server calls, and the guard in front of it deliberately allows loopback and LAN so a self-hosted Ollama works. That is a reasonable trade for whoever runs the instance, because the network is theirs. It is not one for an arbitrary account, which could aim a request from inside that network at a target only it chose and read how the target answered. A free base URL, and the local provider that is one by another name, now come only from an admin controlled source: the addon config, the instance-wide defaults, or an admin's own settings. OpenAI and Anthropic stay available to everyone, because they go to a fixed address with the user's own key. The resolver is where this is enforced, since it is the single point both the booking import and the plugin host pass through, and the only one that also covers values already sitting in the database. The write path refuses as well, so nothing is stored that would never be read. The hint text promised something users no longer have, so it changed with it. Closes #1772 * fix(calendar): span a date-only booking to its end date A DATE event without DTEND is exactly one day per RFC 5545, so a multi-day booking that carries an end date still collapsed onto its first day. It gets an exclusive DTEND now, built the same way the stay branch already builds one. A booking without an end date serialises byte for byte as before. While in here: the stays query joins reservations on the accommodation, and two bookings pointed at the same stay fanned it out into two events sharing one UID. Duplicate UIDs in a feed are undefined behaviour and calendar clients pick one at random. The same "at most one booking per stay" assumption sat in the accommodations service, where it was worse than cosmetic: accommodation_id carries no foreign key, so deleting a stay took only the first booking and left the second pointing at something that no longer existed. Both the sync and the delete walk all of them now. The callers keep reading linkedReservationId; the list is additional. Refs #1869 * fix(vacay): say that the region is required, because it is The placeholder called the region optional while the add button stayed disabled until one was picked. The wording predates multiple calendars per plan, which is what introduced the gate. Only the value of the existing key changed, in all locales, so the key set stays identical. Two things the same screen got wrong were in the way of a clean answer: The button was clickable while the region list was still loading, which put the user in exactly the state the wording describes, and it is the state the report was filed from. It waits for the list now. Changing the country on an existing calendar kept the previous country's regions on screen, so a German region could be picked for the Netherlands. The list is cleared with the country, which only works in combination with the loading gate: without it, the empty list would happen to enable the button instead. Closes #1813 * fix(collections): let a saved place's address be corrected The edit form offered name, category and labels, and the address only on the way in. The cause was not the form: the update contract never declared address, so the validation pipe stripped it and the UPDATE had no column for it. Coordinates were pulled through for #1435 and the address was missed. It travels the whole chain now, contract to service to both forms. No migration, the column has always been there. The mobile add sheet had no address input at all, showing whatever the search result carried as read-only, so editing would have worked while creating still would not. It has one. The MCP tool description had drifted further than the address, never having mentioned the coordinates either. Both are in it now. Closes #1870 * fix(collections): keep the place search usable after the first save The add dialog stays open on purpose so several places can be added in a row, but after a save it gave no way back into the search. A search with no results was silent, with no empty state and no toast, so typing another name and pressing enter looked like nothing happened. The reporter described both halves and it is the silent search that matches "nothing happens" most closely. There is an empty state now, using a key that already existed in every locale and was unused. The search row is sticky inside the dialog body, so it stays reachable after the save instead of sitting above the scroll position. Also fixed while in here: refreshing the collection after each add reset the select mode, the selection and the label filter. In the very flow this dialog is built for, the user quietly lost all three. Closes #1921 * fix(vacay): make the phone year grid and the dock button do what they look like Two separate faults met on this screen. Tapping a day in the year grid did nothing unless the edit mode was already on, and the only way into it was a floating button that looks exactly like the one the dock draws in the same spot. A tap now opens that month in the editor, where the cells are big enough to hit. At 390px the grid cells are around 21px, which is too small to be mutating data with in the first place. The dock's plus was not wired for this route, so it fell through to creating a trip on the dashboard. Two different actions were stacked in one place, and the wrong one was on top whenever the screen had not painted yet, which includes the time its chunk is still loading. The dock leaves the slot to the screen now, and holds the geometry so the tab groups stay symmetric. Closes #1811 * fix(budget): settle in whole cents so balances stop drifting The report is a balance that moves on its own, with nothing added, edited or removed, on a trip where everything is in euro. Two mechanisms were behind it and both are real. The display currency is a fourth value, a per-user setting, so "everything is in euro" never covered it. Each balance was converted and rounded on its own, and the set no longer summed to zero: at 0.855 and 0.9312 it came out one cent over, at 0.94 it came out even. That cent appears and disappears with the six hour rate cache alone, which is exactly the symptom. The flows were rounded separately from the balances, so the two never had to agree. And a receipt line nobody was ticked for counted toward the total, and therefore toward the payer's credit, while landing in no share. That difference is permanent, and the client sent it that way. The settlement converts once, at the edge, and does integer arithmetic after that. Equal splits distribute the credited cents by largest remainder rather than the entered total, so credits and debits of one expense cancel exactly even in a foreign currency. Display conversion goes through the same largest-remainder allocation, so the sum survives it. With that, the 0.01 epsilon can go. It could never create a cent, but it stranded them: a one cent balance was filtered out of the flows while staying visible in the list, which is the "settle up offers nothing" half of the report. Closes #1382 * fix(journey): reach the book export and the suggestion toggle on a phone Below 1024px the hero card is dropped, and it held the only way to either action. The floating bar that replaces it was never given them. It has them now, and the hero is unmounted rather than hidden, so there is no second invisible copy of the same controls in the document. The bar's own buttons moved onto the appearance tokens while they were being touched. The phone screen never had either action at all and now carries both. The book itself was hardwired to English, cover and dates included, while the keys for it had been sitting in every locale unused. It takes the page's translator and locale, sets the document language, and prints mood and weather, which were on screen but never in the book. Closes #1848 * fix(journey): offer the connected photo providers in the phone entry editor The desktop editor grew an external photo source in #1632; the phone editor that came out of the mobile rewrite never did, so editing an entry on a phone had no way to Immich or Synology. It mirrors the desktop mechanics, including the part that is easy to miss: a save that creates the entry and then fails on the photos keeps the sheet open, and without remembering what was just created the retry creates a second entry. The sheet no longer kicks off its own addon load. The list is loaded once on boot and this only reacts to it; a second load races the other consumers and overwrites the store when it lands. Closes #1808 * fix(mobile): let the document scroll so Safari can collapse its URL bar iOS Safari only shrinks its address bar when the document scrolls, and TREK's never did: the root was locked and the mobile shell put a scroll container inside it. On the bottom tab layout that costs about a sixth of the viewport, permanently. Below 768px the document is the scroller again and the full-bleed screens measure their own height instead of leaning on the shell. That makes the body scroll locks real for the first time on a phone, and they were not ready for it. Three of them reset the overflow unconditionally, one of those on every run of its effect rather than on unmount, so a system notice could hand the page back while a sheet was still open. They all go through one reference counted lock now, which is the part that had to land first. Worth knowing for the issue: this reaches the flow screens. The trip planner, the journal, the atlas and the vacay screen keep their own full-height scrollers and still will not collapse the bar. Closes #1809 * fix(mobile): keep the screen gradient visible behind the translucent cards The gradient layer introduced with the scroll change sat at z-index -10. A negative layer paints above the canvas but below the backgrounds of ordinary blocks, and body carries an opaque one, so the gradient was hidden behind it. The mobile cards are translucent and take their depth from what is behind them, so every surface ended up compositing against a single flat colour and the whole phone UI read as one tone. The layer is positioned at z-index 0 now and the content above it at 10, which puts both over body's background. Same visual as before the scroll change, and the shell still owns no scroll container. * test(reservations): teach the accommodation delete stub about the id lists The RPC walks every linked booking since the delete stopped stopping at the first one. The stub still answered with the two singular fields, so the loop had nothing to iterate and the cascade events never fired. * fix(settings): move the free-form LLM endpoint out of user settings entirely Hiding it from non-admins closed the hole, but it left the option sitting in the admin's own personal settings, which is not what was asked for and not where it belongs: an instance has exactly one address it calls out to, and there was a second, invisible place to set it. The provider list is now the two hosted ones for everybody, the endpoint field is gone, and the resolver takes the address from the admin-controlled instance defaults for every caller including an admin. It no longer reads the users table at all. The write path refuses the keys for everyone, so nothing can be stored that would never be read. The endpoint is configured on the addon in the admin settings, which already carries provider, model, base URL and key, plus the Ollama model list. Nothing is lost, it just has one home now. Behaviour change worth a release note: an admin who set a personal Ollama endpoint has to move it to the addon config. A non-admin was already in that position after the previous commit. Refs #1772 * fix(journey): make the book preview usable on a phone The preview document carried no viewport, so a phone laid it out at its default 980px while the page is A4 landscape and about 1123px wide. It spilled out of the frame, which reads as being zoomed far in. Declaring the real width lets the browser scale the page to fit. Printing is unaffected, it goes by @page. The preview chrome had the same problem in miniature: a title and two labels on one row, sized inline, which does not fit a phone and fits translated labels even less. It moved to a stylesheet, and below 640px the card goes full-bleed, the decorative title steps aside and the two actions share the width. Refs #1848 * fix(mobile): stop the shell trapping the screens' own overlays Lifting the background layer into the positioned range meant the content wrapper had to be positioned too, and a positioned wrapper with a z-index is a stacking context. Every fixed overlay a screen renders inside it got clamped to that level: the vacay view/edit FAB sits at z-50 precisely so it beats the dock at z-40, and it disappeared behind it. The layer goes back to a negative z-index and the wrapper back to no positioning at all. What it needed in the first place was for body to stop painting over it, which is one line in the phone media block rather than a new stacking context. * fix(mobile): let the phone background override actually win A media query adds no specificity, so the themed body rule further down the file kept winning on source order and painted over the shell's background layer. The override is on html body now. The gradient was back to being invisible, which is the flat single-tone phone UI again. * fix(vacay): draw today's ring around the day instead of replacing it On the phone, today short-circuited the day-cell matrix and returned a ring on an empty cell. A vacation day or a company holiday logged for today was stored and counted in the totals, but the cell showed nothing, so it read as if the tap had not taken. Today is an annotation now, layered like the school band and the shared-calendar rings, which is how the desktop month card has always treated it. The strong ink stays only where the cell has no fill of its own, since --m-ink is light in dark mode and would disappear on a pastel. * fix(map): stop serving a stale GL style document from the service worker The Workbox rule for Mapbox matched every URL under api.mapbox.com, and the style document lives there too. Under StaleWhileRevalidate the map was built from the previous revision on every load, with the refresh landing one load too late, so a style republished in Studio with different label languages kept rendering the old ones. The network tab showed a 200 throughout, because the service worker's own answer is a real 200, which is why this looked like the app rewriting text-field at runtime. The style now has its own rule ahead of the tile rules, on NetworkFirst so it is current whenever the network is, and still cached so the map opens offline. Its own small cache too: sharing an LRU with three thousand tiles made the staleness come and go at random. OpenFreeMap styles had the same problem and are covered by the same rule. Closes #1924 --- client/e2e/urlbar-scroll-1809.spec.ts | 105 ++++ .../Budget/CostsPanel.helpers.test.ts | 46 +- .../components/Budget/CostsPanel.helpers.ts | 24 +- .../src/components/Budget/CostsPanel.test.tsx | 6 +- client/src/components/Budget/CostsPanel.tsx | 10 +- .../AddPlaceToCollectionModal.test.tsx | 67 ++- .../Collections/AddPlaceToCollectionModal.tsx | 97 +-- .../CollectionPlaceDetail.test.tsx | 57 +- .../Collections/CollectionPlaceDetail.tsx | 13 +- .../components/PDF/JourneyBookPDF.test.tsx | 98 ++- client/src/components/PDF/JourneyBookPDF.tsx | 119 +++- .../Settings/LlmConnectionSection.test.tsx | 120 ++-- .../Settings/LlmConnectionSection.tsx | 93 +-- .../SystemNotices/SystemNoticeModal.tsx | 16 +- .../Vacay/VacaySettings.calendars.test.tsx | 82 ++- client/src/components/Vacay/VacaySettings.tsx | 55 +- client/src/components/shared/Modal.test.tsx | 16 + client/src/components/shared/Modal.tsx | 20 +- client/src/index.css | 24 + client/src/mobile/MobileShell.tsx | 29 +- client/src/mobile/components/MBottomNav.tsx | 12 +- client/src/mobile/components/MSheet.tsx | 22 +- client/src/mobile/screens/admin/MAdmin.tsx | 4 +- client/src/mobile/screens/atlas/MAtlas.tsx | 6 +- .../screens/atlas/MAtlasCountryPopup.test.tsx | 59 +- .../screens/atlas/MAtlasCountryPopup.tsx | 20 +- .../screens/collections/MCollAddSheet.tsx | 18 +- .../screens/collections/MCollPlaceSheet.tsx | 10 +- .../mobile/screens/dashboard/MDashboard.tsx | 4 +- .../src/mobile/screens/journey/MJourney.tsx | 5 +- .../mobile/screens/journey/MJourneyDetail.tsx | 86 ++- .../screens/journey/MJourneyEntrySheet.tsx | 213 ++++++- .../settings/MLlmConnectionSection.tsx | 73 +-- .../screens/trip/tabs/MFileLightbox.tsx | 7 +- client/src/mobile/screens/vacay/MVacay.tsx | 23 +- .../src/mobile/screens/vacay/MVacayMonth.tsx | 1 + .../screens/vacay/MVacaySettingsSheet.tsx | 50 +- client/src/mobile/screens/vacay/useMVacay.ts | 25 +- .../src/mobile/screens/vacay/vacayDayModel.ts | 19 +- client/src/pages/AtlasPage.tsx | 12 +- client/src/pages/AtlasPage.wiring.test.tsx | 20 + client/src/pages/JourneyDetailPage.tsx | 80 ++- .../pages/JourneyDetailPage.wiring.test.tsx | 54 +- client/src/pages/atlas/atlasModel.test.ts | 58 ++ client/src/pages/atlas/atlasModel.ts | 38 ++ client/src/pages/atlas/useAtlas.test.tsx | 37 ++ client/src/pages/atlas/useAtlas.ts | 24 +- .../pages/journeyDetail/useJourneyDetail.ts | 8 +- client/src/store/collectionStore.test.ts | 33 +- client/src/store/collectionStore.ts | 85 ++- client/src/utils/bodyScrollLock.test.ts | 58 ++ client/src/utils/bodyScrollLock.ts | 43 ++ client/tests/unit/mobile/MBottomNav.test.tsx | 63 ++ client/tests/unit/mobile/MobileShell.test.tsx | 60 ++ .../tests/unit/mobile/admin/MAdmin.test.tsx | 11 + .../tests/unit/mobile/atlas/MAtlas.test.tsx | 13 +- .../mobile/collections/MCollAddSheet.test.tsx | 42 +- .../collections/MCollPlaceSheet.test.tsx | 44 +- .../unit/mobile/dashboard/MDashboard.test.tsx | 15 +- .../unit/mobile/journey/MJourney.test.tsx | 12 +- .../mobile/journey/MJourneyDetail.test.tsx | 92 ++- .../journey/MJourneyEntrySheet.test.tsx | 169 +++++- .../settings/MLlmConnectionSection.test.tsx | 118 +++- .../tests/unit/mobile/vacay/MVacay.test.tsx | 23 +- .../mobile/vacay/MVacaySettingsSheet.test.tsx | 84 ++- .../unit/mobile/vacay/useMVacay.test.tsx | 47 +- .../unit/mobile/vacay/vacayDayModel.test.ts | 52 +- client/tests/unit/sync/tilePrefetcher.test.ts | 42 ++ client/vite.config.js | 35 +- .../accommodations.controller.ts | 10 +- .../nest/accommodations/accommodations.mcp.ts | 8 +- .../nest/accommodations/accommodations.rpc.ts | 6 +- .../accommodations/accommodations.service.ts | 51 +- server/src/nest/atlas/atlas.controller.ts | 29 +- server/src/nest/atlas/atlas.mcp.ts | 34 +- server/src/nest/atlas/atlas.rpc.ts | 11 +- server/src/nest/atlas/atlas.service.ts | 96 ++- server/src/nest/budget/budget.service.ts | 134 +++-- server/src/nest/calendar/calendar.service.ts | 30 +- .../src/nest/collections/collections.mcp.ts | 2 +- .../nest/collections/collections.service.ts | 1 + .../src/nest/llm-parse/llm-config.resolver.ts | 28 +- .../src/nest/reservations/reservations.mcp.ts | 314 +++++++++- .../src/nest/settings/settings.controller.ts | 29 +- server/src/nest/settings/settings.service.ts | 17 + server/tests/e2e/collections.e2e.test.ts | 28 + server/tests/e2e/settings.e2e.test.ts | 71 +++ server/tests/integration/atlas.test.ts | 68 +++ .../unit/mcp/tools-atlas-expanded.test.ts | 16 + server/tests/unit/mcp/tools-atlas.test.ts | 30 + .../tests/unit/mcp/tools-transports.test.ts | 565 +++++++++++++++++- .../nest/accommodations.controller.test.ts | 18 +- .../unit/nest/accommodations.service.test.ts | 60 +- .../tests/unit/nest/atlas.controller.test.ts | 21 + server/tests/unit/nest/atlas.service.test.ts | 119 +++- .../unit/nest/budget.service.calc.test.ts | 120 +++- .../tests/unit/nest/budget.service.db.test.ts | 45 ++ .../tests/unit/nest/calendar.service.test.ts | 91 +++ .../unit/nest/collections.service.test.ts | 19 + .../llm-parse/llm-config.resolver.test.ts | 81 ++- .../plugins/addon-domains-errors.rpc.test.ts | 9 + .../unit/reservations/bookings.rpc.test.ts | 7 +- .../src/collection/collection.schema.spec.ts | 22 + shared/src/collection/collection.schema.ts | 4 + shared/src/i18n/ar/atlas.ts | 1 + shared/src/i18n/ar/settings.ts | 4 +- shared/src/i18n/ar/vacay.ts | 2 +- shared/src/i18n/br/atlas.ts | 1 + shared/src/i18n/br/settings.ts | 4 +- shared/src/i18n/br/vacay.ts | 2 +- shared/src/i18n/ca/atlas.ts | 1 + shared/src/i18n/ca/settings.ts | 2 + shared/src/i18n/ca/vacay.ts | 2 +- shared/src/i18n/cs/atlas.ts | 1 + shared/src/i18n/cs/settings.ts | 4 +- shared/src/i18n/cs/vacay.ts | 2 +- shared/src/i18n/de/atlas.ts | 1 + shared/src/i18n/de/settings.ts | 4 +- shared/src/i18n/de/vacay.ts | 2 +- shared/src/i18n/en/atlas.ts | 1 + shared/src/i18n/en/settings.ts | 4 +- shared/src/i18n/en/vacay.ts | 2 +- shared/src/i18n/es/atlas.ts | 1 + shared/src/i18n/es/settings.ts | 4 +- shared/src/i18n/es/vacay.ts | 2 +- shared/src/i18n/fr/atlas.ts | 1 + shared/src/i18n/fr/settings.ts | 4 +- shared/src/i18n/fr/vacay.ts | 2 +- shared/src/i18n/gr/atlas.ts | 1 + shared/src/i18n/gr/settings.ts | 4 +- shared/src/i18n/gr/vacay.ts | 2 +- shared/src/i18n/hu/atlas.ts | 1 + shared/src/i18n/hu/settings.ts | 4 +- shared/src/i18n/hu/vacay.ts | 2 +- shared/src/i18n/id/atlas.ts | 1 + shared/src/i18n/id/settings.ts | 4 +- shared/src/i18n/id/vacay.ts | 2 +- shared/src/i18n/it/atlas.ts | 1 + shared/src/i18n/it/settings.ts | 4 +- shared/src/i18n/it/vacay.ts | 2 +- shared/src/i18n/ja/atlas.ts | 1 + shared/src/i18n/ja/settings.ts | 4 +- shared/src/i18n/ja/vacay.ts | 2 +- shared/src/i18n/ko/atlas.ts | 1 + shared/src/i18n/ko/settings.ts | 4 +- shared/src/i18n/ko/vacay.ts | 2 +- shared/src/i18n/nl/atlas.ts | 1 + shared/src/i18n/nl/settings.ts | 4 +- shared/src/i18n/nl/vacay.ts | 2 +- shared/src/i18n/pl/atlas.ts | 1 + shared/src/i18n/pl/settings.ts | 4 +- shared/src/i18n/pl/vacay.ts | 2 +- shared/src/i18n/ru/atlas.ts | 1 + shared/src/i18n/ru/settings.ts | 4 +- shared/src/i18n/ru/vacay.ts | 2 +- shared/src/i18n/sv/atlas.ts | 1 + shared/src/i18n/sv/settings.ts | 4 +- shared/src/i18n/sv/vacay.ts | 2 +- shared/src/i18n/tr/atlas.ts | 1 + shared/src/i18n/tr/settings.ts | 4 +- shared/src/i18n/tr/vacay.ts | 2 +- shared/src/i18n/uk/atlas.ts | 1 + shared/src/i18n/uk/settings.ts | 4 +- shared/src/i18n/uk/vacay.ts | 2 +- shared/src/i18n/vi/atlas.ts | 1 + shared/src/i18n/vi/settings.ts | 4 +- shared/src/i18n/vi/vacay.ts | 2 +- shared/src/i18n/zh-TW/atlas.ts | 1 + shared/src/i18n/zh-TW/settings.ts | 4 +- shared/src/i18n/zh-TW/vacay.ts | 2 +- shared/src/i18n/zh/atlas.ts | 1 + shared/src/i18n/zh/settings.ts | 5 +- shared/src/i18n/zh/vacay.ts | 2 +- .../reservation/reservation.schema.spec.ts | 50 ++ shared/src/reservation/reservation.schema.ts | 46 ++ wiki/MCP-Addon-Tools.md | 2 +- 176 files changed, 4988 insertions(+), 690 deletions(-) create mode 100644 client/e2e/urlbar-scroll-1809.spec.ts create mode 100644 client/src/utils/bodyScrollLock.test.ts create mode 100644 client/src/utils/bodyScrollLock.ts 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'))}