feat: Better visibility for dashboard alerts #316
Conversation
|
svc-shorpo
force-pushed
the
sr/alerts-vis
branch
from
3a6118b
to
74b868a
Compare
| @@ -697,6 +727,10 @@ export default function DashboardPage() { | |||
| chart={chart} | |||
| dateRange={searchedTimeRange} | |||
| onEditClick={() => setEditedChart(chart)} | |||
| onAddAlertClick={() => { | |||
| setIsAddingAlert(true); | |||
There was a problem hiding this comment.
I'm curious if it would've been possible instead of having a boolean here to set the alert property to either the actual chart alert or DEFAULT_ALERT at this level instead of needing to depend on a useEffect? That feels a bit cleaner, but maybe there's some details that make it more annoying/harder to implement.
There was a problem hiding this comment.
this is a good point. iirc couldn't insert DEFAULT_ALERT to alert property as they're not type compatible (missing fields like slack webhook url). i'm going to clean up form state in a separate pr
 When clicking `Add alert` button it will open the edit modal with `Enable alert` checkbox on
When clicking
Add alertbutton it will open the edit modal withEnable alertcheckbox on