From c0ff4dabb388a30b79daa5380a97fa2ff6d12542 Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 17 Sep 2026 13:48:18 +0200 Subject: [PATCH] fix(roadtrip): a phone stage no longer draws the drive into the next day "Connect the days" is a whole-drive setting: with it on, the leg from one day's last stop to the next day's first is drawn in the colour of the day it leaves, so the desktop map reads as one continuous drive across every day. The phone has no such view by default. It shows ONE day, and the stage filter went by day number alone, so that connection came along: a line running off the day on screen to a place it never visits, and on this trip the longest thing on the map by far. The assembler now says which lines are connections (`lineJoins`, parallel to `lines` and `lineDays`) rather than leaving each surface to guess, because the day number cannot tell them apart: it reads the leaving day for both a day's own legs and for the drive out of it. A stage drops them; the all-days view keeps them, which is the view the setting is for and is one tap away on the same map, next to the day chips. Routes read back from an older cache carry no marks, so nothing is known to be a join and nothing is dropped: those days draw exactly as they did before. --- .../Roadtrip/RoadtripCorridorPanel.test.tsx | 2 +- .../Roadtrip/RoadtripSidebar.test.tsx | 1 + .../src/components/Roadtrip/stageMap.test.ts | 33 +++++++++++++++++++ client/src/components/Roadtrip/stageMap.ts | 11 ++++++- .../Roadtrip/useFollowTrack.test.ts | 2 +- .../Roadtrip/useRoadtripCorridor.test.ts | 2 +- shared/src/roadtrip/assemble.spec.ts | 19 +++++++++++ shared/src/roadtrip/assemble.ts | 9 +++++ shared/src/roadtrip/planning-types.ts | 12 +++++++ 9 files changed, 87 insertions(+), 4 deletions(-) diff --git a/client/src/components/Roadtrip/RoadtripCorridorPanel.test.tsx b/client/src/components/Roadtrip/RoadtripCorridorPanel.test.tsx index 840f2a623..fdf90b226 100644 --- a/client/src/components/Roadtrip/RoadtripCorridorPanel.test.tsx +++ b/client/src/components/Roadtrip/RoadtripCorridorPanel.test.tsx @@ -53,7 +53,7 @@ const day = (dayId: number, dayNumber: number): RoadtripDay => ({ const routes = (days: RoadtripDay[]): RoadtripRoutes => ({ days, - lines: [], lineDays: [], + lines: [], lineDays: [], lineJoins: [], accessLines: [], vias: [], segments: [], diff --git a/client/src/components/Roadtrip/RoadtripSidebar.test.tsx b/client/src/components/Roadtrip/RoadtripSidebar.test.tsx index 5b4c850fa..dbb63d409 100644 --- a/client/src/components/Roadtrip/RoadtripSidebar.test.tsx +++ b/client/src/components/Roadtrip/RoadtripSidebar.test.tsx @@ -67,6 +67,7 @@ function routes(over: Partial = {}): RoadtripRoutes { days: [day()], lines: [], lineDays: [], + lineJoins: [], segments: [], accessLines: [], vias: [], diff --git a/client/src/components/Roadtrip/stageMap.test.ts b/client/src/components/Roadtrip/stageMap.test.ts index 127a149a5..8a63dcd43 100644 --- a/client/src/components/Roadtrip/stageMap.test.ts +++ b/client/src/components/Roadtrip/stageMap.test.ts @@ -80,6 +80,7 @@ function routes(over: Partial = {}): RoadtripRoutes { quietDays: [], lines: LINES, lineDays: [1, 2, 2, 3], + lineJoins: [false, false, false, false], accessLines: [], vias: [], segments: [], @@ -124,6 +125,38 @@ describe('stageMapData with a stage', () => { expect(stageMapData(routes(), stage, true).lines).toEqual([LINES[1], LINES[2]]) }) + it('FE-RTSTAGE-016: the drive into the next day is left off the stage', () => { + // With "connect the days" on, that leg is drawn in the colour of the day it leaves, so + // filtering by day number alone handed the stage a line running off it to a place the + // day never visits. On a phone, where the stage IS the map, it was the longest thing + // on screen and read as the day's own route. + const withJoin = routes({ lineJoins: [false, false, true, false] }) + + expect(stageMapData(withJoin, stage, true).lines).toEqual([LINES[1]]) + }) + + it('FE-RTSTAGE-017: the colours still line up once a join has been dropped', () => { + const data = stageMapData(routes({ lineJoins: [false, false, true, false] }), stage, true) + + expect(data.lineColors).toHaveLength(data.lines.length) + expect(data.lineColors).toEqual([dayColor(2)]) + }) + + it('FE-RTSTAGE-018: the all-days view keeps the joins, which is the view they are for', () => { + // The setting draws the whole drive as one connected line; that is what it is for, and + // on this map the connection has the other days beside it. + expect(stageMapData(routes({ lineJoins: [false, false, true, false] }), null, true).lines).toEqual(LINES) + }) + + it('FE-RTSTAGE-019: routes from before the flag existed are drawn as they were', () => { + // `lineJoins` arrives with this change, and a RoadtripRoutes read back from an older + // cache has none. Without the marks nothing is known to be a join, so nothing is + // dropped: the day is drawn the way it was drawn before. + const legacy = { ...routes(), lineJoins: undefined } as unknown as Parameters[0] + + expect(stageMapData(legacy, stage, true).lines).toEqual([LINES[1], LINES[2]]) + }) + it('FE-RTSTAGE-006: the colours stay lined up with the lines that are left', () => { const data = stageMapData(routes(), stage, true) expect(data.lineColors).toHaveLength(data.lines.length) diff --git a/client/src/components/Roadtrip/stageMap.ts b/client/src/components/Roadtrip/stageMap.ts index 11b53fdae..89079c6bc 100644 --- a/client/src/components/Roadtrip/stageMap.ts +++ b/client/src/components/Roadtrip/stageMap.ts @@ -50,8 +50,17 @@ export function stageMapData( } if (!routes.lines.length) return { ...EMPTY, placeIds: stagePlaceIds(stage), focusPoints: stagePoints(stage) } + // A stage is ONE day, so the drive into the next day is left off it. + // + // With "connect the days" on, that leg is drawn in the colour of the day it leaves, so + // on the whole-drive map it reads as that day carrying on. Filtering by day number + // alone therefore handed a stage a line that runs off it to a place the day never + // visits: on a phone, where the stage IS the map, that read as the day's own route and + // was the longest thing on screen. The desktop has the same connection, but it has it + // beside every other day, which is the view the setting exists for; here that view is + // the "all days" one, a tap away on the same map. const keep: number[] = [] - routes.lineDays.forEach((n, i) => { if (n === stage.dayNumber) keep.push(i) }) + routes.lineDays.forEach((n, i) => { if (n === stage.dayNumber && !routes.lineJoins?.[i]) keep.push(i) }) const placeIds = stagePlaceIds(stage) return { diff --git a/client/src/components/Roadtrip/useFollowTrack.test.ts b/client/src/components/Roadtrip/useFollowTrack.test.ts index f26463f9b..e207e6e30 100644 --- a/client/src/components/Roadtrip/useFollowTrack.test.ts +++ b/client/src/components/Roadtrip/useFollowTrack.test.ts @@ -41,7 +41,7 @@ function day(over: Partial = {}): RoadtripDay { } const routes = (days: RoadtripDay[]): RoadtripRoutes => ({ - days, lines: [], lineDays: [], segments: [], accessLines: [], vias: [], totalDistance: 0, totalDuration: 0, totalStops: 0, quietDays: [], loading: false, + days, lines: [], lineDays: [], lineJoins: [], segments: [], accessLines: [], vias: [], totalDistance: 0, totalDuration: 0, totalStops: 0, quietDays: [], loading: false, }) function viasStub(over: Partial = {}): RoadtripVias { diff --git a/client/src/components/Roadtrip/useRoadtripCorridor.test.ts b/client/src/components/Roadtrip/useRoadtripCorridor.test.ts index 0b1fbc798..9baec57a7 100644 --- a/client/src/components/Roadtrip/useRoadtripCorridor.test.ts +++ b/client/src/components/Roadtrip/useRoadtripCorridor.test.ts @@ -40,7 +40,7 @@ function day(over: Partial = {}): RoadtripDay { } const routes = (days: RoadtripDay[]): RoadtripRoutes => ({ - days, lines: [], lineDays: [], segments: [], accessLines: [], vias: [], totalDistance: 0, totalDuration: 0, totalStops: 0, quietDays: [], loading: false, + days, lines: [], lineDays: [], lineJoins: [], segments: [], accessLines: [], vias: [], totalDistance: 0, totalDuration: 0, totalStops: 0, quietDays: [], loading: false, }) /** The search state the mocked hook hands back, results included. */ diff --git a/shared/src/roadtrip/assemble.spec.ts b/shared/src/roadtrip/assemble.spec.ts index c2a8a90a0..9445f49db 100644 --- a/shared/src/roadtrip/assemble.spec.ts +++ b/shared/src/roadtrip/assemble.spec.ts @@ -162,4 +162,23 @@ describe('assembleRoadtrip connected days', () => { expect(routes.days[1]!.arrivingFrom).toBeUndefined(); expect(routes.days[1]!.geometry[0]).toEqual([60, 10]); }); + + it('ROADTRIP-ASSEMBLE-006: that drive is marked as the connection it is, in the colour of the day it leaves', () => { + const routes = assembleTwoDays(true); + + // Day 1 drives two of its own legs and then on into day 2; day 2 drives one. + expect(routes.lineDays).toEqual([1, 1, 1, 2]); + expect(routes.lines).toHaveLength(routes.lineJoins.length); + // The third line is drawn as day 1 but runs into day 2, and only it is a join. A + // surface showing one day needs that apart from the day number, which says day 1 for + // both the day's own legs and for the drive leading off it. + expect(routes.lineJoins).toEqual([false, false, true, false]); + }); + + it('ROADTRIP-ASSEMBLE-007: with the days unconnected there is no join to mark', () => { + const routes = assembleTwoDays(false); + + expect(routes.lineDays).toEqual([1, 1, 2]); + expect(routes.lineJoins).toEqual([false, false, false]); + }); }); diff --git a/shared/src/roadtrip/assemble.ts b/shared/src/roadtrip/assemble.ts index f2003d9ac..0c282c393 100644 --- a/shared/src/roadtrip/assemble.ts +++ b/shared/src/roadtrip/assemble.ts @@ -99,6 +99,9 @@ export function assembleRoadtrip({ const lines: [number, number][][] = []; const lineDays: number[] = []; + // Parallel to `lines`: true where the line is the drive into the NEXT day, drawn in the + // leaving day's colour. See `lineJoins` on RoadtripRoutes. + const lineJoins: boolean[] = []; const segments: RouteSegment[] = []; const accessLines: RoadtripRoutes['accessLines'] = []; @@ -134,6 +137,10 @@ export function assembleRoadtrip({ if (inbound.line.length > 1) { lines.push(inbound.line); lineDays.push(inbound.drawnAs); + // A spill carries its own `fromDayNumber` and a joined day the number of the day + // it left: either way, a line drawn as a day other than the one it runs in is + // the connection into this one. + lineJoins.push(inbound.drawnAs !== chain.dayNumber); } if (inbound.seg) segments.push(inbound.seg); } @@ -142,6 +149,7 @@ export function assembleRoadtrip({ if (leg.line.length > 1) { lines.push(leg.line); lineDays.push(chain.dayNumber); + lineJoins.push(false); } segments.push(leg.seg); } @@ -288,6 +296,7 @@ export function assembleRoadtrip({ days: drives, lines, lineDays, + lineJoins, segments, accessLines, vias: out.flatMap((d) => d.legVias.flat()), diff --git a/shared/src/roadtrip/planning-types.ts b/shared/src/roadtrip/planning-types.ts index 20ead9100..959923921 100644 --- a/shared/src/roadtrip/planning-types.ts +++ b/shared/src/roadtrip/planning-types.ts @@ -175,6 +175,18 @@ export interface RoadtripRoutes { lineDays: number[]; + /** + * Which of `lines` is the drive INTO the following day rather than a drive within the + * day it is drawn as, parallel to `lines` and `lineDays`. + * + * With "connect the days" on, the leg from one day's last stop to the next day's first + * is drawn in the colour of the day it leaves, so on a map showing the whole drive it + * reads as that day continuing. A surface showing ONE day has to tell the two apart: + * the connection leads off the day on screen and belongs to the next one, and drawn + * there it is a line to a place that day never visits. + */ + lineJoins: boolean[]; + accessLines: AccessSpur[]; vias: RouteVia[];