Alerting: alert list state view #33020
Conversation
domasx2
requested review from
mckn,
peterholmberg,
nathanrodman,
bfmatei and
a team
|
This looks soo good 👍 . Probably will have to do something with the big group names, but anyway awesome work! |
🌞 |
domasx2
force-pushed
the
domas/rule-edit-form-edit
branch
from
5fde045
to
48dfb46
Compare
domasx2
force-pushed
the
domas/rule-list-state-view
branch
from
9589a05
to
bc4aa80
Compare
|
|
||
| const styles = useStyles(getStyles); | ||
| const tableStyles = useStyles(getAlertTableStyles); | ||
|
|
||
| const [expandedKeys, setExpandedKeys] = useState<string[]>([]); | ||
|
|
||
| const [ruleToDelete, setRuleToDelete] = useState<RulerRuleDTO>(); | ||
| const [ruleToDelete, setRuleToDelete] = useState<CombinedRule>(); | ||
|
|
||
| const toggleExpandedState = (ruleKey: string) => | ||
| setExpandedKeys( | ||
| expandedKeys.includes(ruleKey) ? expandedKeys.filter((key) => key !== ruleKey) : [...expandedKeys, ruleKey] | ||
| ); | ||
|
|
||
| const deleteRule = () => { |
There was a problem hiding this comment.
Question: why not just dispatch the deleteRuleAction on click instead of doing it on next render after we set the state? Probably me that is missing something.
There was a problem hiding this comment.
setting ruleToDelete triggers a confirmation modal to be shown, but does not disaptch the delete action.
delete action is dispatched once user clicks confirmation button on the modal
There was a problem hiding this comment.
ah, ok ok - got it! Makes sense now.
| rules: [], | ||
| }; | ||
| combinedGroup.rules = group.rules.map( | ||
| (rule): CombinedRule => |
There was a problem hiding this comment.
nit: I think this code should be split up into multiple smaller functions to improve readability and prevent nesting.
There was a problem hiding this comment.
- break it up a little
There was a problem hiding this comment.
broke it up into several hopefully well-named functions
Thanks!! I will fix the nits, but please approve so I can merge things and keep moving :) |
domasx2
force-pushed
the
domas/rule-list-state-view
branch
from
bc4aa80
to
49b1a6a
Compare
Nailed together a rule list view that shows rules by state.
Some types were rejiggled to make it work
state-view.mp4
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer: