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[];