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: Adjust dates display on the graph x-axis #25123

Closed
wants to merge 5 commits into from

Conversation

anirudh24seven
Copy link
Contributor

Problem

Fix for #2656.

The logic explained:

  1. Find out the graph type (hours, days, weeks, months)
  2. Depending on the graph type, customize what to display

For example, hourly graph will not show the date unless it is a new day, daily graph will not show the year in the date unless it is a new year and so on. The screenshots below better illustrate this.

Changes

2024-09-22_08-46
2024-09-22_08-46_1
2024-09-22_08-47
2024-09-22_08-48

How did you test this code?

I have only manually tested this. I am having trouble running the visual tests.

Concerns

  1. The x-axis spacing changes when switching to a different duration type and coming back to the same view. I think this is an unrelated bug.
  2. I am not sure where to place the code, so I have placed it in utils. Any context here will be helpful.

Regarding point 2, I am not able to assuredly decide which component needs to have this function. I am also not sure if this should go inside a Kea function handler.

To be more precise, this is the tree hierarchy according to my investigations. Currently the change is at a LineGraph level. But should the changes to the labels be higher up the tree? Should it be in the Python code? Should it be in the HogQL query? I have attached these files to the PR for discussion and will remove them once the decision is made.

WebAnalyticsDashboard
Tiles
TabsTileItem
WebQuery
WebStatsTableTile
Query
ErrorBoundary
InsightViz
ErrorBoundary
BindLogic - insight
BindLogic - dataNode
BindLogic - insightVizData
InsightCard__viz
InsightVizDisplay
Trends
ActionsLineGraph
LineGraph

@Twixes Twixes requested a review from a team September 23, 2024 07:30
@posthog-bot
Copy link
Contributor

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week. If you want to permanentely keep it open, use the waiting label.

@posthog-bot
Copy link
Contributor

This PR was closed due to lack of activity. Feel free to reopen if it's still relevant.

@posthog-bot posthog-bot closed this Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants