[APP-421] fix: enforce smallest_time_grain#8267
Conversation
AdityaHegde
left a comment
There was a problem hiding this comment.
This adds a weird state where the data displayed is not for the grain present in the url. Is there a hard requirement to show the warning? If not, would be great if we just correct it in the url as well.
web-common/src/features/dashboards/time-series/MetricsTimeSeriesCharts.svelte
Outdated
Show resolved
Hide resolved
|
This is intentional on my part, but it's not a hard requirement, so I'm happy to consider what others think about it during the applications sync. The thinking is that I want to avoid ever changing the URL without user input. We are not there yet on Explore, but I think this is a step in the right direction. I want the URL to be a reliable indicator of what the user has asked for and I think changing it only leads to hiding problems and making things difficult to debug. I want to follow the principle of what you see is what you get it. Sometimes, what you ask for is not what you're allowed to see, but at least we still follow the principle by acknowledging that it's being asked for and displaying a non-blocking error. You can imagine a situation where a user has manually changed the URL to a grain value that is too small. I think this new UX state is more transparent about what is happening and does a better job of showing that we're guarding against this kind of stuff. Also, if you use a bookmark that has an out of date value, this would make it clear that the bookmark should be updated. |
AdityaHegde
left a comment
There was a problem hiding this comment.
Looks good from code POV. @ericokuma @nishantmonu51 please approve if we are ok with the UX
* wip * remove logs * cleanup * test fixes * remove logs * quality check * test fix * remove logs * test fixes * tooltip adjustment * test fixes * rilltime cleanup * test updates * test fix * test fix * test fix * feedback * comment * ux feedback * use DEFAULT_TIME_RANGES map * type fix * test fix * test fix * test fix
smallest_time_graingetAggregationGrainfunctionChecklist: