Refactor State methods to use idiomatic mounted/context

[dougborg]

Stack order: 4/5 — merge after PR3 (RadioGroup)

Summary

• Refactor State subclass methods to use mounted and this.context instead of passing BuildContext as parameters
• Aligns with Flutter best practices: State objects already have access to context
• Fix three async safety gaps where mounted checks were missing after awaits
• Add 97 unit/integration tests for the state management layer (SessionState, UploadQueueState, cross-module coordination)

Why mounted checks matter

After any await, the widget may be disposed. Using context or Navigator on a disposed widget throws a FlutterError. The refactor to use this.context (from State) instead of captured BuildContext parameters is the Flutter-idiomatic pattern, and combined with mounted checks, prevents using stale context.

Mounted check fixes

Three methods had async gaps without guards — if the sheet/dialog is dismissed during the async operation, these would crash on stale context:

• AddNodeSheet._checkSubmissionGuideAndProceed: guards after hasSeenSubmissionGuide() and showDialog() awaits
• EditNodeSheet._checkSubmissionGuideAndProceed: identical pattern
• DownloadAreaDialog download button: captures AppState before async gap, guards after getOfflineAreaDir(), and guards in catch block

State management tests

The tests verify the ChangeNotifier layer invariants that prevent "stuck state" bugs:

Test file	Tests	What it covers
session_state_test.dart	57	Session lifecycle (start/commit/cancel clears correctly), dirty checking (only notifies on actual changes), direction management, commit guards, changeset comment generation
upload_queue_state_test.dart	27	Queue operations, cache synchronization (temp nodes created/cleaned up correctly for create/edit/extract/delete), direction formatting with FOV, queue persistence round-trip
app_state_integration_test.dart	13	Full commit flows (session→queue), profile deletion cancels sessions, double-commit safety, notification propagation

Small refactor: UploadQueueState now accepts injectable MapDataProvider and NodeProviderWithCache via optional constructor parameters (defaults to existing singletons). Production code unchanged.

Test plan

• flutter test test/state/ — all 97 new tests pass
• flutter test — all 127 tests pass (no regressions)
• Manual verification: open add/edit node sheet, dismiss during submission flow — no crash

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR refactors State class methods to follow idiomatic Flutter patterns by using mounted and this.context instead of passing BuildContext as parameters. Additionally, it updates deprecated APIs and removes unused code.

Changes:

• Refactored State subclass methods in add_node_sheet.dart, edit_node_sheet.dart, and other widgets to access context directly instead of passing it as a parameter
• Updated deprecated withOpacity() API calls to withValues(alpha:) throughout the codebase
• Introduced RadioGroup widget wrapper in language_section.dart and refine_tags_sheet.dart
• Added proper mounted checks after async operations
• Removed unused imports, variables, and dead code
• Updated test configurations and fixed minor code style issues

Reviewed changes

Copilot reviewed 80 out of 81 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
lib/widgets/add_node_sheet.dart	Refactored async methods to use this.context and added mounted checks
lib/widgets/edit_node_sheet.dart	Refactored async methods to use this.context and added mounted checks
lib/widgets/download_area_dialog.dart	Extracted method for download logic and added mounted checks
lib/widgets/refine_tags_sheet.dart	Introduced RadioGroup wrapper for radio button management
lib/screens/settings/sections/language_section.dart	Introduced RadioGroup wrapper for language selection
lib/widgets/*.dart (multiple files)	Updated withOpacity to withValues API
lib/services/*.dart (multiple files)	Replaced print with debugPrint, removed unused code
lib/screens/*.dart (multiple files)	Updated withOpacity to withValues, removed unused imports
test/services/deflock_tile_provider_test.dart	Added mocktail for mocking, updated tests
test/models/tile_provider_test.dart	Updated test expectations for API key requirements
scripts/validate_localizations.dart	Replaced print with debugPrint
pubspec.yaml	Added path_provider, collection, and flutter_lints dependencies

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 77 out of 79 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 80 out of 82 changed files in this pull request and generated 4 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 79 out of 81 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (3)

lib/screens/settings/sections/language_section.dart:51

• _setLanguage awaits LocalizationService.instance.setLanguage(...) and then calls setState unconditionally. Add if (!mounted) return; after the await to avoid setState() called after dispose() if the settings sheet is closed mid-operation.

  Future<void> _setLanguage(String? languageCode) async {
    await LocalizationService.instance.setLanguage(languageCode);
    setState(() {
      _selectedLanguage = languageCode;
    });
  }

lib/widgets/add_node_sheet.dart:155

• _commitWithoutCheck pops the sheet and then immediately uses ScaffoldMessenger.of(context). Once the sheet is popped, this context can be deactivated. Capture the messenger before popping (e.g., store final messenger = ScaffoldMessenger.of(context)), or show the SnackBar from an ancestor context, to avoid stale-context exceptions.
  lib/widgets/edit_node_sheet.dart:153
• _commitWithoutCheck pops the sheet and then calls ScaffoldMessenger.of(context). After Navigator.pop, the sheet context may be deactivated, which can throw at runtime. Capture the ScaffoldMessengerState (or an ancestor context) before popping, then show the SnackBar via that captured messenger.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

lib/widgets/add_node_sheet.dart:155

• _commitWithoutCheck calls Navigator.pop(context) and then immediately looks up ScaffoldMessenger.of(context) to show a SnackBar. After popping the bottom sheet, this context can be deactivated/disposed, which can trigger “Looking up a deactivated widget's ancestor” / stale-context errors. Capture the ScaffoldMessengerState (or a parent context) before popping, or show the SnackBar before closing the sheet.

  void _commitWithoutCheck() {
    final appState = context.read<AppState>();
    final locService = LocalizationService.instance;
    appState.commitSession();
    Navigator.pop(context);
    ScaffoldMessenger.of(context).showSnackBar(
      SnackBar(content: Text(locService.t('node.queuedForUpload'))),
    );

lib/widgets/edit_node_sheet.dart:153

• _commitWithoutCheck pops the sheet (Navigator.pop(context)) and then uses ScaffoldMessenger.of(context) to show a SnackBar. Once the sheet is popped, this context may be deactivated, causing runtime errors. Capture the messenger (or use a stable parent/root context) before popping, or show the SnackBar before closing the sheet.

  void _commitWithoutCheck() {
    final appState = context.read<AppState>();
    final locService = LocalizationService.instance;
    appState.commitEditSession();
    Navigator.pop(context);
    ScaffoldMessenger.of(context).showSnackBar(
      SnackBar(content: Text(locService.t('node.editQueuedForUpload'))),
    );

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The helper _createQueue states “Void methods are auto-stubbed by mocktail — no explicit stubs needed.”, but multiple tests still add explicit when(...).thenReturn(null) stubs for void methods (e.g., addOrUpdate, notifyListeners). This is contradictory and makes it harder to tell when stubbing is actually required; either remove the stubs or update the comment (or centralize any required stubbing in _createQueue).

Suggested change

	when(() => mockCache.addOrUpdate(any())).thenReturn(null);
	when(() => mockProvider.notifyListeners()).thenReturn(null);

[Copilot]

This test asserts and documents behavior that appears unintended: updateEditSession(profile: ...) clears a previously detected operatorProfile unless it’s passed explicitly. Since SessionState.updateEditSession has logic/commentary about restoring the detected operator profile on profile changes, codifying the current “operator gets nulled” behavior will make a future bugfix look like a regression. Consider either (a) asserting the intended behavior and fixing SessionState.updateEditSession, or (b) renaming the test / adding a TODO to clearly mark it as documenting a known issue.