瀏覽代碼

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 <daniele@casaricci.it>
shenlong 1 年之前
父節點
當前提交
f8d26bd865

+ 5 - 0
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 }

+ 0 - 1
mobile/android/app/src/main/AndroidManifest.xml

@@ -55,7 +55,6 @@
 
 
   <uses-permission android:name="android.permission.INTERNET" />
-  <uses-permission android:name="android.permission.ACCESS_FINE_LOCATION" />
   <uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" android:maxSdkVersion="32"/>
   <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
   <uses-permission android:name="android.permission.ACCESS_MEDIA_LOCATION" />

+ 2 - 2
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;

+ 1 - 1
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: [

+ 120 - 108
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<MapPageBottomSheet> {
   // 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<MapPageBottomSheet> {
     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(<Asset>[]);
@@ -68,7 +71,7 @@ class AssetsInBoundBottomSheetState extends ConsumerState<MapPageBottomSheet> {
         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<MapPageBottomSheet> {
         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<MapPageBottomSheet> {
     }
 
     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<MapPageBottomSheet> {
           ? "${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<MapPageBottomSheet> {
               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<MapPageBottomSheet> {
                   ),
                 ),
                 Divider(
+                  height: 10,
                   color: Theme.of(context)
                       .textTheme
                       .displayLarge
@@ -226,6 +230,7 @@ class AssetsInBoundBottomSheetState extends ConsumerState<MapPageBottomSheet> {
       );
       return SingleChildScrollView(
         controller: scrollController,
+        physics: const ClampingScrollPhysics(),
         child: dragHandle,
       );
     }
@@ -238,118 +243,125 @@ class AssetsInBoundBottomSheetState extends ConsumerState<MapPageBottomSheet> {
         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,
+                ),
               ),
             ),
-          ),
-        ],
+          ],
+        ),
       ),
     );
   }

+ 13 - 4
mobile/lib/modules/map/views/map_page.dart

@@ -166,14 +166,15 @@ class MapPageState extends ConsumerState<MapPage> {
         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<MapPage> {
             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<MapPage> {
 
     return AnnotatedRegion<SystemUiOverlayStyle>(
       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

+ 2 - 2
mobile/openapi/lib/model/map_marker_response_dto.dart

@@ -57,8 +57,8 @@ class MapMarkerResponseDto {
 
       return MapMarkerResponseDto(
         id: mapValueOfType<String>(json, r'id')!,
-        lat: mapValueOfType<double>(json, r'lat')!,
-        lon: mapValueOfType<double>(json, r'lon')!,
+        lat: (mapValueOfType<num>(json, r'lat')!).toDouble(),
+        lon: (mapValueOfType<num>(json, r'lon')!).toDouble(),
       );
     }
     return null;

+ 9 - 0
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

+ 5 - 0
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<num>(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}}

+ 21 - 2
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<num>(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}}