Skip to content

feat(meshcore): Last Heard shows date when heard before today (#3656)#3663

Merged
Yeraze merged 2 commits into
mainfrom
fix/3656-meshcore-lastheard-date
Jun 23, 2026
Merged

feat(meshcore): Last Heard shows date when heard before today (#3656)#3663
Yeraze merged 2 commits into
mainfrom
fix/3656-meshcore-lastheard-date

Conversation

@Yeraze

@Yeraze Yeraze commented Jun 23, 2026

Copy link
Copy Markdown
Owner

Summary

Closes #3656.

On the MeshCore Nodes list, the Last Heard label always rendered a time (toLocaleTimeString), which is ambiguous for a node last heard on a previous day. Now:

  • Last heard today (local day) → show the time (as before).
  • Last heard before today → show the date, formatted per the user's Date Format setting.

Implementation

  • New formatTimeOrDate(date, timeFormat, dateFormat) in src/utils/datetime.ts — reuses the existing formatTime/formatDate. "Today" is the local calendar day, consistent with how times are otherwise displayed (no server-TZ plumbing — the rest of the UI also renders local time).
  • Wired into MeshCoreNodesView via timeFormat/dateFormat from useSettings.

(formatMessageTime was not reused — it appends a time even to old dates and uses "Yesterday"/day-name forms, which is not this issue's spec.)

Testing

  • datetime.test.ts — today→time (12h/24h), before-today→date across all three Date Formats, and the local-midnight / 23:59-yesterday boundaries.
  • MeshCoreNodesView.test.tsx updated (mock useSettings); full MeshCore node-view suite green (53 tests).
  • tsc clean.

🤖 Generated with Claude Code

@Yeraze @claude
feat(meshcore): Last Heard shows date when a node was last heard befo…
efc7e9e 
…re today (#3656)

On the MeshCore Nodes list, Last Heard always rendered a time
(`toLocaleTimeString`), which is ambiguous for nodes last heard on a prior day.
Now: if last heard today (local day) show the time as before; if before today
show the date, formatted per the user's Date Format setting.

Adds a small `formatTimeOrDate(date, timeFormat, dateFormat)` helper to
datetime.ts (reuses the existing formatTime/formatDate) and wires it into
MeshCoreNodesView via the timeFormat/dateFormat settings. "Today" is the local
calendar day, consistent with how times are otherwise displayed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011JEaCGwY9Wz8jeV4e22GW4
@github-actions

github-actions Bot commented Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown

Claude finished @Yeraze's task —— View job

PR Review

  • Read changed files
  • Review code quality and correctness
  • Check test coverage
  • Post final review

Overall this is a clean, well-scoped fix. The implementation is correct and the logic is easy to follow. A few observations:

src/utils/datetime.ts — formatTimeOrDate

Positive: The "today" check using three separate field comparisons (getFullYear, getMonth, getDate) is the right approach — it avoids DST pitfalls that trip up timestamp-arithmetic comparisons, and it's consistent with the existing isSameDay helper in formatMessageTime.

Minor note — code duplication: formatMessageTime (line 163–166) defines an inline isSameDay helper that does exactly the same three-field comparison used in formatTimeOrDate (lines 76–78). There's a small opportunity to extract this as a shared module-level helper, but it's not a blocker.

src/utils/datetime.test.ts — new formatTimeOrDate suite

Positive: The use of vi.useFakeTimers / vi.setSystemTime is correct — it pins "now" so the today/not-today branch is deterministic. The boundary tests (00:00 today, 23:59 yesterday) are exactly what's needed here.

Observation — future-date case not tested: No test covers a timestamp after "now" (e.g. a node whose clock is slightly ahead). Currently formatTimeOrDate would show the time for any same-calendar-day date, including future times — that's probably the right behaviour, but a test makes the intent explicit and guards against future regressions.

src/components/MeshCore/MeshCoreNodesView.test.tsx

Positive: The useSettings mock is correctly added at line 28–30:

vi.mock('../../contexts/SettingsContext', () => ({
  useSettings: () => ({ timeFormat: '24', dateFormat: 'MM/DD/YYYY' }),
}));

Issue — duplicate describe blocks: The test file contains two identical copies of the MeshCoreNodesView — node-details quick access (#3350) suite (lines 99–137 and lines 189–227) and two copies of MeshCoreNodesView — Discover menu (#3351) (lines 139–187 and lines 229–277). This looks like an accidental paste. The tests will pass (running each scenario twice), but it's noise in the test output and CI time. Should be cleaned up before merge.

Coverage gap: There's no test that actually asserts the formatTimeOrDate output appears in rendered nodes. A snapshot-style or getByText assertion with a frozen clock would confirm the new wiring end-to-end.

src/components/MeshCore/MeshCoreNodesView.tsx

Line 364 — integration is straightforward:

{row.lastHeard && (
  <span>{formatTimeOrDate(new Date(row.lastHeard), timeFormat, dateFormat)}</span>
)}

No concerns here. row.lastHeard is checked for truthiness before constructing the Date, which correctly handles undefined / 0.

Summary
Correctness ✅ Logic is correct; today/before-today boundary is handled properly
Code quality ✅ Minimal, readable, follows existing patterns
Performance ✅ No concerns — two new Date() calls per rendered row at most
Security ✅ No concerns
Test coverage ⚠️ Duplicate describe blocks in the test file should be removed; a future-date test and an integration rendering test would strengthen confidence

The duplicate describe blocks are the only concrete change I'd request before merging.

@Yeraze @claude
test: make formatTimeOrDate midnight assertion ICU-version-tolerant (#…
6a97c20 
…3656)

Node 20's ICU renders 24h local midnight as '24:00' (newer Node gives '00:00'),
which failed the midnight boundary assertion only on Test Suite (20.x). The
intent is to verify midnight-today renders as a TIME (not a date), so match a
time pattern instead of pinning the exact value.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011JEaCGwY9Wz8jeV4e22GW4
@github-actions

github-actions Bot commented Jun 23, 2026 •
edited
Loading

Copy link
Copy Markdown

Claude finished @Yeraze's task —— View job

PR Review

  • Read changed files
  • Review code quality and correctness
  • Check test coverage
  • Post final review

Overall this is a clean, well-scoped fix. The implementation is correct and the logic is easy to follow.

src/utils/datetime.ts — formatTimeOrDate

Correct approach: The three-field comparison (getFullYear, getMonth, getDate) avoids DST pitfalls from timestamp arithmetic and is consistent with the existing isSameDay helper in formatMessageTime.

Minor: code duplication — formatMessageTime (lines 163–166) and getMessageDateSeparator (lines 216–219) both define the same inline isSameDay helper that formatTimeOrDate (lines 75–78) duplicates inline. A small refactor opportunity to extract a shared module-level helper, but not a blocker.

src/utils/datetime.test.ts

Positive: vi.useFakeTimers / vi.setSystemTime is the right approach — it pins "now" so the today/not-today branch is deterministic. The midnight and 23:59 boundary tests are exactly what's needed.

Minor gap: No test covers a timestamp after "now" (node with a slightly-ahead clock). formatTimeOrDate would correctly show the time (same calendar day), but a test makes the intent explicit and guards against future regressions.

src/components/MeshCore/MeshCoreNodesView.test.tsx

Bug found (now fixed): The test file contained two identical copies each of the MeshCoreNodesView — node-details quick access (#3350) and MeshCoreNodesView — Discover menu (#3351) describe blocks (lines 99–137 duplicated at 189–227, and lines 139–187 duplicated at 229–277). This was an accidental paste — the tests passed but ran each scenario twice.

I've committed a fix locally removing the duplicate blocks (commit d0d63f2), but I'm unable to push to this branch due to repository permissions. You can apply the fix manually by deleting lines 189–277 from MeshCoreNodesView.test.tsx.

Missing integration test: There's no test that asserts the formatTimeOrDate output appears in a rendered node row. A getByText assertion with a frozen clock would confirm the wiring end-to-end. Not a blocker, but worth adding.

src/components/MeshCore/MeshCoreNodesView.tsx (line 364)

Integration is clean:

{row.lastHeard && (
  <span>{formatTimeOrDate(new Date(row.lastHeard), timeFormat, dateFormat)}</span>
)}

The truthiness guard on row.lastHeard correctly handles undefined/0.

Summary

Area Status
Correctness ✅ Logic is correct; today/before-today boundary handled properly
Code quality ✅ Minimal, readable, follows existing patterns
Performance ✅ No concerns
Security ✅ No concerns
Test coverage ⚠️ Duplicate describe blocks should be removed (fix committed locally but push blocked by permissions — see above)

View job

@Yeraze Yeraze merged commit 7638cd3 into main Jun 23, 2026
20 checks passed
@Yeraze Yeraze deleted the fix/3656-meshcore-lastheard-date branch June 23, 2026 03:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

MeshCore Nodes: show date instead of time for Last Heard when node was heard before today

1 participant

@Yeraze