Use text for relative timestamps and numbers for absolute #618
Conversation
ui/src/pages/Detail.tsx
Outdated
| } | ||
| } | ||
|
|
||
| // const from = fromQuery != null ? parseInt(fromQuery) : to - 3600 * 1000 |
There was a problem hiding this comment.
Oh, this needs to be removed now.
| @@ -20,6 +21,7 @@ interface ErrorsGraphProps { | |||
| from: number | |||
| to: number | |||
| uPlotCursor: uPlot.Cursor | |||
| updateTimeRange: (min: number, max: number, absolute: boolean) => void | |||
There was a problem hiding this comment.
I guess, for less duplication and more type rigidity, there can be a base type called GraphProps with fields like from, to, updateTimeRange and that type can be extended in these three components to add their own fields.
There was a problem hiding this comment.
That makes sense, yes. Given this is a PR to fix a bug I won't address this now. We should definitely do this later!
| @@ -70,11 +71,28 @@ const Detail = () => { | |||
|
|
|||
| const name: string = labels[MetricName] | |||
|
|
|||
| let to: number = Date.now() | |||
There was a problem hiding this comment.
If possible, to, from parsing logic and updateTimeRange callback can be extracted into a useTimeRange hook, so that the timeRange parsing and setting logic would be in one place inside that hook without getting spilled into this component.
But I guess since this is the only page that uses timeRange, this hasn't surfaced as a problem.
Using text like `now` and `now-12h` will not spam the browser's history like reported in #590. However, selecting a specific time range with the cursor in the graphs will now still select the very specific timestamps and store them in the URL.
metalmatze
force-pushed
the
ui-refresh-absolute-timestamps
branch
from
d7c4c3a
to
43a8013
Compare
Using text like
nowandnow-12hwill not spam the browser's history like reported in #590.However, selecting a specific time range with the cursor in the graphs will now still select the very specific timestamps and store them in the URL.