mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-23 15:36:33 +00:00
fix(maps): measure a booking line along the road before hiding it (#2275)
A short transport hop is hidden once its two ends sit closer than a few dozen pixels, so a booking that would draw as a dot does not clutter the map. That check compared the endpoints, which was the whole line while every hop was straight. A car booking is routed along the road now, and a road that leaves a town, rounds a lake and comes back to the next town along can run across half the screen while its ends project sixty pixels apart. The whole drive vanished until the map was zoomed far enough in for the ends to separate, which is the gap in the route the report shows. The decision now walks the line that will actually be drawn, road route or arcs, and keeps the hop as soon as that reaches the floor. A straight two-point hop measures exactly what it measured before, so every existing threshold holds for the case it was written for. One helper serves both renderers, which also folds the two hand-copied floor tables into it.
This commit is contained in:
@@ -18,7 +18,13 @@ const mapMock = vi.hoisted(() => ({
|
||||
on: vi.fn(),
|
||||
off: vi.fn(),
|
||||
panBy: vi.fn(),
|
||||
latLngToContainerPoint: vi.fn(() => ({ x: 0, y: 0, distanceTo: () => 1000 })),
|
||||
// A flat projection, 1000 px per degree: far enough apart that every
|
||||
// booking line clears its declutter floor, which is measured along the
|
||||
// projected line (#2275) rather than read off a canned distanceTo.
|
||||
latLngToContainerPoint: vi.fn(([lat, lng]: [number, number]) => ({
|
||||
x: lng * 1000, y: lat * 1000,
|
||||
distanceTo(other: { x: number; y: number }) { return Math.hypot(lng * 1000 - other.x, lat * 1000 - other.y) },
|
||||
})),
|
||||
// Panes: jsdom has none, so keep them in a map the pane tests can read back.
|
||||
panes: new Map<string, HTMLElement>(),
|
||||
getPane: vi.fn(function (this: void, name: string) { return mapMock.panes.get(name) }),
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
// FE-COMP-RESOVERLAY-001 to FE-COMP-RESOVERLAY-024
|
||||
// FE-COMP-RESOVERLAY-001 to FE-COMP-RESOVERLAY-045
|
||||
import React from 'react'
|
||||
import { describe, it, expect, vi, afterEach } from 'vitest'
|
||||
import { render, screen } from '../../../tests/helpers/render'
|
||||
@@ -389,6 +389,43 @@ describe('ReservationOverlay declutter floors and road routes', () => {
|
||||
expect(JSON.parse(lines()[0].getAttribute('data-points') || '[]')).toEqual([[48, 16], [51.5, 16]])
|
||||
})
|
||||
|
||||
it('FE-COMP-RESOVERLAY-043: a routed drive whose ends project close together still draws (#2275)', () => {
|
||||
// Endpoints 50 px apart, under the 80 px car floor, so the straight hop would
|
||||
// hide. The road between them loops out 0.3° (300 px) and back, which is what
|
||||
// is actually drawn — and what the person was missing on their map.
|
||||
const road: [number, number][] = [[48, 16], [48.3, 16.1], [48.3, 16.3], [48.05, 16]]
|
||||
const { unmount } = renderOverlay({ reservations: [hop('car', 7, 0.05)] })
|
||||
expect(lines()).toHaveLength(0)
|
||||
unmount()
|
||||
|
||||
renderOverlay({ reservations: [hop('car', 7, 0.05)], roadRoutes: new Map([[7, road]]) })
|
||||
expect(lines()).toHaveLength(1)
|
||||
expect(JSON.parse(lines()[0].getAttribute('data-points') || '[]')).toEqual(road)
|
||||
})
|
||||
|
||||
it('FE-COMP-RESOVERLAY-044: a drive with a stop on the way is measured along its legs, not its ends', () => {
|
||||
// From and to sit 20 px apart; the stop is 400 px away, so the two legs
|
||||
// together draw 800 px of line.
|
||||
renderOverlay({
|
||||
reservations: [booking({
|
||||
id: 8, type: 'car',
|
||||
endpoints: [
|
||||
endpoint({ role: 'from', sequence: 0, name: 'Home', lat: 48, lng: 16 }),
|
||||
endpoint({ role: 'stop', sequence: 1, name: 'Lake', lat: 48.4, lng: 16 }),
|
||||
endpoint({ role: 'to', sequence: 2, name: 'Back', lat: 48.02, lng: 16 }),
|
||||
],
|
||||
})],
|
||||
})
|
||||
expect(lines()).toHaveLength(2)
|
||||
})
|
||||
|
||||
it('FE-COMP-RESOVERLAY-045: a routed drive that really is tiny on screen stays decluttered', () => {
|
||||
// Every point of the road projects within 30 px: nothing worth drawing yet.
|
||||
const road: [number, number][] = [[48, 16], [48.01, 16.01], [48.02, 16.02], [48.03, 16]]
|
||||
renderOverlay({ reservations: [hop('car', 7, 0.03)], roadRoutes: new Map([[7, road]]) })
|
||||
expect(lines()).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('FE-COMP-RESOVERLAY-036: a booking without an endpoints array is skipped', () => {
|
||||
renderOverlay({ reservations: [booking({ endpoints: undefined })] })
|
||||
expect(lines()).toHaveLength(0)
|
||||
|
||||
@@ -8,6 +8,7 @@ import { getTransitMapSegments, type TransitMapSegment } from './transitGeometry
|
||||
import { geodesicArcs } from './flightGeodesy'
|
||||
import { cleanEndpointName } from './reservationName'
|
||||
import { useSettingsStore } from '../../store/settingsStore'
|
||||
import { hopIsVisible, labelFloorPx } from '../../utils/reservationRoutes'
|
||||
import type { Reservation, ReservationEndpoint } from '../../types'
|
||||
|
||||
const ENDPOINT_PANE = 'reservation-endpoints'
|
||||
@@ -122,6 +123,12 @@ function computeDuration(from: ReservationEndpoint, to: ReservationEndpoint, fal
|
||||
return h > 0 ? `${h}h ${m}m` : `${m}m`
|
||||
}
|
||||
|
||||
/** What a hop will draw: the road it was routed along when there is one, else its arcs. */
|
||||
function linesFor(item: TransportItem, roadRoutes: Map<number, [number, number][]> | undefined): [number, number][][] {
|
||||
const road = roadRoutes?.get(item.res.id)
|
||||
return road && road.length >= 2 ? [road] : item.arcs
|
||||
}
|
||||
|
||||
interface TransportItem {
|
||||
res: Reservation
|
||||
from: ReservationEndpoint
|
||||
@@ -200,26 +207,25 @@ export default function ReservationOverlay({ reservations, showConnections, onEn
|
||||
}, [reservations])
|
||||
|
||||
const visibleItems = useMemo(() => {
|
||||
const project = (p: readonly [number, number]) => map.latLngToContainerPoint([p[0], p[1]])
|
||||
return items.filter(item => {
|
||||
// A transit journey draws its real rail/bus alignment, not a straight from->to
|
||||
// line, so the endpoint-proximity declutter (which exists to hide tiny no-op
|
||||
// straight connectors) must not suppress it. Otherwise a zoomed-out day — e.g. one
|
||||
// with no other places to tighten the map onto — hides the whole route (#1570).
|
||||
if (item.transitSegs.length > 0) return true
|
||||
const fromPx = map.latLngToContainerPoint([item.from.lat, item.from.lng])
|
||||
const toPx = map.latLngToContainerPoint([item.to.lat, item.to.lng])
|
||||
const minPx = item.type === 'flight' ? 50 : item.type === 'cruise' ? 150 : item.type === 'car' ? 80 : 200
|
||||
return fromPx.distanceTo(toPx) >= minPx
|
||||
// Measured along what is drawn, not between the ends: a routed drive can
|
||||
// cover half the screen while its endpoints sit close together (#2275).
|
||||
return hopIsVisible(item.type, linesFor(item, roadRoutes), project)
|
||||
})
|
||||
}, [items, zoom, map])
|
||||
}, [items, zoom, map, roadRoutes])
|
||||
|
||||
const labelVisibleIds = useMemo(() => {
|
||||
const set = new Set<number>()
|
||||
for (const item of visibleItems) {
|
||||
const fromPx = map.latLngToContainerPoint([item.from.lat, item.from.lng])
|
||||
const toPx = map.latLngToContainerPoint([item.to.lat, item.to.lng])
|
||||
const minPx = item.type === 'flight' ? 50 : item.type === 'cruise' ? 300 : item.type === 'car' ? 150 : item.type === 'transit' ? 900 : 400
|
||||
if (fromPx.distanceTo(toPx) >= minPx) set.add(item.res.id)
|
||||
if (fromPx.distanceTo(toPx) >= labelFloorPx(item.type)) set.add(item.res.id)
|
||||
}
|
||||
return set
|
||||
}, [visibleItems, zoom, map])
|
||||
@@ -248,9 +254,7 @@ export default function ReservationOverlay({ reservations, showConnections, onEn
|
||||
))
|
||||
}
|
||||
// Prefer the real road route (car/bus/taxi/bicycle) over the straight arc.
|
||||
const road = roadRoutes?.get(item.res.id)
|
||||
const lines = road && road.length >= 2 ? [road] : item.arcs
|
||||
return lines.map((seg, segIdx) => (
|
||||
return linesFor(item, roadRoutes).map((seg, segIdx) => (
|
||||
<Polyline
|
||||
key={`line-${item.res.id}-${segIdx}`}
|
||||
positions={seg}
|
||||
|
||||
@@ -613,6 +613,26 @@ describe('ReservationMapboxOverlay declutter floors', () => {
|
||||
attach(wide, labelled).update([hop(2, 'cruise', 0.4)], labelled)
|
||||
expect(glMarkers.map(m => m.el.textContent)).toEqual(['GOA', 'PMI'])
|
||||
})
|
||||
|
||||
it('FE-COMP-RESGL-043: a routed drive whose ends project close together still draws (#2275)', () => {
|
||||
// 50 px between the ends, under the 80 px car floor; the road loops out 300 px.
|
||||
const road: [number, number][] = [[48, 16], [48.3, 16.1], [48.3, 16.3], [48.05, 16]]
|
||||
const bare = freshMap()
|
||||
attach(bare).update([hop(7, 'car', 0.05)], opts)
|
||||
expect(features(bare)).toHaveLength(0)
|
||||
|
||||
const routed = freshMap()
|
||||
attach(routed).update([hop(7, 'car', 0.05)], opts, new Map([[7, road]]))
|
||||
expect(features(routed)).toHaveLength(1)
|
||||
expect(features(routed)[0].geometry.coordinates).toEqual(road.map(([lat, lng]) => [lng, lat]))
|
||||
})
|
||||
|
||||
it('FE-COMP-RESGL-044: a routed drive that really is tiny on screen stays decluttered', () => {
|
||||
const road: [number, number][] = [[48, 16], [48.01, 16.01], [48.02, 16.02], [48.03, 16]]
|
||||
const map = freshMap()
|
||||
attach(map).update([hop(7, 'car', 0.03)], opts, new Map([[7, road]]))
|
||||
expect(features(map)).toHaveLength(0)
|
||||
})
|
||||
})
|
||||
|
||||
describe('ReservationMapboxOverlay waypoint edge cases', () => {
|
||||
|
||||
@@ -13,6 +13,7 @@ import { Plane, Train, Ship, Car, Bus, Sailboat, Bike, CarTaxiFront, Route, Tram
|
||||
import { getTransitMapSegments } from './transitGeometry'
|
||||
import { geodesicArcs } from './flightGeodesy'
|
||||
import { cleanEndpointName } from './reservationName'
|
||||
import { hopIsVisible, labelFloorPx } from '../../utils/reservationRoutes'
|
||||
import { escapeHtml } from '@trek/shared'
|
||||
import type { Reservation, ReservationEndpoint } from '../../types'
|
||||
|
||||
@@ -259,27 +260,31 @@ export class ReservationMapboxOverlay {
|
||||
})
|
||||
}
|
||||
|
||||
/** What a hop will draw: the road it was routed along when there is one, else its arcs. */
|
||||
private linesFor(item: TransportItem): [number, number][][] {
|
||||
const road = this.roadRoutes.get(item.res.id)
|
||||
return road && road.length >= 2 ? [road] : item.arcs
|
||||
}
|
||||
|
||||
private render() {
|
||||
const map = this.map
|
||||
if (!this.map.getSource(RESERVATION_SOURCE_ID)) return
|
||||
|
||||
const show = this.opts.showConnections
|
||||
|
||||
// Visible filter: require the on-screen pixel distance between
|
||||
// endpoints to exceed a type-specific minimum, same as the Leaflet
|
||||
// overlay, so tiny no-op transport lines don't clutter the map.
|
||||
// Visible filter: a hop draws once what it will draw is long enough on
|
||||
// screen to be worth it, same as the Leaflet overlay, so tiny no-op
|
||||
// transport lines don't clutter the map. Measured along the drawn line,
|
||||
// not between the ends: a routed drive can cover half the screen while
|
||||
// its endpoints sit close together (#2275).
|
||||
const project = (p: readonly [number, number]) => map.project([p[1], p[0]])
|
||||
const visibleItems = show ? this.items.filter(item => {
|
||||
try {
|
||||
// A transit journey draws its real alignment, not a straight from->to line, so
|
||||
// don't let the endpoint-proximity declutter hide it when the map is zoomed out
|
||||
// (#1570). Mirrors the Leaflet overlay.
|
||||
if (item.type === 'transit' && getTransitMapSegments(item.res).length > 0) return true
|
||||
const fromPx = map.project([item.from.lng, item.from.lat])
|
||||
const toPx = map.project([item.to.lng, item.to.lat])
|
||||
const dx = fromPx.x - toPx.x, dy = fromPx.y - toPx.y
|
||||
const dist = Math.sqrt(dx * dx + dy * dy)
|
||||
const minPx = item.type === 'flight' ? 50 : item.type === 'cruise' ? 150 : item.type === 'car' ? 80 : 200
|
||||
return dist >= minPx
|
||||
return hopIsVisible(item.type, this.linesFor(item), project)
|
||||
} catch { return true }
|
||||
}) : []
|
||||
|
||||
@@ -293,8 +298,7 @@ export class ReservationMapboxOverlay {
|
||||
const toPx = map.project([item.to.lng, item.to.lat])
|
||||
const dx = fromPx.x - toPx.x, dy = fromPx.y - toPx.y
|
||||
const dist = Math.sqrt(dx * dx + dy * dy)
|
||||
const minPx = item.type === 'flight' ? 50 : item.type === 'cruise' ? 300 : item.type === 'car' ? 150 : item.type === 'transit' ? 900 : 400
|
||||
if (dist >= minPx) labelVisibleIds.add(item.res.id)
|
||||
if (dist >= labelFloorPx(item.type)) labelVisibleIds.add(item.res.id)
|
||||
} catch { /* ignore */ }
|
||||
}
|
||||
}
|
||||
@@ -320,9 +324,7 @@ export class ReservationMapboxOverlay {
|
||||
}))
|
||||
}
|
||||
// Prefer the real road route (car/bus/taxi/bicycle) over the straight arc.
|
||||
const road = this.roadRoutes.get(item.res.id)
|
||||
const lines = road && road.length >= 2 ? [road] : item.arcs
|
||||
return lines.map(seg => ({
|
||||
return this.linesFor(item).map(seg => ({
|
||||
type: 'Feature' as const,
|
||||
properties: {
|
||||
resId: item.res.id,
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import { isRoutableReservation, visibleRouteReservations } from './reservationRoutes'
|
||||
import { hopIsVisible, isRoutableReservation, labelFloorPx, lineFloorPx, visibleRouteReservations } from './reservationRoutes'
|
||||
import type { Day, Reservation, ReservationEndpoint } from '../types'
|
||||
|
||||
function endpoint(role: 'from' | 'to', lat: number, lng: number): ReservationEndpoint {
|
||||
@@ -116,3 +116,41 @@ describe('visibleRouteReservations with a selected day', () => {
|
||||
expect(visibleRouteReservations([r], { visibleConnectionIds: [], showTransitRoutes: true, selectedDayId: null, days })).toEqual([r])
|
||||
})
|
||||
})
|
||||
|
||||
describe('hopIsVisible (#2275)', () => {
|
||||
// A flat projection: one unit of lat or lng is one pixel.
|
||||
const px = (p: readonly [number, number]) => ({ x: p[1], y: p[0] })
|
||||
|
||||
it('keeps the type floors of the old endpoint check', () => {
|
||||
expect(lineFloorPx('car')).toBe(80)
|
||||
expect(lineFloorPx('flight')).toBe(50)
|
||||
expect(lineFloorPx('cruise')).toBe(150)
|
||||
expect(lineFloorPx('train')).toBe(200)
|
||||
expect(labelFloorPx('car')).toBe(150)
|
||||
expect(labelFloorPx('transit')).toBe(900)
|
||||
expect(labelFloorPx('bus')).toBe(400)
|
||||
})
|
||||
|
||||
it('measures a straight two-point line exactly as the endpoint gap', () => {
|
||||
expect(hopIsVisible('car', [[[0, 0], [0, 79]]], px)).toBe(false)
|
||||
expect(hopIsVisible('car', [[[0, 0], [0, 80]]], px)).toBe(true)
|
||||
})
|
||||
|
||||
it('walks a routed line, so a loop between close ends counts its full length', () => {
|
||||
const loop: [number, number][] = [[0, 0], [30, 0], [30, 30], [0, 30], [0, 5]]
|
||||
expect(hopIsVisible('car', [loop], px)).toBe(true)
|
||||
expect(hopIsVisible('train', [loop], px)).toBe(false)
|
||||
})
|
||||
|
||||
it('adds the legs of a multi-stop booking together', () => {
|
||||
const legs: [number, number][][] = [[[0, 0], [0, 50]], [[0, 50], [0, 100]]]
|
||||
expect(hopIsVisible('car', legs, px)).toBe(false)
|
||||
expect(hopIsVisible('car', [[[0, 0], [0, 50], [0, 100]]], px)).toBe(true)
|
||||
})
|
||||
|
||||
it('draws nothing for an empty or one-point line', () => {
|
||||
expect(hopIsVisible('car', [], px)).toBe(false)
|
||||
expect(hopIsVisible('car', [[]], px)).toBe(false)
|
||||
expect(hopIsVisible('car', [[[0, 0]]], px)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -52,3 +52,63 @@ export function visibleRouteReservations(reservations: Reservation[], options: R
|
||||
set.has(r.id)
|
||||
)
|
||||
}
|
||||
|
||||
/** The type-specific floors, in screen pixels, under which a hop is not worth drawing. */
|
||||
const LINE_FLOOR_PX: Record<string, number> = { flight: 50, cruise: 150, car: 80 }
|
||||
const LINE_FLOOR_DEFAULT_PX = 200
|
||||
|
||||
/** Under this floor the endpoint labels stay off, so short hops do not stack text. */
|
||||
const LABEL_FLOOR_PX: Record<string, number> = { flight: 50, cruise: 300, car: 150, transit: 900 }
|
||||
const LABEL_FLOOR_DEFAULT_PX = 400
|
||||
|
||||
/** How far the line is allowed to disappear before its endpoints cannot be told apart. */
|
||||
export function lineFloorPx(type: string): number {
|
||||
return LINE_FLOOR_PX[type] ?? LINE_FLOOR_DEFAULT_PX
|
||||
}
|
||||
|
||||
export function labelFloorPx(type: string): number {
|
||||
return LABEL_FLOOR_PX[type] ?? LABEL_FLOOR_DEFAULT_PX
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether a hop is worth drawing at the current zoom.
|
||||
*
|
||||
* The declutter exists for a tiny straight connector, the kind that would sit
|
||||
* under its own endpoint markers and add nothing but clutter. It used to be
|
||||
* decided from the two endpoints alone, which was fine while every hop was a
|
||||
* straight line: the line was never longer than the gap it bridged.
|
||||
*
|
||||
* A routed car booking is not that line. It follows the road, and a road that
|
||||
* leaves a town, loops round a lake and comes back to the next town along
|
||||
* can run across half the screen while its two ends project a handful of
|
||||
* pixels apart. The old check then hid the whole drive, and the person looking
|
||||
* at their trip saw a gap in the route until they zoomed in far enough for
|
||||
* the endpoints to separate (#2275). A booking with stops on the way has the
|
||||
* same shape: the ends may be close, the drawn path is not.
|
||||
*
|
||||
* So the decision looks at what will actually be drawn. A hop stays hidden
|
||||
* only while every polyline it would draw is shorter on screen than the
|
||||
* floor, walked point to point. A straight two-point line measures the same
|
||||
* as before, so every existing floor holds for the case it was written for.
|
||||
*
|
||||
* `project` is whichever renderer's own projection: Leaflet's
|
||||
* latLngToContainerPoint or Mapbox's project, wrapped to take [lat, lng].
|
||||
*/
|
||||
export function hopIsVisible(
|
||||
type: string,
|
||||
lines: readonly (readonly [number, number][])[],
|
||||
project: (point: readonly [number, number]) => { x: number; y: number },
|
||||
): boolean {
|
||||
const floor = lineFloorPx(type)
|
||||
for (const line of lines) {
|
||||
let length = 0
|
||||
let prev = line.length > 0 ? project(line[0]) : null
|
||||
for (let i = 1; i < line.length && prev; i++) {
|
||||
const next = project(line[i])
|
||||
length += Math.hypot(next.x - prev.x, next.y - prev.y)
|
||||
if (length >= floor) return true
|
||||
prev = next
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user