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: display timezone adjusted time range in time picker #6521

Conversation

ahmadshaheer
Copy link
Collaborator

@ahmadshaheer ahmadshaheer commented Nov 25, 2024

Summary

  • When a time range is selected from any graph, display the timezone adjusted selected range in time picker placeholder
  • Display the timezone adjusted time range in custom time range selected of time picker

Related Issues / PR's

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

Screenshots

Before:

2024-11-25.09-22-49.mov

After:

2024-11-25.09-21-49.mov

Affected Areas and Manually Tested Areas

  • time picker placeholder
  • custom range picker of time picker

Copy link

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

@github-actions github-actions bot added the enhancement New feature or request label Nov 25, 2024
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 b66328e in 30 seconds

More details
  • Looked at 66 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. frontend/src/components/CustomTimePicker/RangePickerModal.tsx:65
  • Draft comment:
    Ensure that the dayjs timezone plugin is loaded before using dayjs.tz. This is necessary for the timezone adjustments to work correctly. This applies to all instances where dayjs.tz is used.
  • Reason this comment was not posted:
    Comment did not seem useful.
2. frontend/src/container/TopNav/DateTimeSelectionV2/index.tsx:670
  • Draft comment:
    Ensure that the dayjs timezone plugin is loaded before using dayjs.tz. This is necessary for the timezone adjustments to work correctly. This applies to all instances where dayjs.tz is used.
  • Reason this comment was not posted:
    Marked as duplicate.
3. frontend/src/container/TopNav/DateTimeSelectionV2/index.tsx:614
  • Draft comment:
    Avoid using inline styles in React components. Instead, use external stylesheets, CSS classes, or styled components. This issue is also present in other parts of the code.
  • Reason this comment was not posted:
    Comment was not on a valid diff hunk.

Workflow ID: wflow_DPmojYGctva1U4OQ


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>

4 similar comments
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

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>

@YounixM YounixM merged commit 89cb5af into feat/timezone-formatter-custom-hook-and-adjust-graphs-timezone Nov 25, 2024
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants