fix(dashboards) Add a confirm step to deletion #23098
Conversation
Add a confirmation to dashboard deletion. I needed a ?. in getSeriesSelection() to work around undefined sneaking in during tests.
size-limit report
|
| parameter = 'unselectedSeries' | ||
| ): EChartOption.Legend['selected'] { | ||
| const unselectedSeries = decodeList(location.query[parameter]) ?? []; | ||
| const unselectedSeries = decodeList(location?.query[parameter]) ?? []; |
There was a problem hiding this comment.
I wonder if we should abstract the decode(...location...) pattern a bit? Maybe if we start using hooks we can have functions that just useContext.
There was a problem hiding this comment.
Definitely agree that abstracting the decode...(location.query...) pattern is a good idea. Though I'm not sure about hooks as we have a lot of class components using this pattern.
There was a problem hiding this comment.
I think that we could combine the fallback values into the decodeX functions. We frequently coalesce and having simpler return types would help simplify a bunch of code.
| parameter = 'unselectedSeries' | ||
| ): EChartOption.Legend['selected'] { | ||
| const unselectedSeries = decodeList(location.query[parameter]) ?? []; | ||
| const unselectedSeries = decodeList(location?.query[parameter]) ?? []; |
There was a problem hiding this comment.
Definitely agree that abstracting the decode...(location.query...) pattern is a good idea. Though I'm not sure about hooks as we have a lot of class components using this pattern.
Add a confirmation to dashboard deletion. I needed a ?. in getSeriesSelection() to work around undefined sneaking in during tests.