Alerts time range refactor #4240
Conversation
|
Some UX items:
|
| @@ -36,14 +41,18 @@ The main feature-set component for dashboard filters | |||
| </script> | |||
|
|
|||
| <div class="relative flex flex-row flex-wrap gap-x-2 gap-y-2 items-center"> | |||
| {#if timeRange} | |||
| <ReadOnlyTimeRange {timeRange} /> | |||
There was a problem hiding this comment.
It'd be nice symmetry with DimensionFilterReadOnlyChip and MeasureFilterReadOnlyChip if this component were named TimeRangeReadOnlyChip
web-common/src/features/alerts/delivery-tab/AlertDialogDeliveryTab.svelte
Outdated
Show resolved
Hide resolved
| $: if ($metricsViewSpec?.data && $timeRange?.data) { | ||
| const formValues = extractAlertFormValues( | ||
| queryArgsJson, | ||
| $metricsViewSpec.data, | ||
| $timeRange.data, | ||
| ); | ||
| for (const fk in formValues) { | ||
| $form[fk] = formValues[fk]; | ||
| } | ||
| console.log(formValues); | ||
| } |
There was a problem hiding this comment.
Is this code necessary? Looks like it might have been used for debugging
There was a problem hiding this comment.
Yes. Loading of metrics view spec and time range is async. So we need this to be loaded into the form. The other option is to get the args to the component, but this felt cleaner.
This PR updates how we handle time range in alerts.