-
-
Notifications
You must be signed in to change notification settings - Fork 5
Fix/trip display #1058
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Fix/trip display #1058
Changes from 7 commits
a910643
230b9ad
5b391a2
719e326
7c16525
830ddbf
d1028de
f14234b
896d70a
ce3482f
7a8299e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,6 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { ArrowDownUp } from 'lucide-react' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useEffect, useRef, useState } from 'react' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useTranslation } from 'react-i18next' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { calculateColorRange } from './color-palette' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import MobileBoxLayer from './mobile-box-layer' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { Button } from '~/components/ui/button' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -26,9 +27,9 @@ export default function MobileBoxView({ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="absolute top-10 right-0 flex flex-col gap-4 p-4"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="absolute top-80 right-0 flex max-h-[calc(100vh-21rem)] flex-col gap-4 overflow-y-auto p-4"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {sensors.map((sensor, index) => ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div key={index} className="flex flex-col items-center gap-4"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div key={sensor.id} className="flex flex-col items-center gap-4"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {index === 1 && sensors.length === 2 && ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <Button | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| className="self-center rounded-full px-4 py-2" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -86,6 +87,7 @@ function Legend({ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sensor: SensorWithColor | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onColorChange?: (min: string, max: string) => void | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { t } = useTranslation('mobile-map') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { lowColor, highColor } = calculateColorRange(sensor.color) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const minColorInputRef = useRef<HTMLInputElement>(null) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -95,16 +97,31 @@ function Legend({ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [maxColor, setMaxColor] = useState(highColor) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onColorChange && onColorChange(minColor, maxColor) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onColorChange?.(minColor, maxColor) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, [minColor, maxColor, onColorChange]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const sensorData = Array.isArray(sensor.data) ? sensor.data : [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const sensorData = Array.isArray(sensor.data) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? sensor.data.filter( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (measurement) => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| measurement.value !== null && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Number.isFinite(Number(measurement.value)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : [] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const minValue = Math.min(...sensorData.map((d) => Number(d.value))) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const maxValue = Math.max(...sensorData.map((d) => Number(d.value))) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const minValue = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sensorData.length > 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? Math.min(...sensorData.map((d) => Number(d.value))) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const maxValue = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sensorData.length > 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? Math.max(...sensorData.map((d) => Number(d.value))) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+103
to
+119
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Calculate legend bounds from the rendered point set.
Proposed fix ? sensor.data.filter(
(measurement) =>
+ measurement.location !== null &&
measurement.value !== null &&
Number.isFinite(Number(measurement.value)),
)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div className="z-50 flex w-40 flex-col gap-2 rounded-lg border-gray-200 bg-white p-2 shadow-xs"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <span className="text-muted-foreground text-[10px] font-medium tracking-wide uppercase"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {t('sensorValues')} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| </span> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <span className="font-semibold">{sensor.title}</span> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <div | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| className="flex w-full items-center justify-between rounded-sm p-1" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,15 +1,12 @@ | ||||||||||||||||||||||||||||||||
| import bbox from '@turf/bbox' | ||||||||||||||||||||||||||||||||
| import { point, featureCollection } from '@turf/helpers' | ||||||||||||||||||||||||||||||||
| import { format } from 'date-fns' | ||||||||||||||||||||||||||||||||
| import { type FeatureCollection, type Point } from 'geojson' | ||||||||||||||||||||||||||||||||
| import { CalendarClock } from 'lucide-react' | ||||||||||||||||||||||||||||||||
| import { useState, useEffect, useMemo, useCallback } from 'react' | ||||||||||||||||||||||||||||||||
| import { useTranslation } from 'react-i18next' | ||||||||||||||||||||||||||||||||
| import { Source, Layer, useMap, Popup } from 'react-map-gl/maplibre' | ||||||||||||||||||||||||||||||||
| import MapLegend from './mobile-overview-legend' | ||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||
| type LocationPoint, | ||||||||||||||||||||||||||||||||
| categorizeIntoTrips, | ||||||||||||||||||||||||||||||||
| } from '~/lib/mobile-box-helper' | ||||||||||||||||||||||||||||||||
| import { type LocationPoint, getLatestTrips } from '~/lib/mobile-box-helper' | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| const FIT_PADDING = 100 | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
|
@@ -121,8 +118,17 @@ export default function MobileOverviewLayer({ | |||||||||||||||||||||||||||||||
| }: { | ||||||||||||||||||||||||||||||||
| locations: LocationPoint[] | ||||||||||||||||||||||||||||||||
| }) { | ||||||||||||||||||||||||||||||||
| // Generate trips and assign colors once | ||||||||||||||||||||||||||||||||
| const trips = useMemo(() => categorizeIntoTrips(locations, 50), [locations]) | ||||||||||||||||||||||||||||||||
| const { i18n } = useTranslation('mobile-map') | ||||||||||||||||||||||||||||||||
| const tripDateTimeFormatter = useMemo( | ||||||||||||||||||||||||||||||||
| () => | ||||||||||||||||||||||||||||||||
| new Intl.DateTimeFormat(i18n.language, { | ||||||||||||||||||||||||||||||||
| dateStyle: 'medium', | ||||||||||||||||||||||||||||||||
| timeStyle: 'short', | ||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||
| [i18n.language], | ||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||
| // Apply the same trip definition and limit as the server-side overview loader. | ||||||||||||||||||||||||||||||||
| const trips = useMemo(() => getLatestTrips(locations), [locations]) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Cluster points within each trip | ||||||||||||||||||||||||||||||||
| const clusteredTrips = useMemo(() => { | ||||||||||||||||||||||||||||||||
|
|
@@ -176,7 +182,7 @@ export default function MobileOverviewLayer({ | |||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Legend items state | ||||||||||||||||||||||||||||||||
| const [legendItems, setLegendItems] = useState< | ||||||||||||||||||||||||||||||||
| { label: string; color: string }[] | ||||||||||||||||||||||||||||||||
| { label: string; color: string; isLatest: boolean }[] | ||||||||||||||||||||||||||||||||
| >([]) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // State to track the highlighted trip number | ||||||||||||||||||||||||||||||||
|
|
@@ -198,7 +204,12 @@ export default function MobileOverviewLayer({ | |||||||||||||||||||||||||||||||
| const [showOriginalColors, setShowOriginalColors] = useState(true) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||||||||
| if (!clusteredTrips || clusteredTrips.length === 0) return | ||||||||||||||||||||||||||||||||
| if (clusteredTrips.length === 0) { | ||||||||||||||||||||||||||||||||
| setSourceData(null) | ||||||||||||||||||||||||||||||||
| setExpandedSourceData(null) | ||||||||||||||||||||||||||||||||
| setLegendItems([]) | ||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
Comment on lines
+211
to
+217
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Clear interaction state with empty map data. Line 207 clears the map sources but retains Proposed fix if (clusteredTrips.length === 0) {
setSourceData(null)
setExpandedSourceData(null)
setLegendItems([])
+ setHighlightedTrip(null)
+ setHoveredCluster(null)
+ setPopupInfo(null)
return
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| const colors = generateColors(clusteredTrips.length) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
|
@@ -235,15 +246,20 @@ export default function MobileOverviewLayer({ | |||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Set legend items for the trips | ||||||||||||||||||||||||||||||||
| const legend = clusteredTrips.map((_, index) => ({ | ||||||||||||||||||||||||||||||||
| label: `Trip ${index + 1}`, | ||||||||||||||||||||||||||||||||
| const legend = clusteredTrips.map((trip, index) => ({ | ||||||||||||||||||||||||||||||||
| label: formatTripTimeRange( | ||||||||||||||||||||||||||||||||
| trip.startTime, | ||||||||||||||||||||||||||||||||
| trip.endTime, | ||||||||||||||||||||||||||||||||
| tripDateTimeFormatter, | ||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||
| color: colors[index], | ||||||||||||||||||||||||||||||||
| isLatest: index === clusteredTrips.length - 1, | ||||||||||||||||||||||||||||||||
| })) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| setSourceData(featureCollection(points)) | ||||||||||||||||||||||||||||||||
| setExpandedSourceData(featureCollection(expandedPoints)) | ||||||||||||||||||||||||||||||||
| setLegendItems(legend) | ||||||||||||||||||||||||||||||||
| }, [clusteredTrips]) | ||||||||||||||||||||||||||||||||
| }, [clusteredTrips, tripDateTimeFormatter]) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||||||||
| if (!mapRef || !sourceData) return | ||||||||||||||||||||||||||||||||
|
|
@@ -451,7 +467,7 @@ export default function MobileOverviewLayer({ | |||||||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||||||||
| <div> | ||||||||||||||||||||||||||||||||
| <p className="text-primary text-sm font-bold"> | ||||||||||||||||||||||||||||||||
| {format(new Date(popupInfo.startTime), 'Pp')} | ||||||||||||||||||||||||||||||||
| {tripDateTimeFormatter.format(new Date(popupInfo.startTime))} | ||||||||||||||||||||||||||||||||
| </p> | ||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||
| {popupInfo.isCluster && | ||||||||||||||||||||||||||||||||
|
|
@@ -461,7 +477,7 @@ export default function MobileOverviewLayer({ | |||||||||||||||||||||||||||||||
| To | ||||||||||||||||||||||||||||||||
| </span> | ||||||||||||||||||||||||||||||||
| <p className="text-primary text-sm font-bold"> | ||||||||||||||||||||||||||||||||
| {format(new Date(popupInfo.endTime), 'Pp')} | ||||||||||||||||||||||||||||||||
| {tripDateTimeFormatter.format(new Date(popupInfo.endTime))} | ||||||||||||||||||||||||||||||||
| </p> | ||||||||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||||||||
|
|
@@ -472,7 +488,7 @@ export default function MobileOverviewLayer({ | |||||||||||||||||||||||||||||||
| <MapLegend | ||||||||||||||||||||||||||||||||
| items={legendItems} | ||||||||||||||||||||||||||||||||
| position="top-right" | ||||||||||||||||||||||||||||||||
| toggleTrips={() => setShowOriginalColors(!showOriginalColors)} | ||||||||||||||||||||||||||||||||
| onColorByTripChange={setShowOriginalColors} | ||||||||||||||||||||||||||||||||
| showOriginalColors={showOriginalColors} | ||||||||||||||||||||||||||||||||
| onLegendItemHover={(color) => { | ||||||||||||||||||||||||||||||||
| setHighlightedTrip( | ||||||||||||||||||||||||||||||||
|
|
@@ -485,3 +501,11 @@ export default function MobileOverviewLayer({ | |||||||||||||||||||||||||||||||
| </> | ||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| function formatTripTimeRange( | ||||||||||||||||||||||||||||||||
| startTime: string, | ||||||||||||||||||||||||||||||||
| endTime: string, | ||||||||||||||||||||||||||||||||
| dateTimeFormatter: Intl.DateTimeFormat, | ||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||
| return dateTimeFormatter.formatRange(new Date(startTime), new Date(endTime)) | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate coordinates before creating point features.
The filter accepts a non-null location with
NaNor infinite coordinates. Those values create invalid GeoJSON positions and can prevent MapLibre from rendering the source. Filter both coordinates withNumber.isFinite.Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents