feat(mobile): local notifications for incoming transfers and completed transactions - #666
feat(mobile): local notifications for incoming transfers and completed transactions#666nonchalanttee22-lgtm wants to merge 3 commits into
Conversation
…design-the-token-detail-cs8d4cg2 Update 9 files
|
@nonchalanttee22-lgtm Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
@nonchalanttee22-lgtm is attempting to deploy a commit to the miracle656's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
The notification work here is good — One thing blocks it, and it is not your fault. The root
|
Notification work lands as-is: lib/notifications.ts, lib/notificationPrefs.ts, the useNotifications hook, the settings toggle and the app.config.ts wiring. Splitting preferences from delivery is the right shape. Dropped one thing from the merge: the root package-lock.json, which the branch carried at +20,100/-655. That is not churn — it takes the root from 81 resolved packages to 1,418 with versions moving throughout, so it would change what npm ci installs for everyone, from a PR about mobile notifications. Restored from main. frontend/mobile/package-lock.json is kept, since expo-notifications is a real new dependency there. The contributor did nothing wrong: running npm install at the repo root rewrites that file, because the root declares workspaces its committed lockfile does not describe. #663 hit the identical diff. Tracked in #670. Verified: the three-way merge preserves #674's TxDetailSheet theming (2 useTheme, 0 hardcoded colours) and the current welcome headline. Mobile tsc shows the 4 pre-existing sdk/src errors plus one for expo-notifications, which is only because the new dependency is not installed in this working copy — it is declared in package.json and present in the mobile lockfile. Claude-Session: https://claude.ai/code/session_01USgemLt4Rnz4SGB1Srf3GB
|
Your work is merged. It went in as I merged it by hand rather than through the button so I could drop one file: the root To be clear, that was not a mistake on your part — running This PR is now showing as open only because that lockfile is the single remaining difference between your branch and Two ways to finish it, your call — I am not closing it on you:
I also checked the merge did not undo anything recent: Thanks — splitting preferences from delivery was the right call, and it made the merge painless. |
Summary
The Veil mobile app had no notifications — users only discovered incoming transfers or confirmed transactions by manually opening the app. This PR adds local-only notifications via
expo-notificationsfired by the existing activity poller, with the Veil drape mark as the notification icon, a settings toggle, and full respect for app-lock and hidden-amounts mode. Push/remote notifications are explicitly out of scope.Closes #630
What changed
Notification infrastructure
expo-notifications— requires a new dev build (permissions and notification icon cannot be added OTA)assets/images/in the correct format and resolution for Android and iOS notification trays — does not reuse the existing app iconActivity poller integration
Notification content — app-lock and hidden-amounts
Settings toggle
Notification icon
assets/images/notification-icon.pngat the required dimensionsapp.json/app.config.tsas the Android notification icon and the iOS notification image — Expo default icon is not usedHow to verify
Checklist
expo-notificationsconfigured and registered in app configapp.json/app.config.tsfor Android and iOS