Claude finished @Yeraze's task —— View job

PR Review: fix(meshcore): honor Temperature Unit on the DM telemetry graph

• Gather context
• Read changed files
• Review implementation
• Review test coverage
• Post findings

Summary

This is a clean, minimal, and correct fix. The root cause was clearly identified and the solution mirrors the existing pattern used in MessagesTab.tsx and MeshCoreTelemetryView.tsx.

Implementation (MeshCoreDirectMessagesView.tsx)

No issues. The fix correctly:

• Destructures temperatureUnit and telemetryVisualizationHours from useSettings() (line 67)
• Passes them as temperatureUnit and telemetryHours to <TelemetryGraphs> (lines 422–423)
• Matches the prop names on TelemetryGraphsProps (temperatureUnit?: TemperatureUnit, telemetryHours?: number) — the defaults in TelemetryGraphs are 'C' and 24, so without this fix users with °F or custom hour settings were silently getting wrong values

The added comment (lines 64–67) is clear and references the issue number, which is good for future maintainers.

Test Coverage (MeshCoreDirectMessagesView.test.tsx)

Well done. The key addition is the assertion in the existing 'mounts the TelemetryGraphs component...' test (lines 199–201):

expect(graphs.getAttribute('data-temp-unit')).toBe('F');
expect(graphs.getAttribute('data-telemetry-hours')).toBe('48');

The mock for useSettings returns temperatureUnit: 'F' and telemetryVisualizationHours: 48 (line 27), so these assertions directly verify the regression is fixed. The stub for TelemetryGraphs (lines 39–49) correctly captures both new props.

No new tests were needed beyond extending the existing mount test — this is the right call since the other tests (no-sourceId, prefix-key guard) already cover the gating logic.

Minor Observations (non-blocking)

1. telemetryVisualizationHours default in useSettings mock: The mock returns 48, which is a non-default value. This is deliberate and good — using the default (24) would make the test pass even if the prop were hardcoded. Using 48 ensures the setting is actually wired through.

2. No Fahrenheit default test: There's no test that verifies the absence of temperatureUnit would fall back to 'C' (which is the TelemetryGraphs default). But this is already covered by TelemetryGraphs's own test suite and isn't the responsibility of this component's tests.

3. data-telemetry-hours attribute type: The test asserts .toBe('48') (string) because HTML attributes are strings. The stub uses props.telemetryHours ?? '' which would give '0' if telemetryHours were 0 — not a real-world issue but a subtle edge case in the stub only.

Verdict

✅ Approved. Small, focused, correct fix with appropriate test coverage. No bugs, no security concerns, no performance issues. Ready to merge.