-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Funnel time frame change not updating on the dashboard #5358
Comments
@paolodamico It'd be great if your team could take a look here - I think you should have enough context to own dashboards! |
I spent a while last night gathering context for this. More specifically, the issue seems to be that updating specific dashboard item date frames doesn't reflect on the dashboard, when the dashboard itself has a different date filter. Easier to explain in code, so I wrote a test for it, but haven't yet managed to fix it. It appears to be a feature request more than a bug: "Allow dashboard item time filters to be saved independently from dashboard time filters" - but when dashboard time filters are updated, update all item time filters as well. Not enough context to decide the approach myself. The test, though, is here: #5395 |
We're happy to take this on but based on @neilkakkar's work it sounds like you may already have a head start on this? Want us to pick it up? When you talk about the context for the approach @neilkakkar do you mean UX-wise? As in how do global dashboard filters should override local filters? Just of the top of my head I think in general we should respect the most recent changes. I wonder if should introduce an option to let you switch between individually set filters and the global filter configured for the entire dashboard. Thoughts @clarkus ? |
Does the work happening at #3408 solve for this job? A user could click through the graph on the dashboard to the insight detail view to do any deeper analysis or explore different time ranges.
This could be really clever and powerful, but also really hard to communicate and really confusing for users. I would advocate for a simpler approach and default dashboards to fresh, recent time ranges (for example, the last 7 days, the last 24 hours, etc). The dashboard time context should be constant across all the graphs on the dashboard. I could also be misunderstanding the problem we're trying to solve here. Is this a case where dashboards should also serve as tools for comparing unlike time ranges? I was seeing comparison ranges in insights as the solution for that, but there might be more to it? My biggest concern is that it's not obvious that a dashboard is reflecting multiple time ranges and a user draws conclusions thinking that all graphs share the same time range. |
I agree with @clarkus , it reflects some of my own thinking. @paolodamico : I have so far just done the investigation to get to the core issue. About fixing / deciding not to do it, leaving it up to you? More context on the use case: https://posthog.slack.com/archives/C01HZB27BGX/p1627898599000100?thread_ts=1627395111.001800&cid=C01HZB27BGX |
UX-wise I'd like to propose this approach: user has the option to set a global dashboard time range or keep it in "Custom" / individual time ranges (in which the saved time range for insight is applied). Thing is that with #3408, dashboard items will now be accessible standalone and it'd be pretty confusing if their timeframe changes from a dashboard config (it further lends itself to more edge cases such as if a single insight is in multiple dashboards). So if a dashboard has a global custom time range, that would prevail as the time range for all items, but only for that dashboard. Thoughts? |
That's exactly how it works currently AFAIK - we store dashboard-specific filters (e.g. time range) in the dashboard object and merge it with the items config. :) |
To make sure I understand how this works:
Is that accurate? Or is it that once an insight is on a dashboard it defers to the dashboard's time context? |
Ok, so your scenario is correct @clarkus. However, there's the edge case where you have a dashboard with a configured global timeframe and afterwards you add a new insight. In this case, when you add it to the dashboard, it will also reflect the dashboard's global config. So I see two UX issues with what we currently have today:
|
Based on @neilkakkar's findings this seemed like an existing bug but apparently it's only started happening for users just recently? |
Given the work happening in #3408, I don't think we should change the time definition for an insight outside the context of directly editing that insight. If a dashboard can someone change the time definition saved for an insight, that's going to be really confusing.
"Custom" feels like the wrong label here. I could see custom implying a specific, fixed range of time. What custom really means here is "defer to the time range of each insight". The dashboard has no time context. Maybe "none" would be more descriptive? |
100% agreed @clarkus, exactly my point as something to address. Re "None", I don't think this communciates well what's going on either. Perhaps we should be more verbose here? Or at least combine with an info tooltip. |
I have been thinking about this and testing it a bit and I'm not sure custom actually works the way it's being described. In this video I have two charts - one set to the last 24 hours and one set to 90 days. Note that the dashboard reads this as custom. If I change this as at all to another option and then go back to "custom" the charts default to our 7 day range. This is the same default we use when starting a new insight. Is this how it's supposed to work? Screen.Recording.2021-08-10.at.1.34.15.PM.movI am working through some ideas for date control values that are more approachable. |
Hmm definitely not the expected behavior. Did you try refreshing? Might be a frontend caching issue. Really love the proposal for the dropdown and the tooltips, think definitely makes things a lot clearer. One additional thing worth considering is the role of timezone, particularly for short timeframes we may want to show something around that. Great point on all the options, I believe we do capture information around all these options so will take a look tomorrow to see if we can optimize. |
Yeah I tried it with cache disabled and it was reproducible.
I've been thinking about where to place the timezone conversion. We talked about placing in the user / utility menu updates, but maybe it needs to be more front and center. I had also tried in in the global header, but there was feedback that was too prominent. Ideally I'd like to place that prominently to set context for the entire product. Short of that, we can append it to the control that's setting the time context for everything below it. |
I think having it here makes a lot of sense. It's clear and right where you need it. |
I fixed one bug reported here. Whenever we would have a custom date range in the filter without an end time (e.g. "2021-05-10" - today), the filter would say "Last 7 days". Now it'll give the right number of days. Why the date was hardcoded in the range to begin with, I don't know. Regarding dashboard items with custom date ranges, the proposed solution makes sense. However if we're to be accurate, I think we should communicate the per-insight date range within the dashboard item. Or will custom descriptions ("Browsers last 30 days") and chart axes be enough? I'm leaning towards "explicit is better than implicit". We could show the same "Last 7 days" dropdown inside or above each dashboard item, even if just in a tooltip with a calendar icon. One added consideration. Anyone can change the date filter on a dashboard and it will then persist the new value in the "database items" table for all insights on the dashboard, overriding whatever was there before. This means after changing the date filter once, we won't have "default" value to revert back to. What to do? Should we display a warning/alert when changing away from the "default" option to something else? Should we update the database schema to persist the default range for each insight and have some flakey code to swap them back? |
I don't think we can rely on meaningful names or descriptions. I agree that we should be explicit here. I'm not sure about allowing for time context changes at a granular level like this. I mean it's very flexible, but it could also lead to a dashboard state that is very confusing. Also if the time context changes and it disagrees with any representation of time in the dashboard item name, it could lead to more confusion.
I encountered this previously and it is very confusing. How does the work happening for #3408 impact this behavior? Are we modifying the time context for a "saved insight" outside of the editing / creation workflow? Does this only impact the previous model for saving insights? I feel like it's going to be a pretty bad experience if something other than the insight editing workflow modifies your insight definition. There are other considerations, too. With funnels, the time to convert option should be kept in sync with the time range for the the visualization. If those can be changed independently without any synchronization, it could lead to really confusing results. #5557 |
Updating here on the date options based on usage in the last 90 days. Based on the data, I think we can get rid of the following options due to low usage (less than 1% of insights) and to simplify:
CC @clarkus |
👍 on dropping the options that are seeing less use.
|
Superseded by #6712 |
This issue has 2282 words at 22 comments. Issues this long are hard to read or contribute to, and tend to take very long to reach a conclusion. Instead, why not:
Is this issue intended to be sprawling? Consider adding label |
Bug description
Please describe.
If this affects the front-end, screenshots would be of great help.
After updating a funnel's time frame from
All time
to90 days
, the insight itself updates but does not save to the dashboard correctly, rendering an incorrect old view**note: I haven't been able to reproduce this error myself, I encounter a different 502 bad gateway error instead actually
Expected behavior
Updating a dashboard insight should render it correctly in the dashboards view.
How to reproduce
Environment
Additional context
Thank you for your bug report – we love squashing them!
The text was updated successfully, but these errors were encountered: