feat(app): consolidate the alert detail header into the shared row menu - #3016
feat(app): consolidate the alert detail header into the shared row menu#3016jordan-simonovski wants to merge 2 commits into
Conversation
…ource icons The alerts page row menu now opens the alert editor, so changing a threshold no longer means navigating into the alert first. The modal needs a range for its threshold preview; a list row has none, so it derives one from the alert's interval when the modal opens. Each row's source icon gains a tooltip and accessible label naming what the alert watches, a new filter narrows the list by that source, and free-text search matches it too. All three read one getAlertSourceLabel helper so their wording cannot drift. Named 'alert source' rather than 'type' (taken by detection type) or bare 'source' (reads as a data source). The creator moves out of the shared properties line into its own dimmed sub-line: it is provenance, not configuration, and at equal weight it pushed the line into a second row that broke mid-phrase. Remaining segments no longer wrap mid-phrase. Team settings tabs gain icons.
Edit, Delete and Terraform export move behind the overflow menu the alerts list already uses, so the header stops spreading four buttons across the top and the two surfaces cannot drift apart on which actions an alert offers. The menu gains an optional dateRange (the detail page has a picked range for the edit modal's threshold preview, where a list row does not) and an onDeleted callback so the page can navigate away once the alert is gone. The link to what the alert watches becomes an icon beside the alert name: it navigates to the alert's subject, so it belongs with the identity rather than with the verbs acting on the alert. The properties block separates configuration from provenance, moving the creator down to join the created and updated timestamps in one dimmed line.
🦋 Changeset detectedLatest commit: 16b0e48 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🟡 Tier 3 — StandardIntroduces new logic, modifies core functionality, or touches areas with non-trivial risk. Why this tier:
Review process: Full human review — logic, architecture, edge cases. Stats
|
Greptile SummaryThe PR consolidates alert edit, delete, source navigation, and Terraform export behavior around the shared row menu while adding source filtering and clearer alert metadata presentation.
Confidence Score: 4/5The PR appears safe to merge, with non-blocking cleanup recommended for semantic color usage and eager per-row edit-modal mounting. The shared action and filtering flows preserve their functional contracts, while the remaining accepted concerns are theme consistency and avoidable closed-modal work in virtualized alert rows. Files Needing Attention: packages/app/src/AlertDetailPage.tsx, packages/app/src/components/alerts/AlertRowMenu.tsx
|
| Filename | Overview |
|---|---|
| packages/app/src/components/alerts/AlertRowMenu.tsx | Centralizes edit, export, and delete behavior, but unconditionally mounts a query-consuming edit modal for every rendered row. |
| packages/app/src/AlertDetailPage.tsx | Replaces dedicated detail actions with the shared menu and a title source link; the new ActionIcon bypasses semantic color tokens. |
| packages/app/src/AlertsPage.tsx | Adds URL-backed source filtering and source-label search without an established functional defect. |
| packages/app/src/components/alerts/AlertDetailProperties.tsx | Separates creator and timestamp provenance from alert configuration while retaining defensive time formatting. |
| packages/app/src/utils/alerts.ts | Adds a shared, deterministic human-readable label for supported and unresolved alert sources. |
| packages/app/src/TeamPage.tsx | Adds icons to existing team settings tabs without changing their navigation behavior. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Detail[Alert detail page] --> Menu[Shared AlertRowMenu]
List[Alerts list row] --> Menu
Menu --> Edit[EditAlertModal]
Menu --> Export[Terraform export]
Menu --> Confirm[Delete confirmation]
Confirm --> Delete[Delete alert]
Delete --> Invalidate[Invalidate alert and source caches]
Invalidate --> Callback[Optional onDeleted callback]
Callback --> Alerts[Alerts list route]
Reviews (1): Last reviewed commit: "feat(app): consolidate the alert detail ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| <ActionIcon | ||
| component={Link} | ||
| href={alertUrl} | ||
| variant="subtle" |
There was a problem hiding this comment.
The new source-link ActionIcon uses the raw gray Mantine color rather than a semantic theme token, making this control inconsistent across themes and future palette changes.
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| {/* Outside the dropdown, which unmounts on close. */} | ||
| <EditAlertModal | ||
| alert={alert} | ||
| opened={editOpened} | ||
| onClose={() => setEditOpened(false)} | ||
| dateRange={previewRange} | ||
| /> |
There was a problem hiding this comment.
Every rendered alert row now mounts an EditAlertModal even while closed, so its source queries, form state, watched values, and preview calculation run for each visible virtualized row. Mounting the modal on demand would avoid this hidden work and reduce its coupling to row virtualization.
E2E Test Results✅ All tests passed • 322 passed • 1 skipped • 1204s
Tests ran across 4 shards in parallel. |
Deep Review✅ No critical issues found. No P0/P1 ship-blockers survived re-grading — the change is a frontend-only UI consolidation with no data, auth, or contract impact. The recommendations below are worth addressing before merge. 🟡 P2 -- recommended
🔵 P3 nitpicks (4)
Pre-existing (not counted toward verdict)
Reviewers (11): correctness, adversarial, kieran-typescript, julik-frontend-races, testing, maintainability, project-standards, performance, agent-native, learnings-researcher, previous-comments. Testing gaps:
|
1 participant
The alert detail header spread Edit, Delete and a source link across four controls next to the time picker, and offered a different set of actions from the alerts list for the same alert. This moves them behind the same overflow menu the list uses.
What changed
Key decisions
Shared menu over a page-specific one. The page's own edit modal, delete handler and confirmation are deleted in favour of the list's. Two surfaces maintaining parallel action sets is how they drifted apart in the first place. The menu takes an optional
dateRange(this page has a picked range for the preview chart; a list row does not) and anonDeletedcallback so the page can navigate away.Source link next to the title, not in the actions. It navigates to the alert's subject, so it belongs with the identity rather than the verbs acting on the alert. The menu's own source-link item is suppressed here to avoid offering it twice.
Impact
Frontend only. The delete confirmation on this page changes from
ConfirmDeleteMenuto the shareduseConfirmdialog, and deletion now returns you to/alerts.This branch sits on top of #3015 and its diff currently includes that commit; the diff shrinks to this page's changes once #3015 merges. Review the second commit (
consolidate the alert detail header…) in isolation, or wait for the rebase.Implementation detail
alerts,saved-search,dashboards) as the removed handler; navigation moved intoonDeleted.