-
Notifications
You must be signed in to change notification settings - Fork 141
Fix 12-hour time format issue in graphs by implementing custom 24-hour formatter #254
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: main
Are you sure you want to change the base?
Conversation
…r formatter Co-authored-by: abh <[email protected]>
Co-authored-by: abh <[email protected]>
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.
Pull Request Overview
This PR implements a custom 24-hour time formatter to resolve inconsistent time displays in the graphs by replacing D3.js’s default formatter in both server and zone visualizations.
- Improved time formatting logic using a custom formatter for consistent 24-hour display
- Updates made separately in graphs.server.js and graphs.zone.js to address the issue
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
docs/shared/static/js/graphs.zone.js | Added custom24HourFormatter and replaced tickFormat() call |
docs/shared/static/js/graphs.server.js | Added custom24HourFormatter and replaced tickFormat() call |
@@ -57,12 +57,34 @@ function zone_chart(div, data, options) { | |||
.attr("y1", 0) | |||
.attr("y2", h); | |||
|
|||
// Custom 24-hour time formatter to avoid 12-hour format in minute displays |
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.
The custom24HourFormatter function is duplicated in multiple files. Consider extracting it to a shared utility module to reduce redundancy and simplify future maintenance.
Copilot uses AI. Check for mistakes.
@@ -62,12 +62,34 @@ function server_chart(div, data, options) { | |||
.attr("y1", 0) | |||
.attr("y2", h); | |||
|
|||
// Custom 24-hour time formatter to avoid 12-hour format in minute displays |
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.
The custom24HourFormatter function appears to be identically implemented here as in graphs.zone.js. Extracting this common logic to a shared module could improve maintainability and consistency.
Copilot uses AI. Check for mistakes.
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:Solution
Implemented a custom 24-hour time formatter that replaces D3's default
x.tickFormat()
in bothgraphs.server.js
andgraphs.zone.js
:Result
Time displays are now consistent with 24-hour format throughout:
Changes
docs/shared/static/js/graphs.server.js
- Added custom formatter for server graphsdocs/shared/static/js/graphs.zone.js
- Added custom formatter for zone graphsTesting
Fixes #216.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
http://168.63.129.16:80/machine/
/usr/bin/python3 -u bin/WALinuxAgent-2.13.1.1-py3.9.egg -collect-logs
(http block)If you need me to access, download, or install something from one of these locations, you can either:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.