docs: add Settings and Notification Settings specs - #32
Conversation
- Rewrote SETTINGS_SCREEN.md with architecture, card behaviors, and cross-references - Added NOTIFICATION_SETTINGS.md detailing /notification_settings UI and side-effects - Updated README.md index with the new spec entry - Linked wallet/notification routes in NAVIGATION_ROUTES.md to their respective specs
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis pull request updates v1 reference documentation for the settings and notification systems. It creates a new NOTIFICATION_SETTINGS.md documentation file detailing push notification toggles and sound/vibration preferences, restructures SETTINGS_SCREEN.md from a single-screen spec to a multi-route architecture documentation, and updates NAVIGATION_ROUTES.md and README.md with cross-references to the new notification settings documentation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.specify/v1-reference/NOTIFICATION_SETTINGS.md:
- Line 57: Update the "Disable push" row to remove ambiguity by explicitly
stating that push controls are disabled while the underlying stored values for
sound and vibration remain unchanged; reference the "Disable push" label and the
"sound/vibration" toggles in your update and reword the sentence to something
like: "Immediately unregisters tokens and deletes FCM token; disables sound and
vibration controls in the UI while preserving their stored values (values remain
cached but are not applied)". Ensure the row clearly separates the action
(unregister/delete) from the UI state (controls disabled) and the preservation
of cached values.
In @.specify/v1-reference/SETTINGS_SCREEN.md:
- Line 77: The phrase "Orders created afterwards pre-fill this fiat code." uses
the British variant "afterwards"; update the sentence in SETTINGS_SCREEN.md to
use the US-English variant "afterward" so it reads "Orders created afterward
pre-fill this fiat code."—search for that exact sentence text to locate and
replace it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eaf46475-3893-46c3-bf67-7bedd20752ce
📒 Files selected for processing (4)
.specify/v1-reference/NAVIGATION_ROUTES.md.specify/v1-reference/NOTIFICATION_SETTINGS.md.specify/v1-reference/README.md.specify/v1-reference/SETTINGS_SCREEN.md
- NOTIFICATION_SETTINGS.md: clarify Disable push behavior (unregister/delete vs UI state vs cached values) - SETTINGS_SCREEN.md: use US-English 'afterward' instead of British 'afterwards'
Summary
Documents the complete Settings system (point 11 from the original feature list) including the Settings hub and Notification Settings sub-screen.
Changes
New Files
NOTIFICATION_SETTINGS.md
/notification_settingsbehaviorUpdated Files
SETTINGS_SCREEN.md (complete rewrite)
README.md
NAVIGATION_ROUTES.md
/notification_settingsroute to specSettings System Deep Dive
After analyzing v1 code, discovered:
State Management
Settingsclass: immutable model with 14 fieldsSettingsNotifierhandles async init, mutations, and side effectsKey Fields & Usage
selectedLanguage→ MaterialApp.locale, background servicesdefaultFiatCode→ AddOrderScreen pre-fill, exchange providersdefaultLightningAddress→ auto-fills invoice generation for buyersrelays/blacklistedRelays/userRelays→ relay sync subsystemmostroPublicKey→ Nostr handshake headerspushNotificationsEnabled→ FCM token managementnotificationSoundEnabled/notificationVibrationEnabled→ notification prefsisLoggingEnabled→ in-memory log buffer togglefullPrivacyMode→ (not exposed in UI, gates master key storage)Side Effects
All specs are now cross-linked with no orphaned documents.
Summary by CodeRabbit