Skip to content
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

feat: Centralize and Standardize Date/Time Format Handling #6925

Merged

Conversation

ahmadshaheer
Copy link
Collaborator

@ahmadshaheer ahmadshaheer commented Jan 27, 2025

Summary

  • Created a centralized DATE_TIME_FORMATS constant to maintain all date/time formats in one place
  • Standardized all formats to use 24-hour time format (HH:mm)
  • Added a utility function convert24hTo12h to support 12-hour format when needed
  • Organized formats into logical groups (ISO, UTC, regional standards, etc.)

Related Issues / PR's

Part of https://github.com/SigNoz/engineering-pod/issues/2155

Screenshots

Before:

2025-01-27.10-10-17.mov

After:

2025-01-27.10-13-09.mov

Affected Areas and Manually Tested Areas

  • Conducted overall testing across the app

Important

Centralizes and standardizes date/time format handling using DATE_TIME_FORMATS across the codebase, ensuring consistent format usage.

  • Centralization:
    • Introduces DATE_TIME_FORMATS constant in constants/dateTimeFormats.ts for centralized date/time formats.
    • Adds convert24hTo12h utility function for converting 24-hour formats to 12-hour.
  • Standardization:
    • Updates date/time format usage in CustomTimePicker.tsx, RangePickerModal.tsx, and Graph/index.tsx to use DATE_TIME_FORMATS.
    • Applies DATE_TIME_FORMATS in Logs/ListLogView/index.tsx, Logs/RawLogView/index.tsx, and Logs/TableView/useTableView.tsx.
    • Refactors TraceTable/index.tsx and TraceDetail/index.tsx to use standardized formats.
  • Miscellaneous:
    • Updates useTimezoneFormatter.ts to use DATE_TIME_FORMATS for default format.
    • Adjusts tooltipPlugin.ts and getAxes.ts to incorporate new format constants.
    • Minor updates in IntegrationDetailHeader.tsx and SaveView/index.tsx for format consistency.

This description was created by Ellipsis for a5892ea. It will automatically update as commits are pushed.

@ahmadshaheer ahmadshaheer requested a review from YounixM as a code owner January 27, 2025 05:25
@github-actions github-actions bot added the enhancement New feature or request label Jan 27, 2025
Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Reviewed everything up to 94c9fe3 in 1 minute and 32 seconds

More details
  • Looked at 1155 lines of code in 44 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. frontend/src/utils/timeUtils.ts:1
  • Draft comment:
    The use of centralized DATE_TIME_FORMATS for date formatting is consistent and aligns with the PR's intent to standardize date/time handling. Good job!
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The code in this file is well-structured and follows best practices. The use of constants for date/time formats is consistent with the PR's intent to centralize and standardize date/time handling. The functions are clear and concise, and the use of dayjs for date manipulation is appropriate. No issues found.
2. frontend/src/pages/Integrations/IntegrationDetailPage/IntegrationDetailHeader.tsx:250
  • Draft comment:
    Avoid using inline styles. Consider using CSS classes or styled components instead. This is also applicable in other parts of this file.
  • Reason this comment was not posted:
    Comment was not on a valid diff hunk.
3. frontend/src/pages/SaveView/index.tsx:222
  • Draft comment:
    Avoid using inline styles. Consider using CSS classes or styled components instead. This is also applicable in other parts of this file.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_Alcl02zF1RDR4eeZ


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Incremental review on a5892ea in 34 seconds

More details
  • Looked at 20 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. frontend/src/container/AlertHistory/Timeline/Graph/Graph.tsx:64
  • Draft comment:
    Consider using values: uPlotXAxisValuesFormat for consistency in date/time formatting in other graphs as well.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
2. frontend/src/container/AlertHistory/Timeline/Graph/Graph.tsx:61
  • Draft comment:
    Avoid using inline styles in React components. Use external stylesheets, CSS classes, or styled components instead. This is applicable in other parts of the file as well.
  • Reason this comment was not posted:
    Comment was not on a valid diff hunk.

Workflow ID: wflow_vuSswssMKNiMCHTx


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

1 similar comment
Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

Copy link

Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id>

@ahmadshaheer ahmadshaheer enabled auto-merge (squash) January 27, 2025 11:50
@ahmadshaheer ahmadshaheer merged commit 8d6731c into main Jan 27, 2025
15 of 16 checks passed
@ahmadshaheer ahmadshaheer deleted the centralize-date/time-format-nad-change-to-24-hours-format branch January 27, 2025 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs required enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants