Custom page for unveiling (theme and styling)#217
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Code Review
This pull request introduces a "Realtime Showcase" dashboard for live vehicle telemetry, featuring metrics for battery health, driver inputs, suspension, and thermal distribution. Key feedback includes aggregating connection status across all Kafka topics to prevent false offline reports, updating the toFiniteNumber utility to correctly handle null values, and using a state-driven interval for the "last seen" timer. Additionally, minor indentation corrections were suggested for the navigation components.
| const { data: dashboard, kafkaConnected, lastMessageAt } = useKafkaJSON<DashboardData>({ | ||
| topic: "dashboard_screen", | ||
| ...streamOpts, | ||
| }); | ||
| const { data: liveBanner } = useKafkaJSON<LiveBannerData>({ | ||
| topic: "live_banner", | ||
| ...streamOpts, | ||
| }); | ||
| const { data: mapData } = useKafkaJSON<MapData>({ | ||
| topic: "map", | ||
| ...streamOpts, | ||
| }); | ||
| const { data: energy } = useKafkaJSON<EnergyBudgetData>({ | ||
| topic: "energy_budget", | ||
| ...streamOpts, | ||
| }); | ||
| const { data: sensorData } = useKafkaJSON<SensorData>({ | ||
| topic: "sensor_data", | ||
| ...streamOpts, | ||
| }); |
There was a problem hiding this comment.
The connection status (kafkaConnected) and last message timestamp (lastMessageAt) are currently only derived from the dashboard_screen topic. If other topics (e.g., sensor_data, map, energy_budget) are receiving data while dashboard_screen is idle, the UI will incorrectly report the vehicle as offline or stale.
Consider aggregating these states across all active hooks (e.g., using Math.max for timestamps and a logical OR for connection status) to provide a more accurate overall connection state for the vehicle.
| function toFiniteNumber(value: unknown): number | undefined { | ||
| const n = typeof value === "number" ? value : Number(value); | ||
| return Number.isFinite(n) ? n : undefined; | ||
| } |
There was a problem hiding this comment.
The toFiniteNumber function currently converts null to 0 because Number(null) is 0. In telemetry data, null often represents missing or unavailable data rather than a literal zero. This can lead to misleading UI states, such as showing 0% battery or 0 mph speed when the data is actually absent. It is safer to return undefined for null or undefined inputs.
| function toFiniteNumber(value: unknown): number | undefined { | |
| const n = typeof value === "number" ? value : Number(value); | |
| return Number.isFinite(n) ? n : undefined; | |
| } | |
| function toFiniteNumber(value: unknown): number | undefined { | |
| if (value === null || value === undefined) return undefined; | |
| const n = typeof value === "number" ? value : Number(value); | |
| return Number.isFinite(n) ? n : undefined; | |
| } |
| const lastSeenSeconds = | ||
| typeof lastMessageAt === "number" ? Math.max(0, Math.floor((Date.now() - lastMessageAt) / 1000)) : undefined; |
There was a problem hiding this comment.
The lastSeenSeconds value is calculated during render. Since it depends on Date.now(), the 'seconds ago' display will not increment in real-time unless a new message arrives or another state change triggers a re-render. To provide a ticking counter, you should use a local state for the current time that updates via setInterval within a useEffect hook.
| <DropdownMenuContent> | ||
| <DropdownMenuItem asChild><Link href="/">Home</Link></DropdownMenuItem> | ||
| <DropdownMenuItem asChild><Link href="/driveday">Driveday Page</Link></DropdownMenuItem> | ||
| <DropdownMenuItem asChild><Link href="/tune">Texas Tune</Link></DropdownMenuItem> | ||
| <DropdownMenuItem asChild><Link href="/dashboards">Grafana</Link></DropdownMenuItem> | ||
| <DropdownMenuItem asChild><Link href="/live-viewer">Live Viewer</Link></DropdownMenuItem> | ||
| <DropdownMenuItem asChild><Link href="/realtime-showcase">Realtime Showcase</Link></DropdownMenuItem> | ||
| <DropdownMenuItem asChild><Link href="/replay">Replay</Link></DropdownMenuItem> | ||
| <DropdownMenuItem onClick={handleSignOut}>Logout</DropdownMenuItem> | ||
| </DropdownMenuContent> |
There was a problem hiding this comment.
The indentation for DropdownMenuContent and its children has been increased by 2 spaces unnecessarily. It should be aligned with the DropdownMenuTrigger at 10 spaces of indentation to maintain consistency with the rest of the file.
<DropdownMenuContent>
<DropdownMenuItem asChild><Link href="/">Home</Link></DropdownMenuItem>
<DropdownMenuItem asChild><Link href="/driveday">Driveday Page</Link></DropdownMenuItem>
<DropdownMenuItem asChild><Link href="/tune">Texas Tune</Link></DropdownMenuItem>
<DropdownMenuItem asChild><Link href="/dashboards">Grafana</Link></DropdownMenuItem>
<DropdownMenuItem asChild><Link href="/live-viewer">Live Viewer</Link></DropdownMenuItem>
<DropdownMenuItem asChild><Link href="/realtime-showcase">Realtime Showcase</Link></DropdownMenuItem>
<DropdownMenuItem asChild><Link href="/replay">Replay</Link></DropdownMenuItem>
<DropdownMenuItem onClick={handleSignOut">Logout</DropdownMenuItem>
</DropdownMenuContent>
| <DropdownMenuContent className="z-[1001]"> | ||
| <DropdownMenuItem asChild><Link href="/">Home</Link></DropdownMenuItem> | ||
| <DropdownMenuItem asChild><Link href="/driveday">Driveday Page</Link></DropdownMenuItem> | ||
| <DropdownMenuItem asChild><Link href="/tune">Texas Tune</Link></DropdownMenuItem> | ||
| <DropdownMenuItem asChild><Link href="/dashboards">Grafana</Link></DropdownMenuItem> | ||
| <DropdownMenuItem asChild><Link href="/live-viewer">Live Viewer</Link></DropdownMenuItem> | ||
| <DropdownMenuItem asChild><Link href="/realtime-showcase">Realtime Showcase</Link></DropdownMenuItem> | ||
| <DropdownMenuItem asChild><Link href="/replay">Replay</Link></DropdownMenuItem> | ||
| <DropdownMenuItem onClick={handleSignOut}>Logout</DropdownMenuItem> | ||
| </DropdownMenuContent> |
There was a problem hiding this comment.
The indentation for DropdownMenuContent and its children has been increased by 2 spaces unnecessarily. It should be aligned with the DropdownMenuTrigger at 12 spaces of indentation to maintain consistency with the rest of the file.
<DropdownMenuContent className="z-[1001]">
<DropdownMenuItem asChild><Link href="/">Home</Link></DropdownMenuItem>
<DropdownMenuItem asChild><Link href="/driveday">Driveday Page</Link></DropdownMenuItem>
<DropdownMenuItem asChild><Link href="/tune">Texas Tune</Link></DropdownMenuItem>
<DropdownMenuItem asChild><Link href="/dashboards">Grafana</Link></DropdownMenuItem>
<DropdownMenuItem asChild><Link href="/live-viewer">Live Viewer</Link></DropdownMenuItem>
<DropdownMenuItem asChild><Link href="/realtime-showcase">Realtime Showcase</Link></DropdownMenuItem>
<DropdownMenuItem asChild><Link href="/replay">Replay</Link></DropdownMenuItem>
<DropdownMenuItem onClick={handleSignOut">Logout</DropdownMenuItem>
</DropdownMenuContent>
No description provided.