feat: #72 — Replace Data nav icon with map marker; extract Alerts into dedicated screen#108
feat: #72 — Replace Data nav icon with map marker; extract Alerts into dedicated screen#108luandro wants to merge 1 commit into
Conversation
…tract Alerts into dedicated screen
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| <Link | ||
| key={alert.localId} | ||
| to="/data/alerts/$alertId" | ||
| params={{ alertId: alert.localId }} | ||
| className="no-underline" | ||
| > | ||
| <Card className="p-4 hover:shadow-elevated transition-shadow cursor-pointer h-full"> | ||
| <AlertCard alert={alert} /> | ||
| </Card> | ||
| </Link> |
There was a problem hiding this comment.
AlertDetailScreen hardcodes its back button to to="/data" (line 213 of AlertDetailScreen.tsx). A user who navigates /alerts → clicks an alert card → /data/alerts/$alertId → presses back will land on the Data screen instead of returning to Alerts. The new /alerts entry point shares the same detail route that was originally only reachable from /data, so the hardcoded back link is now wrong for this flow. Consider using router.history.go(-1) (or a from search param) in AlertDetailScreen, or creating a separate /alerts/$alertId route.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/screens/AlertsScreen.tsx
Line: 148-157
Comment:
**Back navigation broken when entering alert detail from Alerts screen**
`AlertDetailScreen` hardcodes its back button to `to="/data"` (line 213 of `AlertDetailScreen.tsx`). A user who navigates `/alerts` → clicks an alert card → `/data/alerts/$alertId` → presses back will land on the Data screen instead of returning to Alerts. The new `/alerts` entry point shares the same detail route that was originally only reachable from `/data`, so the hardcoded back link is now wrong for this flow. Consider using `router.history.go(-1)` (or a `from` search param) in `AlertDetailScreen`, or creating a separate `/alerts/$alertId` route.
How can I resolve this? If you propose a fix, please make it concise.1 participant
Summary
Automated implementation of #72 by forge.
Changes:
Closes #72
*Implemented by the agent implementation worker.
Greptile Summary
This PR extracts the Alerts tab from
DataScreeninto a dedicated/alertsroute and screen, replaces the Data nav icon with a map-marker SVG, adds a bell icon for the new Alerts nav item, and wires up i18n and tests for all changes.AlertsScreen.tsxis a new top-level screen at/alertsthat mirrors the former alerts tab inDataScreen, with full skeleton/error/empty states and an "Add Alert" CTA linking to/data/alerts/new.DataScreen.tsxis simplified to observations-only by removing theTabswrapper, the alerts query, and associated messages.es.json/pt.jsonreceived the newalerts.*translations but were not cleaned up of the five staledata.*alert keys removed fromen.json.Confidence Score: 3/5
Safe to merge for the structural refactor, but the back-navigation from alert detail will silently drop users on the Data screen when they arrived from Alerts.
The alerts-to-detail navigation path is now reachable from two entry points (/data and /alerts), but AlertDetailScreen back button permanently hardcodes to /data. Any user who enters the detail from the new /alerts screen will be routed to Data on pressing back, making the Alerts screen feel like a dead end. The stale translation keys in es.json/pt.json are a secondary concern that may surface in CI i18n parity check.
src/screens/AlertDetailScreen.tsx (not in this diff) needs its back-navigation updated to handle being reached from /alerts; src/i18n/messages/es.json and pt.json need the five orphaned data.* alert keys removed.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD NAV[Bottom Nav] NAV -->|/| HOME[HomeScreen] NAV -->|/data| DATA[DataScreen Observations only] NAV -->|/alerts| ALERTS[AlertsScreen new] NAV -->|/settings| SETTINGS[SettingsScreen] ALERTS -->|Add Alert CTA| CREATE[CreateAlertScreen /data/alerts/new] ALERTS -->|Click card| DETAIL[AlertDetailScreen /data/alerts/$alertId] DATA -->|Click observation| OBS_DETAIL[ObservationDetail] DETAIL -->|Back button hardcoded| DATA DETAIL -.->|Expected back to| ALERTSComments Outside Diff (1)
src/i18n/messages/es.json, line 51-94 (link)es.jsonandpt.jsonen.jsonhaddata.addAlert,data.alertsError,data.noAlerts,data.tabs.alerts, anddata.tabs.observationsremoved (they moved toalerts.*), butes.jsonandpt.jsonstill contain all five of those olddata.*keys. They are now dead strings not referenced by any component. If CI runs a locale key-parity check these files will diverge fromen.json; at minimum they'll silently inflate the translation bundle.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat: implement #72 — feat: Replace Data..." | Re-trigger Greptile