From f8d26bd86531a4d741b9a2bf0ecb795be4975def Mon Sep 17 00:00:00 2001 From: shenlong <139912620+shalong-tanwen@users.noreply.github.com> Date: Mon, 4 Sep 2023 23:08:43 +0000 Subject: [PATCH] fix(mobile): map markers not loading with int coordinates (#3957) * fix(mobile): increase zoom-level for map zoom to asset * refactor(mobile): map-view - rename lastAssetOffsetInSheet * Workaround OpenAPI Dart generator bug * fix(mobile): map - increase appbar top padding * fix(mobile): navigation bar overlapping map bottom sheet * fix(mobile): map - do not animate the drag handle of bottom sheet on scroll * fix(mobile): F-Droid build failure due to map view * fix(mobile): remove jank on map asset marker update * fix(mobile): map view app-bar padding is made dynamic * fix(mobile): reduce debounce time in bottom sheet asset scroll * fix(mobile): bottom sheet - reduce drag handle total height --------- Co-authored-by: Daniele Ricci --- mobile/android/app/build.gradle | 5 + .../android/app/src/main/AndroidManifest.xml | 1 - .../lib/modules/map/ui/asset_marker_icon.dart | 4 +- .../lib/modules/map/ui/map_page_app_bar.dart | 2 +- .../modules/map/ui/map_page_bottom_sheet.dart | 228 +++++++++--------- mobile/lib/modules/map/views/map_page.dart | 17 +- .../lib/model/map_marker_response_dto.dart | 4 +- mobile/pubspec.yaml | 9 + .../native/native_class.mustache | 5 + .../native/native_class.mustache.patch | 23 +- 10 files changed, 178 insertions(+), 120 deletions(-) diff --git a/mobile/android/app/build.gradle b/mobile/android/app/build.gradle index 732a8c753..c46bcecfd 100644 --- a/mobile/android/app/build.gradle +++ b/mobile/android/app/build.gradle @@ -96,3 +96,8 @@ dependencies { implementation "com.github.bumptech.glide:glide:$glide_version" kapt "com.github.bumptech.glide:compiler:$glide_version" } + +// This is uncommented in F-Droid build script +//f configurations.all { +//f exclude group: 'com.google.android.gms' +//f } diff --git a/mobile/android/app/src/main/AndroidManifest.xml b/mobile/android/app/src/main/AndroidManifest.xml index 71ff1230d..a57d56c21 100644 --- a/mobile/android/app/src/main/AndroidManifest.xml +++ b/mobile/android/app/src/main/AndroidManifest.xml @@ -55,7 +55,6 @@ - diff --git a/mobile/lib/modules/map/ui/asset_marker_icon.dart b/mobile/lib/modules/map/ui/asset_marker_icon.dart index db6d1a10e..969c78e70 100644 --- a/mobile/lib/modules/map/ui/asset_marker_icon.dart +++ b/mobile/lib/modules/map/ui/asset_marker_icon.dart @@ -5,10 +5,10 @@ import 'package:immich_mobile/utils/image_url_builder.dart'; class AssetMarkerIcon extends StatelessWidget { const AssetMarkerIcon({ - Key? key, + super.key, required this.id, this.isDarkTheme = false, - }) : super(key: key); + }); final String id; final bool isDarkTheme; diff --git a/mobile/lib/modules/map/ui/map_page_app_bar.dart b/mobile/lib/modules/map/ui/map_page_app_bar.dart index c43cd9d3c..e9ed75cb1 100644 --- a/mobile/lib/modules/map/ui/map_page_app_bar.dart +++ b/mobile/lib/modules/map/ui/map_page_app_bar.dart @@ -122,7 +122,7 @@ class MapAppBar extends HookWidget implements PreferredSizeWidget { @override Widget build(BuildContext context) { return Padding( - padding: const EdgeInsets.only(top: 30), + padding: EdgeInsets.only(top: MediaQuery.of(context).padding.top + 15), child: Row( mainAxisAlignment: MainAxisAlignment.spaceBetween, children: [ diff --git a/mobile/lib/modules/map/ui/map_page_bottom_sheet.dart b/mobile/lib/modules/map/ui/map_page_bottom_sheet.dart index f74df4331..c9c3cb8aa 100644 --- a/mobile/lib/modules/map/ui/map_page_bottom_sheet.dart +++ b/mobile/lib/modules/map/ui/map_page_bottom_sheet.dart @@ -1,4 +1,5 @@ import 'dart:async'; +import 'dart:io'; import 'package:easy_localization/easy_localization.dart'; import 'package:flutter/material.dart'; @@ -41,7 +42,7 @@ class AssetsInBoundBottomSheetState extends ConsumerState { // Non-State variables bool userTappedOnMap = false; RenderList? _cachedRenderList; - int lastAssetOffsetInSheet = -1; + int assetOffsetInSheet = -1; late final DraggableScrollableController bottomSheetController; late final Debounce debounce; @@ -50,14 +51,16 @@ class AssetsInBoundBottomSheetState extends ConsumerState { super.initState(); bottomSheetController = DraggableScrollableController(); debounce = Debounce( - const Duration(milliseconds: 200), + const Duration(milliseconds: 100), ); } @override Widget build(BuildContext context) { - var isDarkMode = Theme.of(context).brightness == Brightness.dark; - double maxHeight = MediaQuery.of(context).size.height; + final isDarkMode = Theme.of(context).brightness == Brightness.dark; + final bottomPadding = + Platform.isAndroid ? MediaQuery.of(context).padding.bottom - 10 : 0.0; + final maxHeight = MediaQuery.of(context).size.height - bottomPadding; final isSheetScrolled = useState(false); final isSheetExpanded = useState(false); final assetsInBound = useState([]); @@ -68,7 +71,7 @@ class AssetsInBoundBottomSheetState extends ConsumerState { assetsInBound.value = event.assets; } else if (event is MapPageOnTapEvent) { userTappedOnMap = true; - lastAssetOffsetInSheet = -1; + assetOffsetInSheet = -1; bottomSheetController.animateTo( 0.1, duration: const Duration(milliseconds: 200), @@ -98,8 +101,8 @@ class AssetsInBoundBottomSheetState extends ConsumerState { columnOffset = columnOffset < renderElement.totalCount ? columnOffset : renderElement.totalCount - 1; - lastAssetOffsetInSheet = rowOffset + columnOffset; - final asset = _cachedRenderList?.allAssets?[lastAssetOffsetInSheet]; + assetOffsetInSheet = rowOffset + columnOffset; + final asset = _cachedRenderList?.allAssets?[assetOffsetInSheet]; userTappedOnMap = false; if (!userTappedOnMap && isSheetExpanded.value) { widget.bottomSheetEventSC.add( @@ -162,10 +165,10 @@ class AssetsInBoundBottomSheetState extends ConsumerState { } void onTapMapButton() { - if (lastAssetOffsetInSheet != -1) { + if (assetOffsetInSheet != -1) { widget.bottomSheetEventSC.add( MapPageZoomToAsset( - _cachedRenderList?.allAssets?[lastAssetOffsetInSheet], + _cachedRenderList?.allAssets?[assetOffsetInSheet], ), ); } @@ -176,7 +179,7 @@ class AssetsInBoundBottomSheetState extends ConsumerState { ? "${assetsInBound.value.length} photo${assetsInBound.value.length > 1 ? "s" : ""}" : "map_no_assets_in_bounds".tr(); final dragHandle = Container( - height: 75, + height: 60, width: double.infinity, decoration: BoxDecoration( color: isDarkMode ? Colors.grey[900] : Colors.grey[100], @@ -187,9 +190,9 @@ class AssetsInBoundBottomSheetState extends ConsumerState { crossAxisAlignment: CrossAxisAlignment.center, mainAxisAlignment: MainAxisAlignment.center, children: [ - const SizedBox(height: 12), + const SizedBox(height: 5), const CustomDraggingHandle(), - const SizedBox(height: 12), + const SizedBox(height: 15), Text( textToDisplay, style: TextStyle( @@ -199,6 +202,7 @@ class AssetsInBoundBottomSheetState extends ConsumerState { ), ), Divider( + height: 10, color: Theme.of(context) .textTheme .displayLarge @@ -226,6 +230,7 @@ class AssetsInBoundBottomSheetState extends ConsumerState { ); return SingleChildScrollView( controller: scrollController, + physics: const ClampingScrollPhysics(), child: dragHandle, ); } @@ -238,118 +243,125 @@ class AssetsInBoundBottomSheetState extends ConsumerState { if (!sheetExtended) { // reset state userTappedOnMap = false; - lastAssetOffsetInSheet = -1; + assetOffsetInSheet = -1; isSheetScrolled.value = false; } return true; }, - child: Stack( - children: [ - DraggableScrollableSheet( - controller: bottomSheetController, - initialChildSize: 0.1, - minChildSize: 0.1, - maxChildSize: 0.55, - snap: true, - builder: ( - BuildContext context, - ScrollController scrollController, - ) { - return Card( - color: isDarkMode ? Colors.grey[900] : Colors.grey[100], - surfaceTintColor: Colors.transparent, - elevation: 18.0, - margin: const EdgeInsets.all(0), - child: Column( - children: [ - buildDragHandle(scrollController), - if (isSheetExpanded.value && assetsInBound.value.isNotEmpty) - ref - .watch( - renderListProvider( - assetsInBound.value, - ), - ) - .when( - data: (renderList) { - _cachedRenderList = renderList; - final assetGrid = ImmichAssetGrid( - shrinkWrap: true, - renderList: renderList, - showDragScroll: false, - selectionActive: widget.selectionEnabled, - showMultiSelectIndicator: false, - listener: widget.selectionlistener, - visibleItemsListener: visibleItemsListener, - ); + child: Padding( + padding: EdgeInsets.only( + bottom: bottomPadding, + ), + child: Stack( + children: [ + DraggableScrollableSheet( + controller: bottomSheetController, + initialChildSize: 0.1, + minChildSize: 0.1, + maxChildSize: 0.55, + snap: true, + builder: ( + BuildContext context, + ScrollController scrollController, + ) { + return Card( + color: isDarkMode ? Colors.grey[900] : Colors.grey[100], + surfaceTintColor: Colors.transparent, + elevation: 18.0, + margin: const EdgeInsets.all(0), + child: Column( + children: [ + buildDragHandle(scrollController), + if (isSheetExpanded.value && + assetsInBound.value.isNotEmpty) + ref + .watch( + renderListProvider( + assetsInBound.value, + ), + ) + .when( + data: (renderList) { + _cachedRenderList = renderList; + final assetGrid = ImmichAssetGrid( + shrinkWrap: true, + renderList: renderList, + showDragScroll: false, + selectionActive: widget.selectionEnabled, + showMultiSelectIndicator: false, + listener: widget.selectionlistener, + visibleItemsListener: visibleItemsListener, + ); - return Expanded(child: assetGrid); - }, - error: (error, stackTrace) { - log.warning( - "Cannot get assets in the current map bounds ${error.toString()}", - error, - stackTrace, - ); - return const SizedBox.shrink(); - }, - loading: () => const SizedBox.shrink(), + return Expanded(child: assetGrid); + }, + error: (error, stackTrace) { + log.warning( + "Cannot get assets in the current map bounds ${error.toString()}", + error, + stackTrace, + ); + return const SizedBox.shrink(); + }, + loading: () => const SizedBox.shrink(), + ), + if (isSheetExpanded.value && assetsInBound.value.isEmpty) + Expanded( + child: SingleChildScrollView( + child: buildNoPhotosWidget(), ), - if (isSheetExpanded.value && assetsInBound.value.isEmpty) - Expanded( - child: SingleChildScrollView( - child: buildNoPhotosWidget(), ), - ), - ], + ], + ), + ); + }, + ), + Positioned( + bottom: maxHeight * currentExtend.value, + left: 0, + child: GestureDetector( + onTap: () => launchUrl( + Uri.parse('https://openstreetmap.org/copyright'), ), - ); - }, - ), - Positioned( - bottom: maxHeight * currentExtend.value, - left: 0, - child: GestureDetector( - onTap: () => launchUrl( - Uri.parse('https://openstreetmap.org/copyright'), - ), - child: ColoredBox( - color: - (widget.isDarkTheme ? Colors.grey[900] : Colors.grey[100])!, - child: Padding( - padding: const EdgeInsets.all(3), - child: Text( - '© OpenStreetMap contributors', - style: TextStyle( - fontSize: 6, - color: !widget.isDarkTheme - ? Colors.grey[900] - : Colors.grey[100], + child: ColoredBox( + color: (widget.isDarkTheme + ? Colors.grey[900] + : Colors.grey[100])!, + child: Padding( + padding: const EdgeInsets.all(3), + child: Text( + '© OpenStreetMap contributors', + style: TextStyle( + fontSize: 6, + color: !widget.isDarkTheme + ? Colors.grey[900] + : Colors.grey[100], + ), ), ), ), ), ), - ), - Positioned( - bottom: maxHeight * (0.14 + (currentExtend.value - 0.1)), - right: 15, - child: ElevatedButton( - onPressed: () => - widget.bottomSheetEventSC.add(const MapPageZoomToLocation()), - style: ElevatedButton.styleFrom( - shape: const CircleBorder(), - padding: const EdgeInsets.all(12), - ), - child: const Icon( - Icons.my_location, - size: 22, - fill: 1, + Positioned( + bottom: maxHeight * (0.14 + (currentExtend.value - 0.1)), + right: 15, + child: ElevatedButton( + onPressed: () => widget.bottomSheetEventSC + .add(const MapPageZoomToLocation()), + style: ElevatedButton.styleFrom( + shape: const CircleBorder(), + padding: const EdgeInsets.all(12), + ), + child: const Icon( + Icons.my_location, + size: 22, + fill: 1, + ), ), ), - ), - ], + ], + ), ), ); } diff --git a/mobile/lib/modules/map/views/map_page.dart b/mobile/lib/modules/map/views/map_page.dart index 379b209d9..d228c3873 100644 --- a/mobile/lib/modules/map/views/map_page.dart +++ b/mobile/lib/modules/map/views/map_page.dart @@ -166,14 +166,15 @@ class MapPageState extends ConsumerState { final mapMarker = mapMarkerData.value .firstWhereOrNull((e) => e.asset.id == assetInBottomSheet.id); if (mapMarker != null) { + const zoomLevel = 16.0; LatLng? newCenter = mapController.centerBoundsWithPadding( mapMarker.point, const Offset(0, -120), - zoomLevel: 6, + zoomLevel: zoomLevel, ); if (newCenter != null) { forceAssetUpdate = true; - mapController.move(newCenter, 6); + mapController.move(newCenter, zoomLevel); } } } @@ -385,6 +386,7 @@ class MapPageState extends ConsumerState { builder: (ctx) => GestureDetector( onTap: () => openAssetInViewer(closestAssetMarker.value!.asset), child: AssetMarkerIcon( + key: Key(closestAssetMarker.value!.asset.remoteId!), isDarkTheme: isDarkTheme, id: closestAssetMarker.value!.asset.remoteId!, ), @@ -421,8 +423,15 @@ class MapPageState extends ConsumerState { return AnnotatedRegion( value: SystemUiOverlayStyle( - statusBarColor: Colors.black.withOpacity(0.5), - statusBarIconBrightness: Brightness.light, + statusBarColor: + (isDarkTheme ? Colors.black : Colors.white).withOpacity(0.5), + statusBarIconBrightness: + isDarkTheme ? Brightness.light : Brightness.dark, + systemNavigationBarColor: + isDarkTheme ? Colors.grey[900] : Colors.grey[100], + systemNavigationBarIconBrightness: + isDarkTheme ? Brightness.light : Brightness.dark, + systemNavigationBarDividerColor: Colors.transparent, ), child: Theme( // Override app theme based on map theme diff --git a/mobile/openapi/lib/model/map_marker_response_dto.dart b/mobile/openapi/lib/model/map_marker_response_dto.dart index ded8ac167..7c7f4fad5 100644 --- a/mobile/openapi/lib/model/map_marker_response_dto.dart +++ b/mobile/openapi/lib/model/map_marker_response_dto.dart @@ -57,8 +57,8 @@ class MapMarkerResponseDto { return MapMarkerResponseDto( id: mapValueOfType(json, r'id')!, - lat: mapValueOfType(json, r'lat')!, - lon: mapValueOfType(json, r'lon')!, + lat: (mapValueOfType(json, r'lat')!).toDouble(), + lon: (mapValueOfType(json, r'lon')!).toDouble(), ); } return null; diff --git a/mobile/pubspec.yaml b/mobile/pubspec.yaml index 72df831ff..10d7f8f8c 100644 --- a/mobile/pubspec.yaml +++ b/mobile/pubspec.yaml @@ -60,6 +60,15 @@ dependencies: image_picker: ^0.8.5+3 # only used to select user profile image from system gallery -> we can simply select an image from within immich? logging: ^1.1.0 +# This is uncommented in F-Droid build script +# Taken from https://github.com/Myzel394/locus/blob/445013d22ec1d759027d4303bd65b30c5c8588c8/pubspec.yaml#L105 +#fdependency_overrides: +#f geolocator_android: +#f git: +#f url: https://github.com/Zverik/flutter-geolocator.git +#f ref: floss +#f path: geolocator_android + dev_dependencies: flutter_test: sdk: flutter diff --git a/server/openapi-generator/templates/mobile/serialization/native/native_class.mustache b/server/openapi-generator/templates/mobile/serialization/native/native_class.mustache index 25e4f66e2..7c6bdcf6e 100644 --- a/server/openapi-generator/templates/mobile/serialization/native/native_class.mustache +++ b/server/openapi-generator/templates/mobile/serialization/native/native_class.mustache @@ -204,6 +204,10 @@ class {{{classname}}} { ? {{#defaultValue}}{{{.}}}{{/defaultValue}}{{^defaultValue}}null{{/defaultValue}} : {{{datatypeWithEnum}}}.parse(json[r'{{{baseName}}}'].toString()), {{/isNumber}} + {{#isDouble}} + {{{name}}}: (mapValueOfType(json, r'{{{baseName}}}'){{#required}}{{^isNullable}}!{{/isNullable}}{{/required}}{{^required}}{{#defaultValue}} ?? {{{.}}}{{/defaultValue}}{{/required}}).toDouble(), + {{/isDouble}} + {{^isDouble}} {{^isNumber}} {{^isEnum}} {{{name}}}: mapValueOfType<{{{datatypeWithEnum}}}>(json, r'{{{baseName}}}'){{#required}}{{^isNullable}}!{{/isNullable}}{{/required}}{{^required}}{{#defaultValue}} ?? {{{.}}}{{/defaultValue}}{{/required}}, @@ -212,6 +216,7 @@ class {{{classname}}} { {{{name}}}: {{{enumName}}}.fromJson(json[r'{{{baseName}}}']){{#required}}{{^isNullable}}!{{/isNullable}}{{/required}}{{^required}}{{#defaultValue}} ?? {{{.}}}{{/defaultValue}}{{/required}}, {{/isEnum}} {{/isNumber}} + {{/isDouble}} {{/isMap}} {{/isArray}} {{/complexType}} diff --git a/server/openapi-generator/templates/mobile/serialization/native/native_class.mustache.patch b/server/openapi-generator/templates/mobile/serialization/native/native_class.mustache.patch index 98ea98a78..02e07f933 100644 --- a/server/openapi-generator/templates/mobile/serialization/native/native_class.mustache.patch +++ b/server/openapi-generator/templates/mobile/serialization/native/native_class.mustache.patch @@ -1,5 +1,5 @@ ---- native_class.mustache 2023-06-22 12:56:11.090350406 -0500 -+++ native_class1.mustache 2023-06-22 12:57:14.498184792 -0500 +--- native_class.mustache 2023-08-31 23:09:59.584269162 +0200 ++++ native_class1.mustache 2023-08-31 22:59:53.633083270 +0200 @@ -91,14 +91,14 @@ {{/isDateTime}} {{#isNullable}} @@ -35,3 +35,22 @@ return {{{classname}}}( {{#vars}} {{#isDateTime}} +@@ -215,6 +204,10 @@ + ? {{#defaultValue}}{{{.}}}{{/defaultValue}}{{^defaultValue}}null{{/defaultValue}} + : {{{datatypeWithEnum}}}.parse(json[r'{{{baseName}}}'].toString()), + {{/isNumber}} ++ {{#isDouble}} ++ {{{name}}}: (mapValueOfType(json, r'{{{baseName}}}'){{#required}}{{^isNullable}}!{{/isNullable}}{{/required}}{{^required}}{{#defaultValue}} ?? {{{.}}}{{/defaultValue}}{{/required}}).toDouble(), ++ {{/isDouble}} ++ {{^isDouble}} + {{^isNumber}} + {{^isEnum}} + {{{name}}}: mapValueOfType<{{{datatypeWithEnum}}}>(json, r'{{{baseName}}}'){{#required}}{{^isNullable}}!{{/isNullable}}{{/required}}{{^required}}{{#defaultValue}} ?? {{{.}}}{{/defaultValue}}{{/required}}, +@@ -223,6 +216,7 @@ + {{{name}}}: {{{enumName}}}.fromJson(json[r'{{{baseName}}}']){{#required}}{{^isNullable}}!{{/isNullable}}{{/required}}{{^required}}{{#defaultValue}} ?? {{{.}}}{{/defaultValue}}{{/required}}, + {{/isEnum}} + {{/isNumber}} ++ {{/isDouble}} + {{/isMap}} + {{/isArray}} + {{/complexType}}