From 12196bef4f81a4a9faa0bf8b59f3c439d01fa07e Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Thu, 15 Oct 2026 23:57:39 +0600 Subject: [PATCH] fix(mobile): save typed coordinates in the location picker (#32460) --- mobile/lib/extensions/string_extensions.dart | 4 - .../lib/widgets/common/location_picker.dart | 68 +++++++++------- .../widgets/common/location_picker_test.dart | 80 +++++++++++++++++++ 3 files changed, 118 insertions(+), 34 deletions(-) create mode 100644 mobile/test/widgets/common/location_picker_test.dart diff --git a/mobile/lib/extensions/string_extensions.dart b/mobile/lib/extensions/string_extensions.dart index 1a4bd637d..4aa497249 100644 --- a/mobile/lib/extensions/string_extensions.dart +++ b/mobile/lib/extensions/string_extensions.dart @@ -22,10 +22,6 @@ extension DurationExtension on String { } } - double toDouble() { - return double.parse(this); - } - int toInt() { return int.parse(this); } diff --git a/mobile/lib/widgets/common/location_picker.dart b/mobile/lib/widgets/common/location_picker.dart index cf3240977..02a6e704e 100644 --- a/mobile/lib/widgets/common/location_picker.dart +++ b/mobile/lib/widgets/common/location_picker.dart @@ -1,9 +1,7 @@ import 'package:auto_route/auto_route.dart'; import 'package:flutter/material.dart'; -import 'package:flutter/services.dart'; import 'package:flutter_hooks/flutter_hooks.dart'; import 'package:immich_mobile/extensions/build_context_extensions.dart'; -import 'package:immich_mobile/extensions/string_extensions.dart'; import 'package:immich_mobile/generated/translations.g.dart'; import 'package:immich_mobile/routing/router.dart'; import 'package:maplibre_gl/maplibre_gl.dart'; @@ -21,14 +19,14 @@ class _LocationPicker extends HookWidget { const _LocationPicker({this.initialLatLng}); - bool _validateLat(String value) { - final l = double.tryParse(value); - return l != null && l > -90 && l < 90; + double? _parseLat(String value) { + final l = double.tryParse(value.replaceAll(',', '.')); + return l != null && l > -90 && l < 90 ? l : null; } - bool _validateLong(String value) { - final l = double.tryParse(value); - return l != null && l > -180 && l < 180; + double? _parseLong(String value) { + final l = double.tryParse(value.replaceAll(',', '.')); + return l != null && l > -180 && l < 180 ? l : null; } @override @@ -36,35 +34,39 @@ class _LocationPicker extends HookWidget { final latitude = useState(initialLatLng?.latitude ?? 0.0); final longitude = useState(initialLatLng?.longitude ?? 0.0); final latlng = LatLng(latitude.value, longitude.value); + final isLatitudeValid = useState(true); + final isLongitudeValid = useState(true); final latitiudeFocusNode = useFocusNode(); final longitudeFocusNode = useFocusNode(); final latitudeController = useTextEditingController(text: latitude.value.toStringAsFixed(4)); final longitudeController = useTextEditingController(text: longitude.value.toStringAsFixed(4)); - useEffect(() { - latitudeController.text = latitude.value.toStringAsFixed(4); - longitudeController.text = longitude.value.toStringAsFixed(4); - return null; - }, [latitude.value, longitude.value]); - Future onMapTap() async { final newLatLng = await context.pushRoute(MapLocationPickerRoute(initialLatLng: latlng)); if (newLatLng != null) { latitude.value = newLatLng.latitude; longitude.value = newLatLng.longitude; + latitudeController.text = newLatLng.latitude.toStringAsFixed(4); + longitudeController.text = newLatLng.longitude.toStringAsFixed(4); } } - void onLatitudeUpdated(double value) { - latitude.value = value; + void onLatitudeUpdated() { longitudeFocusNode.requestFocus(); } - void onLongitudeEditingCompleted(double value) { - longitude.value = value; + void onLongitudeEditingCompleted() { longitudeFocusNode.unfocus(); } + Future onUpdate() async { + isLatitudeValid.value = _parseLat(latitudeController.text) != null; + isLongitudeValid.value = _parseLong(longitudeController.text) != null; + if (isLatitudeValid.value && isLongitudeValid.value) { + await context.maybePop(LatLng(latitude.value, longitude.value)); + } + } + return AlertDialog( contentPadding: const EdgeInsets.all(30), alignment: Alignment.center, @@ -89,7 +91,9 @@ class _LocationPicker extends HookWidget { hintText: context.t.location_picker_latitude_hint, errorText: context.t.location_picker_latitude_error, focusNode: latitiudeFocusNode, - validator: _validateLat, + coordinate: latitude, + isValid: isLatitudeValid, + parse: _parseLat, onUpdated: onLatitudeUpdated, ), const SizedBox(height: 24), @@ -99,7 +103,9 @@ class _LocationPicker extends HookWidget { hintText: context.t.location_picker_longitude_hint, errorText: context.t.location_picker_longitude_error, focusNode: longitudeFocusNode, - validator: _validateLong, + coordinate: longitude, + isValid: isLongitudeValid, + parse: _parseLong, onUpdated: onLongitudeEditingCompleted, ), ], @@ -117,7 +123,7 @@ class _LocationPicker extends HookWidget { ), ), TextButton( - onPressed: () => context.maybePop(latlng), + onPressed: onUpdate, child: Text( context.t.action_common_update, style: context.textTheme.bodyMedium?.copyWith(fontWeight: FontWeight.w600, color: context.primaryColor), @@ -128,14 +134,16 @@ class _LocationPicker extends HookWidget { } } -class _ManualPickerInput extends HookWidget { +class _ManualPickerInput extends StatelessWidget { final TextEditingController controller; final String decorationText; final String hintText; final String errorText; final FocusNode focusNode; - final bool Function(String value) validator; - final Function(double value) onUpdated; + final ValueNotifier coordinate; + final ValueNotifier isValid; + final double? Function(String value) parse; + final VoidCallback onUpdated; const _ManualPickerInput({ required this.controller, @@ -143,17 +151,17 @@ class _ManualPickerInput extends HookWidget { required this.hintText, required this.errorText, required this.focusNode, - required this.validator, + required this.coordinate, + required this.isValid, + required this.parse, required this.onUpdated, }); @override Widget build(BuildContext context) { - final isValid = useState(true); - void onEditingComplete() { - isValid.value = validator(controller.text); + isValid.value = parse(controller.text) != null; if (isValid.value) { - onUpdated(controller.text.toDouble()); + onUpdated(); } } @@ -171,9 +179,9 @@ class _ManualPickerInput extends HookWidget { hintStyle: const TextStyle(fontWeight: FontWeight.normal, fontSize: 14), errorText: isValid.value ? null : errorText, ), + onChanged: (text) => coordinate.value = parse(text) ?? coordinate.value, onEditingComplete: onEditingComplete, keyboardType: const TextInputType.numberWithOptions(decimal: true, signed: true), - inputFormatters: [LengthLimitingTextInputFormatter(8)], onTapOutside: (_) => focusNode.unfocus(), ); } diff --git a/mobile/test/widgets/common/location_picker_test.dart b/mobile/test/widgets/common/location_picker_test.dart new file mode 100644 index 000000000..3e6f0221d --- /dev/null +++ b/mobile/test/widgets/common/location_picker_test.dart @@ -0,0 +1,80 @@ +import 'package:auto_route/auto_route.dart'; +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:immich_mobile/widgets/common/location_picker.dart'; +import 'package:maplibre_gl/maplibre_gl.dart'; + +import '../../widget_tester_extensions.dart'; + +void main() { + LatLng? picked; + + Future openPicker(WidgetTester tester, {LatLng? initialLatLng}) async { + picked = null; + final router = RootStackRouter.build( + routes: [ + AutoRoute( + initial: true, + page: PageInfo( + 'Home', + builder: (_) => Builder( + builder: (context) => TextButton( + onPressed: () async => + picked = await showLocationPicker(context: context, initialLatLng: initialLatLng), + child: const Text('open'), + ), + ), + ), + ), + ], + ); + await tester.pumpConsumerWidget(Router.withConfig(config: router.config())); + await tester.tap(find.text('open')); + await tester.pumpAndSettle(); + } + + Future typeAndUpdate(WidgetTester tester, String latitude, String longitude) async { + await tester.enterText(find.byType(TextField).at(0), latitude); + await tester.enterText(find.byType(TextField).at(1), longitude); + await tester.tap(find.text('Update')); + await tester.pumpAndSettle(); + } + + testWidgets('update saves both typed values when done was pressed in only one field', (tester) async { + await openPicker(tester); + await tester.enterText(find.byType(TextField).at(0), '37.7749'); + await tester.enterText(find.byType(TextField).at(1), '-122.4194'); + await tester.testTextInput.receiveAction(TextInputAction.done); + await tester.pump(); + await tester.tap(find.text('Update')); + await tester.pumpAndSettle(); + + expect(picked, const LatLng(37.7749, -122.4194)); + }); + + testWidgets('accepts a comma as the decimal separator', (tester) async { + await openPicker(tester); + await typeAndUpdate(tester, '45,4642', '9,19'); + + expect(picked, const LatLng(45.4642, 9.19)); + }); + + testWidgets('an out of range latitude shows the error and saves nothing', (tester) async { + await openPicker(tester); + await typeAndUpdate(tester, '95', '9.19'); + + expect(picked, isNull); + expect(find.text('Enter a valid latitude'), findsOneWidget); + }); + + testWidgets('an existing location that is not edited keeps its full value', (tester) async { + await openPicker(tester, initialLatLng: const LatLng(37.760078, -122.509567)); + await tester.tap(find.byType(TextField).at(0)); + await tester.testTextInput.receiveAction(TextInputAction.done); + await tester.pump(); + await tester.tap(find.text('Update')); + await tester.pumpAndSettle(); + + expect(picked, const LatLng(37.760078, -122.509567)); + }); +}