fix(mobile): dispose API-key dialog controller on any dismissal (#2399)

* fix(mobile): own the API-key dialog controller in its own State

_showApiKeyDialog created a TextEditingController in the parent State and
only disposed it in the Cancel/Sign-In handlers, so it leaked whenever the
dialog was dismissed via a barrier tap or the system back button.

Disposing it right after `await showDialog` (an earlier attempt) instead
risks disposing the controller while the dialog's TextField is still
mounted during the route's exit transition ("TextEditingController was
used after being disposed").

Extract the dialog into a small StatefulWidget (_ApiKeyLoginDialog) that
owns the controller and disposes it in State.dispose(), tying the
controller's lifecycle to the dialog's widget tree. The dialog pops
true/false/null and the screen shows the error snackbar on a failed
attempt.

flutter analyze: no new issues; flutter test: all green.

* test(mobile): cover ApiKeyLoginDialog controller disposal

Per review: add a widget test for the lifecycle contract that is the core
of this change. Exposes the dialog as `ApiKeyLoginDialog` (@visibleForTesting)
with an injectable controller, and asserts the controller is disposed on
all three dismissal paths — Cancel button, barrier tap, and system back —
by checking that using the controller afterward throws (a disposed
ChangeNotifier throws on reuse).

119 tests pass; flutter analyze: no new issues.

* test(mobile): derive the barrier-tap point from dialog geometry

Per review: replace the hard-coded Offset(10, 10) in the barrier-dismissal
test. Tapping the ModalBarrier widget directly hits the dialog that
occludes its centre, so instead tap halfway between the screen corner and
the dialog's top-left — a point derived from the dialog's real geometry,
resilient to layout changes, and always on the dismissible barrier.

* fix(mobile): keyboard submit + disable Sign In when API key is empty

- Add onSubmitted to the API key TextField so the keyboard Done/Enter key
  submits the form (no need to tap the button).
- Wire a controller listener to rebuild the dialog state and disable the
  Sign In ElevatedButton while the field is blank, giving clear feedback
  before any network call is made.
This commit is contained in:
ghost
2026-06-29 22:06:03 -07:00
committed by GitHub
parent 1b403d64e5
commit 6d3e39e588
2 changed files with 194 additions and 97 deletions

View File

@@ -88,105 +88,29 @@ class _LoginScreenState extends State<LoginScreen> {
}
}
void _showApiKeyDialog() {
final apiKeyController = TextEditingController();
final outerContext = context;
bool isLoading = false;
showDialog(
Future<void> _showApiKeyDialog() async {
// The dialog owns its TextEditingController and disposes it in its own
// State.dispose(), so the controller's lifecycle is tied to the dialog's
// widget tree: no leak on barrier/back dismissal, and it is never disposed
// out from under a still-mounted TextField during the exit transition.
final result = await showDialog<bool>(
context: context,
builder: (dialogContext) {
final dl = AppLocalizations.of(dialogContext);
return StatefulBuilder(
builder: (_, setDialogState) {
return AlertDialog(
title: Text(dl.loginApiKeyDialogTitle),
content: Column(
mainAxisSize: MainAxisSize.min,
children: [
Text(
dl.loginApiKeyDialogBody,
style:
Theme.of(outerContext).textTheme.bodyMedium?.copyWith(
color: Theme.of(outerContext)
.colorScheme
.onSurfaceVariant,
),
),
const SizedBox(height: 16),
TextField(
controller: apiKeyController,
decoration: InputDecoration(
labelText: dl.loginApiKeyLabel,
prefixIcon: const Icon(Icons.vpn_key_outlined),
),
obscureText: true,
maxLines: 1,
enabled: !isLoading,
),
],
),
actions: [
TextButton(
onPressed: isLoading
? null
: () {
apiKeyController.dispose();
Navigator.of(dialogContext).pop();
},
child: Text(dl.commonCancel),
),
ElevatedButton(
onPressed: isLoading
? null
: () async {
final apiKey = apiKeyController.text.trim();
if (apiKey.isEmpty) return;
setDialogState(() {
isLoading = true;
});
final authProvider = Provider.of<AuthProvider>(
outerContext,
listen: false,
);
final success = await authProvider.loginWithApiKey(
apiKey: apiKey,
);
if (!dialogContext.mounted) return;
final errorMsg = authProvider.errorMessage;
apiKeyController.dispose();
Navigator.of(dialogContext).pop();
if (!success && mounted) {
ScaffoldMessenger.of(outerContext).showSnackBar(
SnackBar(
content: Text(
errorMsg ?? dl.loginApiKeyInvalid,
),
backgroundColor:
Theme.of(outerContext).colorScheme.error,
),
);
}
},
child: isLoading
? const SizedBox(
height: 20,
width: 20,
child: CircularProgressIndicator(strokeWidth: 2),
)
: Text(dl.loginApiKeySignIn),
),
],
);
},
);
},
builder: (_) => const ApiKeyLoginDialog(),
);
// result: true = signed in, false = login attempt failed, null = dismissed.
if (result == false && mounted) {
final authProvider = Provider.of<AuthProvider>(context, listen: false);
ScaffoldMessenger.of(context).showSnackBar(
SnackBar(
content: Text(
authProvider.errorMessage ??
AppLocalizations.of(context).loginApiKeyInvalid,
),
backgroundColor: Theme.of(context).colorScheme.error,
),
);
}
}
Future<void> _handleLogin() async {
@@ -538,3 +462,101 @@ class _LoginScreenState extends State<LoginScreen> {
);
}
}
/// API-key login dialog. Owns its [TextEditingController] so it is disposed
/// with the dialog's State — never leaked (barrier/back dismissal) and never
/// disposed while the field is still mounted. Pops `true` on a successful
/// sign-in, `false` on a failed attempt, and `null` when dismissed.
@visibleForTesting
class ApiKeyLoginDialog extends StatefulWidget {
const ApiKeyLoginDialog({super.key, this.controller});
/// Test seam: a controller the dialog will adopt and dispose, so a test can
/// assert disposal. Production passes none and the dialog creates its own.
@visibleForTesting
final TextEditingController? controller;
@override
State<ApiKeyLoginDialog> createState() => _ApiKeyLoginDialogState();
}
class _ApiKeyLoginDialogState extends State<ApiKeyLoginDialog> {
late final TextEditingController _apiKeyController =
widget.controller ?? TextEditingController();
bool _isLoading = false;
@override
void initState() {
super.initState();
_apiKeyController.addListener(_onTextChanged);
}
void _onTextChanged() => setState(() {});
@override
void dispose() {
_apiKeyController.removeListener(_onTextChanged);
_apiKeyController.dispose();
super.dispose();
}
Future<void> _submit() async {
final apiKey = _apiKeyController.text.trim();
if (apiKey.isEmpty) return;
setState(() => _isLoading = true);
final authProvider = Provider.of<AuthProvider>(context, listen: false);
final success = await authProvider.loginWithApiKey(apiKey: apiKey);
if (!mounted) return;
Navigator.of(context).pop(success);
}
@override
Widget build(BuildContext context) {
final l = AppLocalizations.of(context);
return AlertDialog(
title: Text(l.loginApiKeyDialogTitle),
content: Column(
mainAxisSize: MainAxisSize.min,
children: [
Text(
l.loginApiKeyDialogBody,
style: Theme.of(context).textTheme.bodyMedium?.copyWith(
color: Theme.of(context).colorScheme.onSurfaceVariant,
),
),
const SizedBox(height: 16),
TextField(
controller: _apiKeyController,
decoration: InputDecoration(
labelText: l.loginApiKeyLabel,
prefixIcon: const Icon(Icons.vpn_key_outlined),
),
obscureText: true,
maxLines: 1,
enabled: !_isLoading,
onSubmitted: (_) => _submit(),
),
],
),
actions: [
TextButton(
onPressed: _isLoading ? null : () => Navigator.of(context).pop(),
child: Text(l.commonCancel),
),
ElevatedButton(
onPressed: _isLoading || _apiKeyController.text.trim().isEmpty ? null : _submit,
child: _isLoading
? const SizedBox(
height: 20,
width: 20,
child: CircularProgressIndicator(strokeWidth: 2),
)
: Text(l.loginApiKeySignIn),
),
],
);
}
}

View File

@@ -0,0 +1,75 @@
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:sure_mobile/l10n/app_localizations.dart';
import 'package:sure_mobile/screens/login_screen.dart';
void main() {
// Opens the dialog with an injected controller and returns it so the test can
// assert the dialog disposed it. A disposed ChangeNotifier throws when used
// again, which is how we verify disposal.
Future<TextEditingController> openDialog(WidgetTester tester) async {
final controller = TextEditingController();
await tester.pumpWidget(
MaterialApp(
localizationsDelegates: AppLocalizations.localizationsDelegates,
supportedLocales: AppLocalizations.supportedLocales,
home: Scaffold(
body: Builder(
builder: (context) => ElevatedButton(
onPressed: () => showDialog<bool>(
context: context,
builder: (_) => ApiKeyLoginDialog(controller: controller),
),
child: const Text('open'),
),
),
),
),
);
await tester.tap(find.text('open'));
await tester.pumpAndSettle();
expect(find.text('API Key Login'), findsOneWidget);
return controller;
}
void expectDisposed(TextEditingController controller) {
expect(() => controller.addListener(() {}), throwsA(isA<FlutterError>()));
}
testWidgets('disposes its controller when cancelled', (tester) async {
final controller = await openDialog(tester);
await tester.tap(find.text('Cancel'));
await tester.pumpAndSettle();
expect(find.text('API Key Login'), findsNothing);
expectDisposed(controller);
});
testWidgets('disposes its controller when dismissed by a barrier tap',
(tester) async {
final controller = await openDialog(tester);
// The barrier fills the screen but its centre is occluded by the dialog, so
// tap halfway between the screen corner and the dialog's top-left — a point
// derived from the dialog's real geometry (not a fixed coordinate) that is
// always on the dismissible barrier.
final dialogTopLeft = tester.getTopLeft(find.byType(AlertDialog));
await tester.tapAt(dialogTopLeft / 2);
await tester.pumpAndSettle();
expect(find.text('API Key Login'), findsNothing);
expectDisposed(controller);
});
testWidgets('disposes its controller when dismissed by the system back button',
(tester) async {
final controller = await openDialog(tester);
await tester.binding.handlePopRoute();
await tester.pumpAndSettle();
expect(find.text('API Key Login'), findsNothing);
expectDisposed(controller);
});
}