[PR #254] Fix 12-hour time format issue in graphs by implementing custom 24-hour formatter #822

Open
opened 2026-03-13 15:14:26 +03:00 by kerem · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/abh/ntppool/pull/254
Author: @Copilot
Created: 6/4/2025
Status: 🔄 Open

Base: mainHead: copilot/fix-216


📝 Commits (4)

  • 1e6ed4c Initial plan for issue
  • ededda3 Fix 12-hour time format issue in graphs by implementing custom 24-hour formatter
  • 3da5c13 Update time formatter to ES5-compatible syntax for older browser support
  • dcb9ae2 Merge main branch and apply 24-hour time fix to TypeScript zone-chart

📊 Changes

1 file changed (+23 additions, -1 deletions)

View changed files

📝 client/src/charts/zone-chart.ts (+23 -1)

📄 Description

This PR fixes the confusing time display issue in graphs where minutes were shown in 12-hour format without AM/PM indicators, while hours were correctly displayed in 24-hour format.

Problem

The issue occurred when D3.js default tickFormat() method mixed 12-hour and 24-hour time formats, creating confusing sequences like:

  • "8:45", "21", "9:15" (mixing 12-hour minutes with 24-hour hours)
  • "08:45", "09 PM", "09:15" (inconsistent format)

Solution

Implemented a custom 24-hour time formatter that replaces D3's default tickFormat() in client/src/charts/zone-chart.ts:

function format24HourDate(date: Date): string {
  if (d3.utcSecond(date) < date) return formatMillisecond(date);
  if (d3.utcMinute(date) < date) return formatSecond(date);
  if (d3.utcHour(date) < date) return formatMinute24(date);  // %H:%M — always 24-hour
  if (d3.utcDay(date) < date) return formatHour24(date);    // %H — always 24-hour
  if (d3.utcMonth(date) < date) return d3.utcWeek(date) < date ? formatDay(date) : formatWeek(date);
  if (d3.utcYear(date) < date) return formatMonth(date);
  return formatYear(date);
}

server-chart.ts already used explicit d3.utcFormat('%H:%M') specifiers via its createDateFormatter function.

Result

Time displays are now consistent with 24-hour format throughout:

  • "08:45", "21", "09:15" (consistent 24-hour format)
  • "20:45", "21", "21:15" (proper progression)

Changes

  • Modified: client/src/charts/zone-chart.ts — Added format24HourDate() helper and replaced xScale.tickFormat() call with it

Note

: The chart code was migrated from docs/shared/static/js/graphs.server.js and docs/shared/static/js/graphs.zone.js to TypeScript (client/src/charts/) in the main branch. This PR targets the TypeScript implementation accordingly.

Testing

  • TypeScript compiles without errors
  • Tested problematic time sequences that caused the original issue
  • Confirmed fix works for both server and zone graph views
  • Maintains all existing graph functionality

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/abh/ntppool/pull/254 **Author:** [@Copilot](https://github.com/apps/copilot-swe-agent) **Created:** 6/4/2025 **Status:** 🔄 Open **Base:** `main` ← **Head:** `copilot/fix-216` --- ### 📝 Commits (4) - [`1e6ed4c`](https://github.com/abh/ntppool/commit/1e6ed4c8289b330f59ea59f793b29a9735500484) Initial plan for issue - [`ededda3`](https://github.com/abh/ntppool/commit/ededda30112a617befe84cf698859f2cef72d4fa) Fix 12-hour time format issue in graphs by implementing custom 24-hour formatter - [`3da5c13`](https://github.com/abh/ntppool/commit/3da5c132fb7e1aaeccff19954e90e635318c621e) Update time formatter to ES5-compatible syntax for older browser support - [`dcb9ae2`](https://github.com/abh/ntppool/commit/dcb9ae2e9cf3c23770c15314d515eda5f64369b9) Merge main branch and apply 24-hour time fix to TypeScript zone-chart ### 📊 Changes **1 file changed** (+23 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `client/src/charts/zone-chart.ts` (+23 -1) </details> ### 📄 Description This PR fixes the confusing time display issue in graphs where minutes were shown in 12-hour format without AM/PM indicators, while hours were correctly displayed in 24-hour format. ## Problem The issue occurred when D3.js default `tickFormat()` method mixed 12-hour and 24-hour time formats, creating confusing sequences like: - "8:45", "21", "9:15" (mixing 12-hour minutes with 24-hour hours) - "08:45", "09 PM", "09:15" (inconsistent format) ## Solution Implemented a custom 24-hour time formatter that replaces D3's default `tickFormat()` in `client/src/charts/zone-chart.ts`: ```typescript function format24HourDate(date: Date): string { if (d3.utcSecond(date) < date) return formatMillisecond(date); if (d3.utcMinute(date) < date) return formatSecond(date); if (d3.utcHour(date) < date) return formatMinute24(date); // %H:%M — always 24-hour if (d3.utcDay(date) < date) return formatHour24(date); // %H — always 24-hour if (d3.utcMonth(date) < date) return d3.utcWeek(date) < date ? formatDay(date) : formatWeek(date); if (d3.utcYear(date) < date) return formatMonth(date); return formatYear(date); } ``` `server-chart.ts` already used explicit `d3.utcFormat('%H:%M')` specifiers via its `createDateFormatter` function. ## Result Time displays are now consistent with 24-hour format throughout: - ✅ "08:45", "21", "09:15" (consistent 24-hour format) - ✅ "20:45", "21", "21:15" (proper progression) ## Changes - **Modified**: `client/src/charts/zone-chart.ts` — Added `format24HourDate()` helper and replaced `xScale.tickFormat()` call with it > **Note**: The chart code was migrated from `docs/shared/static/js/graphs.server.js` and `docs/shared/static/js/graphs.zone.js` to TypeScript (`client/src/charts/`) in the main branch. This PR targets the TypeScript implementation accordingly. ## Testing - ✅ TypeScript compiles without errors - ✅ Tested problematic time sequences that caused the original issue - ✅ Confirmed fix works for both server and zone graph views - ✅ Maintains all existing graph functionality <!-- START COPILOT CODING AGENT TIPS --> --- 🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. [Learn more about Advanced Security.](https://gh.io/cca-advanced-security) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/ntppool#822
No description provided.