feat(final-phase): polish & cross-cutting - #69
Merged
Conversation
- T118: complete ARB localization (138 keys, 5 languages: EN/ES/IT/FR/DE) - T119: responsive layout — 1/2/3-col order grid, persistent sidebar on desktop, chat side panel on tablet+, hide bottom nav on desktop (AppBreakpoints) - T120: theme toggle (Dark/Light/System) in Settings screen via settingsProvider - T121: PlatformAwareQrScanner — camera on native, paste-fallback on web - T122: graceful degradation — ConnectWallet uses PlatformAwareQrScanner - T123: offline queue flushing — MessageOutbox with backoff/prune, auto-flush on connection Online event; outbox tests (68 passing) - T124: CountdownTimer widget — circular progress, green→yellow→red thresholds - T125: cargo test (68 pass), cargo clippy -D warnings (clean), flutter analyze (clean)
- app_theme.dart: fix doc comment refs (kBreakpointTablet→tablet, kBreakpointDesktop→desktop)
- bottom_nav_bar.dart: remove dead private _isDesktop function
- settings_screen.dart: fix duplicate comment numbering (Lightning=4, NWC=5, Relays=6)
- countdown_timer.dart: zero-duration timer returns destructiveRed not green
- currency_selector_dialog.dart: removeListener before dispose to prevent leak
- relay_management_card.dart: pop dialog when parent is unmounted
- app_it.arb: align lightning address error format with hint text (add .com)
- language_selector.dart: use l10n selectLanguageTitle instead of hardcoded string
- settings.rs: tighten lightning address validation — require valid hostname with dot
- types.rs + outbox.rs: add InFlight status to prevent duplicate flush on concurrent calls
- l10n ARBs (all 5): offersCount uses ICU plural syntax (=1{1 offer} other{N offers})
68/68 Rust tests pass, clean clippy, clean Flutter analyze.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
WalkthroughAdds responsive breakpoints and layout adaptations (chat/home/drawer), a persistent/persistent-overlay drawer mode, platform-aware QR scanner and ConnectWallet handler change, a CountdownTimer widget, BottomNavBar desktop hide, extensive localization (ARB + generated getters), a Rust in-memory outbox with flush/retry/backoff integrated into nostr init, and assorted Rust API/type/validation tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant NostrAPI as Nostr API
participant ConnState as Relay Connection\nStream
participant OutBox as Message Outbox
participant RelayClient as Relay Client
App->>NostrAPI: initialize()
NostrAPI->>ConnState: subscribe connection-state stream
loop When connection emits Online
ConnState->>NostrAPI: Online
NostrAPI->>OutBox: flush_message_queue()
activate OutBox
OutBox->>OutBox: Snapshot Pending messages\nMark as InFlight
OutBox->>RelayClient: publish_fn (deserialize & send_event)
alt publish success
OutBox->>OutBox: Mark Sent
else publish failure
OutBox->>OutBox: Increment retry_count\nSet next_retry_at (backoff) or Mark Failed if max
end
OutBox->>OutBox: Prune Sent / old Failed
deactivate OutBox
OutBox-->>NostrAPI: return sent count
end
ConnState-->>NostrAPI: stream closed -> task exits
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
rust/src/queue/outbox.rs (1)
67-71: Mutex unwrap() can panic on poison.The
lock().unwrap()pattern is used throughout. If a thread panics while holding the lock, subsequent calls will panic. For a message queue, this might be acceptable, but consider usinglock().unwrap_or_else(|e| e.into_inner())to recover from poisoned locks if message delivery should be resilient.Alternative approach for poison recovery
pub fn enqueue(&self, event_json: String) { let now = unix_now(); let msg = QueuedMessage::new(event_json, now); - self.queue.lock().unwrap().push(msg); + self.queue + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + .push(msg); }Apply similar changes to other
lock().unwrap()call sites if resilience is desired.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust/src/queue/outbox.rs` around lines 67 - 71, The use of lock().unwrap() in enqueue can panic on a poisoned Mutex; update the Mutex acquisition in enqueue (and other similar call sites like where queue.lock().unwrap() appears) to recover from poison by using lock().unwrap_or_else(|e| e.into_inner()) so the code continues to push the QueuedMessage (created via QueuedMessage::new) into the inner Vec even if the lock was poisoned; ensure you change every occurrence of queue.lock().unwrap() to this pattern to make message enqueueing resilient.lib/shared/widgets/platform_aware_qr_scanner.dart (1)
45-53: Consider clearing error state on successful paste.When paste succeeds,
_errorTextisn't cleared before callingonDetected. While the widget likely gets disposed after detection, for robustness it's cleaner to clear the error:Proposed fix
Future<void> _pasteFromClipboard() async { final data = await Clipboard.getData(Clipboard.kTextPlain); final text = data?.text?.trim() ?? ''; if (text.isEmpty) { setState(() => _errorText = 'Clipboard is empty'); return; } + setState(() => _errorText = null); widget.onDetected(text); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/shared/widgets/platform_aware_qr_scanner.dart` around lines 45 - 53, The paste handler _pasteFromClipboard leaves a previous error message in _errorText on successful paste; update it to clear the error state before notifying the consumer by calling setState(() => _errorText = null or '') just prior to invoking widget.onDetected(text) so the UI resets reliably; make sure to reference the _pasteFromClipboard method, the _errorText field, and the widget.onDetected(text) call when applying the change.lib/l10n/app_localizations_it.dart (1)
409-411: Minor inconsistency in format hint.The Italian
invalidLightningAddressFormatincludes.comin the domain example (utente@dominio.com), while English (user@domain) and French (utilisateur@domaine) omit the TLD. Consider aligning for consistency, though this doesn't affect functionality.🔧 Suggested alignment with other locales
`@override` String get invalidLightningAddressFormat => - 'Deve essere nel formato utente@dominio.com'; + 'Deve essere nel formato utente@dominio';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/l10n/app_localizations_it.dart` around lines 409 - 411, The locale string invalidLightningAddressFormat currently uses a TLD example ('utente@dominio.com'); update its value to match other locales by removing the TLD so it reads 'utente@dominio' (modify the getter String get invalidLightningAddressFormat in lib/l10n/app_localizations_it.dart).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/features/home/screens/home_screen.dart`:
- Around line 191-206: Replace the hardcoded texts with localized strings:
change the 'FILTER' Text to use the generated localization key (e.g.,
AppLocalizations.of(context).filterButtonLabel) and change the
'${filteredOrders.length} offers' Text to use the pluralized localization (e.g.,
AppLocalizations.of(context).offersCount(filteredOrders.length) or the
equivalent generated method) so the ARB plural rules are used; ensure the widget
has access to BuildContext and the generated localization import is present and
remove the hardcoded literals in home_screen.dart.
- Around line 159-162: The Tab labels are hardcoded in the tabs list (the Tab
widgets inside the tabs property) and should use the new localized keys
tabBuyBtc and tabSellBtc; replace the literal 'BUY BTC' and 'SELL BTC' with the
localized values from your localization class (e.g., S.of(context).tabBuyBtc and
S.of(context).tabSellBtc or AppLocalizations.of(context)...) and remove the
surrounding const on the tabs list so the runtime-localized strings can be used;
ensure the localization import is present and context is in scope where the Tab
widgets are created.
In `@lib/features/settings/screens/connect_wallet_screen.dart`:
- Around line 115-117: The hardcoded scanner hint in ConnectWalletScreen
(PlatformAwareQrScanner hint: 'Paste NWC URI') must be replaced with a localized
string; update the PlatformAwareQrScanner call in connect_wallet_screen.dart to
use your app's localization accessor (e.g.,
AppLocalizations.of(context).pasteNwcUri or S.of(context).pasteNwcUri) and add
the corresponding localization key/value (pasteNwcUri) to the ARB/translation
files for all supported locales so the hint is translated.
- Around line 94-97: Normalize the scanned QR string in _onQrDetected before
validating the prefix: trim leading/trailing whitespace and perform
case-insensitive scheme comparison (e.g., compare
normalized.toLowerCase().startsWith('nostr+walletconnect://')), and assign the
cleaned value to _uriController.text before hiding the scanner (_showScanner =
false) so valid payloads with whitespace or uppercase scheme are accepted.
In `@lib/features/settings/screens/settings_screen.dart`:
- Around line 49-57: Replace hardcoded English strings in the settings UI with
localized values from AppLocalizations: change the title passed to _settingsCard
from 'Appearance' to AppLocalizations.of(context).appearance (or the ARB key you
added), replace any hardcoded labels returned by _themeLabel(settings.themeMode)
so it uses localized strings (e.g.,
AppLocalizations.themeLight/themeDark/themeSystem) and update any labels inside
_showThemeDialog to pull text from AppLocalizations as well; ensure all
occurrences that build the Appearance card and its dialog (references:
_settingsCard, _themeLabel, _showThemeDialog) use
AppLocalizations.of(context).<key> instead of literal English strings.
In `@lib/features/settings/widgets/relay_management_card.dart`:
- Around line 111-114: The current early-return checks only the widget's
State.mounted but calls Navigator.of(ctx).pop() even if the dialog BuildContext
(ctx) is unmounted; update the guard to verify both mounted and ctx.mounted
before calling Navigator.of(ctx).pop(), e.g., if (!mounted || !ctx.mounted) {
return; } or check ctx.mounted specifically before calling
Navigator.of(ctx).pop() in the function handling the dialog (the block using the
local variable ctx and calling Navigator.of(ctx).pop()) so you never call
Navigator.of on an unmounted context.
In `@lib/shared/widgets/countdown_timer.dart`:
- Around line 39-63: The CountdownTimer widget fails to handle updates to
widget.duration and doesn't trigger onExpired immediately for zero/negative
durations; add an override of didUpdateWidget in the State to detect when
widget.duration changes, reset _remaining to widget.duration, cancel and restart
_timer via _start (or cancel if duration is zero), and ensure that both
initState and didUpdateWidget check for Duration.zero or negative values and
call widget.onExpired immediately (and avoid scheduling a periodic timer) while
still cancelling any existing _timer; reference the State lifecycle methods
initState, didUpdateWidget, dispose, and the helper _start and field _remaining
and onExpired when making changes.
In `@lib/shared/widgets/platform_aware_qr_scanner.dart`:
- Around line 98-104: The camera scanner path in onDetect uses
capture.barcodes.firstOrNull?.rawValue without trimming, causing inconsistency
with the web fallback; update the onDetect handler (the onDetect callback that
references _detected, capture.barcodes.firstOrNull?.rawValue and
widget.onDetected) to trim the raw string (e.g., call .trim() on the rawValue),
then check emptiness on the trimmed value and call widget.onDetected with the
trimmed value while preserving the existing _detected guard and early return
behavior.
In `@rust/src/api/settings.rs`:
- Around line 107-129: The validation uses a trimmed value (`trimmed`) but the
setter persists the original `address`, so inputs like " alice@example.com "
pass but get saved with spaces; in `set_default_lightning_address` (and any
place using `address`/`trimmed`) persist the normalized value instead of the raw
input — e.g., compute and use `trimmed` (and optionally `.to_lowercase()` for
canonicalization) when calling the storage/update path so the stored Lightning
address contains the normalized form, not the original string with whitespace.
---
Nitpick comments:
In `@lib/l10n/app_localizations_it.dart`:
- Around line 409-411: The locale string invalidLightningAddressFormat currently
uses a TLD example ('utente@dominio.com'); update its value to match other
locales by removing the TLD so it reads 'utente@dominio' (modify the getter
String get invalidLightningAddressFormat in lib/l10n/app_localizations_it.dart).
In `@lib/shared/widgets/platform_aware_qr_scanner.dart`:
- Around line 45-53: The paste handler _pasteFromClipboard leaves a previous
error message in _errorText on successful paste; update it to clear the error
state before notifying the consumer by calling setState(() => _errorText = null
or '') just prior to invoking widget.onDetected(text) so the UI resets reliably;
make sure to reference the _pasteFromClipboard method, the _errorText field, and
the widget.onDetected(text) call when applying the change.
In `@rust/src/queue/outbox.rs`:
- Around line 67-71: The use of lock().unwrap() in enqueue can panic on a
poisoned Mutex; update the Mutex acquisition in enqueue (and other similar call
sites like where queue.lock().unwrap() appears) to recover from poison by using
lock().unwrap_or_else(|e| e.into_inner()) so the code continues to push the
QueuedMessage (created via QueuedMessage::new) into the inner Vec even if the
lock was poisoned; ensure you change every occurrence of queue.lock().unwrap()
to this pattern to make message enqueueing resilient.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f04e50e2-980e-4641-b307-30555db6141d
📒 Files selected for processing (34)
lib/core/app_theme.dartlib/features/chat/screens/chat_room_screen.dartlib/features/drawer/screens/drawer_menu.dartlib/features/home/screens/home_screen.dartlib/features/settings/screens/connect_wallet_screen.dartlib/features/settings/screens/settings_screen.dartlib/features/settings/widgets/currency_selector_dialog.dartlib/features/settings/widgets/language_selector.dartlib/features/settings/widgets/relay_management_card.dartlib/l10n/app_de.arblib/l10n/app_en.arblib/l10n/app_es.arblib/l10n/app_fr.arblib/l10n/app_it.arblib/l10n/app_localizations.dartlib/l10n/app_localizations_de.dartlib/l10n/app_localizations_en.dartlib/l10n/app_localizations_es.dartlib/l10n/app_localizations_fr.dartlib/l10n/app_localizations_it.dartlib/shared/widgets/bottom_nav_bar.dartlib/shared/widgets/countdown_timer.dartlib/shared/widgets/platform_aware_qr_scanner.dartrust/src/api/nostr.rsrust/src/api/orders.rsrust/src/api/reputation.rsrust/src/api/settings.rsrust/src/api/types.rsrust/src/config.rsrust/src/crypto/keys.rsrust/src/mostro/session.rsrust/src/nwc/client.rsrust/src/queue/outbox.rsspecs/004-mostro-p2p-client/tasks.md
- home_screen.dart: use l10n for tab labels, filter text, offersCount plural - connect_wallet_screen.dart: trim+lower-case QR input before scheme check; use pasteNwcUri l10n key (added to all 5 ARBs) - settings_screen.dart: localize Appearance card title and theme dialog using appearanceSettingTitle/appearanceDialogTitle/themeDark/themeLight/themeSystemDefault - relay_management_card.dart: guard ctx.mounted before dialog pop on unmount - countdown_timer.dart: add didUpdateWidget for duration changes; fire onExpired immediately (via addPostFrameCallback) for zero/negative initial durations - platform_aware_qr_scanner.dart: trim rawValue in camera handler; clear _errorText before calling onDetected in paste path - settings.rs: persist trimmed lightning address instead of raw input - app_it.arb: revert invalidLightningAddressFormat to utente@dominio (no TLD) to align with all other locales; regenerated l10n - outbox.rs: replace lock().unwrap() with unwrap_or_else(|e| e.into_inner()) in all production lock sites for poison resilience 68/68 Rust tests pass, clean clippy, clean Flutter analyze. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/features/settings/widgets/relay_management_card.dart (1)
64-127:⚠️ Potential issue | 🟡 MinorRelay management is still English-only.
This dialog and its inline controls are all literal English right now, so Settings remains partially untranslated even after the new relay/settings l10n keys were added. Please wire the title/button/tooltip/semantics copy through
AppLocalizations, and add keys for the remaining validation/error strings.Also applies to: 177-205
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/settings/widgets/relay_management_card.dart` around lines 64 - 127, Replace all hard-coded English strings in _showAddRelayDialog (title, TextField hintText, TextButton labels "Cancel"/"Add" and validation messages "Must start with wss://", "URL is too short", "Relay already in list") with localized values from AppLocalizations (e.g. AppLocalizations.of(context)!.relayAddTitle, relayHint, cancel, add, relayErrorMustStartWith, relayErrorTooShort, relayErrorDuplicate) and call those from inside the dialog builder using the dialog context; add matching l10n keys/translations for each new string in the app localization files and update the other similar relay dialog block (the second dialog around the other relay-management code) the same way so both dialogs use AppLocalizations instead of literals.
♻️ Duplicate comments (1)
lib/features/settings/screens/settings_screen.dart (1)
33-47:⚠️ Potential issue | 🟡 MinorMost of
SettingsScreenstill bypassesAppLocalizations.Only the new Appearance row is localized right now. The app bar title, existing settings cards, and the Lightning Address dialog still render English literals, so non-English locales will get a mixed-language settings page even though most of these getters were added in this PR.
Also applies to: 61-94, 123-130, 159-186, 291-345
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/settings/screens/settings_screen.dart` around lines 33 - 47, The SettingsScreen currently uses hardcoded English literals (e.g., AppBar title 'Settings', card titles/subtitles, and the Lightning Address dialog text) which causes mixed-language UI; replace those string literals by fetching localized strings from AppLocalizations (via AppLocalizations.of(context) or the project's context.l10n helper) wherever you build the UI in SettingsScreen and the related _settingsCard usages, including the title in AppBar, the 'Language' title/subtitle (languageNameForCode can remain but its label should be localized), the existing settings card titles/subtitles, and any Lightning Address dialog text (the dialog builder function); ensure you reference the existing localization keys (or add new ones) and use them instead of hardcoded English so all rows and dialogs use localized strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/features/settings/screens/settings_screen.dart`:
- Around line 257-269: The selected theme is only updated in-memory by calling
settingsProvider.notifier.setThemeMode from _showThemeDialog, so it’s not
persisted across restarts; modify the persistence flow so setThemeMode (in
SettingsNotifier inside settings_provider) writes the new theme into the backing
store (e.g., call your existing settingsRepository/Preferences API or add a
persistTheme/persistSettings method) after updating state, or alternatively have
_showThemeDialog call that persistence method immediately after setThemeMode;
update the code paths referencing setThemeMode/settingsProvider to ensure the
chosen ThemeMode is saved to disk/storage as well as to Riverpod state.
In `@lib/shared/widgets/platform_aware_qr_scanner.dart`:
- Around line 22-23: The web fallback strings in PlatformAwareQrScanner are
hardcoded in the constructor and other places (hint, heading, button labels,
clipboard/error messages); replace these literal English strings with the app
localization keys already added (e.g., pasteNwcUri, submitButtonLabel) and add
new localization keys for the remaining texts (hint, clipboard empty/error,
heading) and use the l10n lookup (e.g.,
AppLocalizations.of(context).pasteNwcUri) wherever those literals appear
(notably in the constructor assignment this.hint and the UI build methods
referenced around platform_aware_qr_scanner's constructor, the build
headings/buttons and the clipboard/error handling code).
- Around line 35-79: The web path can call widget.onDetected multiple times (via
_pasteFromClipboard and _submit) violating the “exactly once” contract; add a
single guarded emitter (e.g., private bool _hasEmitted = false and a method
_emitDetectedOnce(String value)) that checks _hasEmitted, sets it true, and then
calls widget.onDetected(value), and replace direct calls to widget.onDetected in
_pasteFromClipboard, _submit (and the analogous handlers around lines 93-107)
with calls to _emitDetectedOnce so every code path funnels through the one-time
emitter.
---
Outside diff comments:
In `@lib/features/settings/widgets/relay_management_card.dart`:
- Around line 64-127: Replace all hard-coded English strings in
_showAddRelayDialog (title, TextField hintText, TextButton labels "Cancel"/"Add"
and validation messages "Must start with wss://", "URL is too short", "Relay
already in list") with localized values from AppLocalizations (e.g.
AppLocalizations.of(context)!.relayAddTitle, relayHint, cancel, add,
relayErrorMustStartWith, relayErrorTooShort, relayErrorDuplicate) and call those
from inside the dialog builder using the dialog context; add matching l10n
keys/translations for each new string in the app localization files and update
the other similar relay dialog block (the second dialog around the other
relay-management code) the same way so both dialogs use AppLocalizations instead
of literals.
---
Duplicate comments:
In `@lib/features/settings/screens/settings_screen.dart`:
- Around line 33-47: The SettingsScreen currently uses hardcoded English
literals (e.g., AppBar title 'Settings', card titles/subtitles, and the
Lightning Address dialog text) which causes mixed-language UI; replace those
string literals by fetching localized strings from AppLocalizations (via
AppLocalizations.of(context) or the project's context.l10n helper) wherever you
build the UI in SettingsScreen and the related _settingsCard usages, including
the title in AppBar, the 'Language' title/subtitle (languageNameForCode can
remain but its label should be localized), the existing settings card
titles/subtitles, and any Lightning Address dialog text (the dialog builder
function); ensure you reference the existing localization keys (or add new ones)
and use them instead of hardcoded English so all rows and dialogs use localized
strings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5b756e92-d927-4e3d-b3f7-ae1032f7d045
📒 Files selected for processing (19)
lib/features/home/screens/home_screen.dartlib/features/settings/screens/connect_wallet_screen.dartlib/features/settings/screens/settings_screen.dartlib/features/settings/widgets/relay_management_card.dartlib/l10n/app_de.arblib/l10n/app_en.arblib/l10n/app_es.arblib/l10n/app_fr.arblib/l10n/app_it.arblib/l10n/app_localizations.dartlib/l10n/app_localizations_de.dartlib/l10n/app_localizations_en.dartlib/l10n/app_localizations_es.dartlib/l10n/app_localizations_fr.dartlib/l10n/app_localizations_it.dartlib/shared/widgets/countdown_timer.dartlib/shared/widgets/platform_aware_qr_scanner.dartrust/src/api/settings.rsrust/src/queue/outbox.rs
🚧 Files skipped from review as they are similar to previous changes (9)
- lib/features/settings/screens/connect_wallet_screen.dart
- rust/src/api/settings.rs
- lib/l10n/app_es.arb
- lib/shared/widgets/countdown_timer.dart
- rust/src/queue/outbox.rs
- lib/l10n/app_it.arb
- lib/l10n/app_localizations_it.dart
- lib/l10n/app_localizations_es.dart
- lib/l10n/app_en.arb
Comment on lines
+257
to
+269
| Future<void> _showThemeDialog(BuildContext context) async { | ||
| final current = ref.read(settingsProvider).themeMode; | ||
| await showDialog<void>( | ||
| context: context, | ||
| builder: (ctx) => SimpleDialog( | ||
| title: Text(AppLocalizations.of(ctx).appearanceDialogTitle), | ||
| children: ThemeMode.values.map((mode) { | ||
| return ListTile( | ||
| title: Text(_themeLabel(ctx, mode)), | ||
| trailing: mode == current ? const Icon(Icons.check) : null, | ||
| onTap: () { | ||
| ref.read(settingsProvider.notifier).setThemeMode(mode); | ||
| Navigator.of(ctx).pop(); |
Contributor
There was a problem hiding this comment.
Theme mode isn’t persisted yet.
Line 268 only calls setThemeMode(mode), and lib/features/settings/providers/settings_provider.dart:49-64 shows that setter just copyWiths the in-memory Riverpod state. The UI will re-theme immediately, but the choice will fall back to the default on the next cold start unless this path writes through to the backing settings store.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/features/settings/screens/settings_screen.dart` around lines 257 - 269,
The selected theme is only updated in-memory by calling
settingsProvider.notifier.setThemeMode from _showThemeDialog, so it’s not
persisted across restarts; modify the persistence flow so setThemeMode (in
SettingsNotifier inside settings_provider) writes the new theme into the backing
store (e.g., call your existing settingsRepository/Preferences API or add a
persistTheme/persistSettings method) after updating state, or alternatively have
_showThemeDialog call that persistence method immediately after setThemeMode;
update the code paths referencing setThemeMode/settingsProvider to ensure the
chosen ThemeMode is saved to disk/storage as well as to Riverpod state.
…persistence skip - settings_screen.dart: localize all hardcoded card titles/subtitles and Lightning Address dialog text using existing and new l10n keys (settingsScreenTitle, languageSettingTitle, defaultFiatCurrencyTitle, allCurrencies, tapToSetSubtitle, lightningAddressSettingTitle, nwcWalletSettingTitle, nwcConnectPrompt, nwcConnectedBalance, relaysSettingTitle, manageRelayConnections, pushNotificationsSettingTitle, manageNotificationPreferences, logReportSettingTitle, viewDiagnosticLogs, mostroNodeSettingTitle, lightningAddressDialogTitle, lightningAddressHintText, invalidLightningAddressFormat, clearButtonLabel, cancel, saveButtonLabel) - relay_management_card.dart: localize Add Relay dialog (title, hint, Cancel/Add buttons, three validation error messages) using new l10n keys; import AppLocalizations - platform_aware_qr_scanner.dart: add _hasEmitted guard to prevent web path from calling onDetected more than once; localize all web fallback strings (heading, Paste/Submit buttons, clipboard/empty error messages) using l10n; import AppLocalizations - l10n (all 5 ARBs): add 11 new keys — addButtonLabel, relayHintText, relayErrorMustStartWithWss, relayErrorUrlTooShort, relayErrorDuplicate, nwcConnectedBalance, pasteQrCodeHeading, pasteButtonLabel, clipboardEmptyError, enterValueError, pasteOrScanQrCode - theme persistence skipped: all settings are uniformly in-memory (Phase 18+ TODOs); singling out theme would be inconsistent No issues from flutter analyze. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced Apr 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
chat side panel on tablet+, hide bottom nav on desktop (AppBreakpoints)
on connection Online event; outbox tests (68 passing)
Summary by CodeRabbit
New Features
Bug Fixes