From 8df538e7f7da1bcaae4420c3cd8ff4d8e7436d48 Mon Sep 17 00:00:00 2001 From: Thibault Deckers Date: Fri, 8 Oct 2021 18:31:01 +0900 Subject: [PATCH] map: update on entry removal --- lib/model/source/collection_lens.dart | 29 +++-- .../collection/entry_set_action_delegate.dart | 1 + lib/widgets/collection/grid/thumbnail.dart | 5 +- lib/widgets/common/map/geo_map.dart | 42 ++++++- lib/widgets/common/map/google/map.dart | 103 ++++++++++-------- lib/widgets/common/map/leaflet/map.dart | 12 +- lib/widgets/map/map_page.dart | 38 ++++--- lib/widgets/viewer/info/location_section.dart | 4 +- 8 files changed, 152 insertions(+), 82 deletions(-) diff --git a/lib/model/source/collection_lens.dart b/lib/model/source/collection_lens.dart index 8936faa5e..76c6026fe 100644 --- a/lib/model/source/collection_lens.dart +++ b/lib/model/source/collection_lens.dart @@ -46,8 +46,8 @@ class CollectionLens with ChangeNotifier { id ??= hashCode; if (listenToSource) { final sourceEvents = source.eventBus; - _subscriptions.add(sourceEvents.on().listen((e) => onEntryAdded(e.entries))); - _subscriptions.add(sourceEvents.on().listen((e) => onEntryRemoved(e.entries))); + _subscriptions.add(sourceEvents.on().listen((e) => _onEntryAdded(e.entries))); + _subscriptions.add(sourceEvents.on().listen((e) => _onEntryRemoved(e.entries))); _subscriptions.add(sourceEvents.on().listen((e) => _refresh())); _subscriptions.add(sourceEvents.on().listen((e) => _refresh())); _subscriptions.add(sourceEvents.on().listen((e) => _refresh())); @@ -73,6 +73,20 @@ class CollectionLens with ChangeNotifier { super.dispose(); } + CollectionLens copyWith({ + CollectionSource? source, + Set? filters, + bool? listenToSource, + List? fixedSelection, + }) => + CollectionLens( + source: source ?? this.source, + filters: filters ?? this.filters, + id: id, + listenToSource: listenToSource ?? this.listenToSource, + fixedSelection: fixedSelection ?? this.fixedSelection, + ); + bool get isEmpty => _filteredSortedEntries.isEmpty; int get entryCount => _filteredSortedEntries.length; @@ -103,16 +117,16 @@ class CollectionLens with ChangeNotifier { filters.removeWhere((old) => old.category == filter.category); } filters.add(filter); - onFilterChanged(); + _onFilterChanged(); } void removeFilter(CollectionFilter filter) { if (!filters.contains(filter)) return; filters.remove(filter); - onFilterChanged(); + _onFilterChanged(); } - void onFilterChanged() { + void _onFilterChanged() { _refresh(); filterChangeNotifier.notifyListeners(); } @@ -229,11 +243,11 @@ class CollectionLens with ChangeNotifier { } } - void onEntryAdded(Set? entries) { + void _onEntryAdded(Set? entries) { _refresh(); } - void onEntryRemoved(Set entries) { + void _onEntryRemoved(Set entries) { if (groupBursts) { // find impacted burst groups final obsoleteBurstEntries = {}; @@ -256,6 +270,7 @@ class CollectionLens with ChangeNotifier { // we should remove obsolete entries and sections // but do not apply sort/section // as section order change would surprise the user while browsing + fixedSelection?.removeWhere(entries.contains); _filteredSortedEntries.removeWhere(entries.contains); _sortedEntries?.removeWhere(entries.contains); sections.forEach((key, sectionEntries) => sectionEntries.removeWhere(entries.contains)); diff --git a/lib/widgets/collection/entry_set_action_delegate.dart b/lib/widgets/collection/entry_set_action_delegate.dart index e528c2e6c..10bd549f5 100644 --- a/lib/widgets/collection/entry_set_action_delegate.dart +++ b/lib/widgets/collection/entry_set_action_delegate.dart @@ -288,6 +288,7 @@ class EntrySetActionDelegate with FeedbackMixin, PermissionAwareMixin, SizeAware MaterialPageRoute( settings: const RouteSettings(name: MapPage.routeName), builder: (context) => MapPage( + // need collection with fresh ID to prevent hero from scroller on Map page to Collection page collection: CollectionLens( source: collection.source, filters: collection.filters, diff --git a/lib/widgets/collection/grid/thumbnail.dart b/lib/widgets/collection/grid/thumbnail.dart index 97d4db0f7..0a8e5b142 100644 --- a/lib/widgets/collection/grid/thumbnail.dart +++ b/lib/widgets/collection/grid/thumbnail.dart @@ -73,10 +73,7 @@ class InteractiveThumbnail extends StatelessWidget { TransparentMaterialPageRoute( settings: const RouteSettings(name: EntryViewerPage.routeName), pageBuilder: (context, a, sa) { - final viewerCollection = CollectionLens( - source: collection.source, - filters: collection.filters, - id: collection.id, + final viewerCollection = collection.copyWith( listenToSource: false, ); assert(viewerCollection.sortedEntries.map((e) => e.contentId).contains(entry.contentId)); diff --git a/lib/widgets/common/map/geo_map.dart b/lib/widgets/common/map/geo_map.dart index 04aaf83ae..3fe2d3905 100644 --- a/lib/widgets/common/map/geo_map.dart +++ b/lib/widgets/common/map/geo_map.dart @@ -6,6 +6,7 @@ import 'package:aves/model/settings/map_style.dart'; import 'package:aves/model/settings/settings.dart'; import 'package:aves/services/common/services.dart'; import 'package:aves/theme/durations.dart'; +import 'package:aves/utils/change_notifier.dart'; import 'package:aves/utils/constants.dart'; import 'package:aves/utils/math_utils.dart'; import 'package:aves/widgets/common/map/attribution.dart'; @@ -27,6 +28,7 @@ import 'package:provider/provider.dart'; class GeoMap extends StatefulWidget { final AvesMapController? controller; + final Listenable? collectionListenable; final List entries; final AvesEntry? initialEntry; final ValueNotifier isAnimatingNotifier; @@ -42,6 +44,7 @@ class GeoMap extends StatefulWidget { const GeoMap({ Key? key, this.controller, + this.collectionListenable, required this.entries, this.initialEntry, required this.isAnimatingNotifier, @@ -63,8 +66,9 @@ class _GeoMapState extends State { // so we prevent loading it while scrolling or animating bool _googleMapsLoaded = false; late final ValueNotifier _boundsNotifier; - late final Fluster _defaultMarkerCluster; + Fluster? _defaultMarkerCluster; Fluster? _slowMarkerCluster; + final AChangeNotifier _clusterChangeNotifier = AChangeNotifier(); List get entries => widget.entries; @@ -84,7 +88,29 @@ class _GeoMapState extends State { _boundsNotifier = ValueNotifier(bounds.copyWith( zoom: max(bounds.zoom, minInitialZoom), )); - _defaultMarkerCluster = _buildFluster(); + _registerWidget(widget); + _onCollectionChanged(); + } + + @override + void didUpdateWidget(covariant GeoMap oldWidget) { + super.didUpdateWidget(oldWidget); + _unregisterWidget(oldWidget); + _registerWidget(widget); + } + + @override + void dispose() { + _unregisterWidget(widget); + super.dispose(); + } + + void _registerWidget(GeoMap widget) { + widget.collectionListenable?.addListener(_onCollectionChanged); + } + + void _unregisterWidget(GeoMap widget) { + widget.collectionListenable?.removeListener(_onCollectionChanged); } @override @@ -99,7 +125,7 @@ class _GeoMapState extends State { return {geoEntry.entry!}; } - var points = _defaultMarkerCluster.points(clusterId); + var points = _defaultMarkerCluster?.points(clusterId) ?? []; if (points.length != geoEntry.pointsSize) { // `Fluster.points()` method does not always return all the points contained in a cluster // the higher `nodeSize` is, the higher the chance to get all the points (i.e. as many as the cluster `pointsSize`) @@ -144,6 +170,7 @@ class _GeoMapState extends State { Widget child = isGoogleMaps ? EntryGoogleMap( controller: widget.controller, + clusterListenable: _clusterChangeNotifier, boundsNotifier: _boundsNotifier, minZoom: 0, maxZoom: 20, @@ -158,6 +185,7 @@ class _GeoMapState extends State { ) : EntryLeafletMap( controller: widget.controller, + clusterListenable: _clusterChangeNotifier, boundsNotifier: _boundsNotifier, minZoom: 2, maxZoom: 16, @@ -237,6 +265,12 @@ class _GeoMapState extends State { ); } + void _onCollectionChanged() { + _defaultMarkerCluster = _buildFluster(); + _slowMarkerCluster = null; + _clusterChangeNotifier.notifyListeners(); + } + Fluster _buildFluster({int nodeSize = 64}) { final markers = entries.map((entry) { final latLng = entry.latLng!; @@ -266,7 +300,7 @@ class _GeoMapState extends State { Map _buildMarkerClusters() { final bounds = _boundsNotifier.value; - final geoEntries = _defaultMarkerCluster.clusters(bounds.boundingBox, bounds.zoom.round()); + final geoEntries = _defaultMarkerCluster?.clusters(bounds.boundingBox, bounds.zoom.round()) ?? []; return Map.fromEntries(geoEntries.map((v) { if (v.isCluster!) { final uri = v.childMarkerId; diff --git a/lib/widgets/common/map/google/map.dart b/lib/widgets/common/map/google/map.dart index f76734750..e4f6d4485 100644 --- a/lib/widgets/common/map/google/map.dart +++ b/lib/widgets/common/map/google/map.dart @@ -21,6 +21,7 @@ import 'package:provider/provider.dart'; class EntryGoogleMap extends StatefulWidget { final AvesMapController? controller; + final Listenable clusterListenable; final ValueNotifier boundsNotifier; final double? minZoom, maxZoom; final EntryMapStyle style; @@ -35,6 +36,7 @@ class EntryGoogleMap extends StatefulWidget { const EntryGoogleMap({ Key? key, this.controller, + required this.clusterListenable, required this.boundsNotifier, this.minZoom, this.maxZoom, @@ -93,9 +95,11 @@ class _EntryGoogleMapState extends State with WidgetsBindingObse if (avesMapController != null) { _subscriptions.add(avesMapController.moveCommands.listen((event) => _moveTo(_toGoogleLatLng(event.latLng)))); } + widget.clusterListenable.addListener(_updateMarkers); } void _unregisterWidget(EntryGoogleMap widget) { + widget.clusterListenable.removeListener(_updateMarkers); _subscriptions ..forEach((sub) => sub.cancel()) ..clear(); @@ -167,53 +171,54 @@ class _EntryGoogleMapState extends State with WidgetsBindingObse final interactive = context.select((v) => v.interactive); return ValueListenableBuilder( - valueListenable: widget.dotEntryNotifier ?? ValueNotifier(null), - builder: (context, dotEntry, child) { - return GoogleMap( - initialCameraPosition: CameraPosition( - bearing: -bounds.rotation, - target: _toGoogleLatLng(bounds.center), - zoom: bounds.zoom, - ), - onMapCreated: (controller) async { - _googleMapController = controller; - final zoom = await controller.getZoomLevel(); - await _updateVisibleRegion(zoom: zoom, rotation: bounds.rotation); - setState(() {}); - }, - // compass disabled to use provider agnostic controls - compassEnabled: false, - mapToolbarEnabled: false, - mapType: _toMapType(widget.style), - minMaxZoomPreference: MinMaxZoomPreference(widget.minZoom, widget.maxZoom), - rotateGesturesEnabled: true, - scrollGesturesEnabled: interactive, - // zoom controls disabled to use provider agnostic controls - zoomControlsEnabled: false, - zoomGesturesEnabled: interactive, - // lite mode disabled because it lacks camera animation - liteModeEnabled: false, - // tilt disabled to match leaflet - tiltGesturesEnabled: false, - myLocationEnabled: false, - myLocationButtonEnabled: false, - markers: { - ...markers, - if (dotEntry != null && _dotMarkerBitmap != null) - Marker( - markerId: const MarkerId('dot'), - anchor: const Offset(.5, .5), - consumeTapEvents: true, - icon: BitmapDescriptor.fromBytes(_dotMarkerBitmap!), - position: _toGoogleLatLng(dotEntry.latLng!), - zIndex: 1, - ) - }, - onCameraMove: (position) => _updateVisibleRegion(zoom: position.zoom, rotation: -position.bearing), - onCameraIdle: _onIdle, - onTap: (position) => widget.onMapTap?.call(), - ); - }); + valueListenable: widget.dotEntryNotifier ?? ValueNotifier(null), + builder: (context, dotEntry, child) { + return GoogleMap( + initialCameraPosition: CameraPosition( + bearing: -bounds.rotation, + target: _toGoogleLatLng(bounds.center), + zoom: bounds.zoom, + ), + onMapCreated: (controller) async { + _googleMapController = controller; + final zoom = await controller.getZoomLevel(); + await _updateVisibleRegion(zoom: zoom, rotation: bounds.rotation); + setState(() {}); + }, + // compass disabled to use provider agnostic controls + compassEnabled: false, + mapToolbarEnabled: false, + mapType: _toMapType(widget.style), + minMaxZoomPreference: MinMaxZoomPreference(widget.minZoom, widget.maxZoom), + rotateGesturesEnabled: true, + scrollGesturesEnabled: interactive, + // zoom controls disabled to use provider agnostic controls + zoomControlsEnabled: false, + zoomGesturesEnabled: interactive, + // lite mode disabled because it lacks camera animation + liteModeEnabled: false, + // tilt disabled to match leaflet + tiltGesturesEnabled: false, + myLocationEnabled: false, + myLocationButtonEnabled: false, + markers: { + ...markers, + if (dotEntry != null && _dotMarkerBitmap != null) + Marker( + markerId: const MarkerId('dot'), + anchor: const Offset(.5, .5), + consumeTapEvents: true, + icon: BitmapDescriptor.fromBytes(_dotMarkerBitmap!), + position: _toGoogleLatLng(dotEntry.latLng!), + zIndex: 1, + ) + }, + onCameraMove: (position) => _updateVisibleRegion(zoom: position.zoom, rotation: -position.bearing), + onCameraIdle: _onIdle, + onTap: (position) => widget.onMapTap?.call(), + ); + }, + ); }, ); } @@ -221,6 +226,10 @@ class _EntryGoogleMapState extends State with WidgetsBindingObse void _onIdle() { if (!mounted) return; widget.controller?.notifyIdle(bounds); + _updateMarkers(); + } + + void _updateMarkers() { setState(() => _geoEntryByMarkerKey = widget.markerClusterBuilder()); } diff --git a/lib/widgets/common/map/leaflet/map.dart b/lib/widgets/common/map/leaflet/map.dart index baf91536f..cfdcb0eb5 100644 --- a/lib/widgets/common/map/leaflet/map.dart +++ b/lib/widgets/common/map/leaflet/map.dart @@ -23,6 +23,7 @@ import 'package:provider/provider.dart'; class EntryLeafletMap extends StatefulWidget { final AvesMapController? controller; + final Listenable clusterListenable; final ValueNotifier boundsNotifier; final double minZoom, maxZoom; final EntryMapStyle style; @@ -38,6 +39,7 @@ class EntryLeafletMap extends StatefulWidget { const EntryLeafletMap({ Key? key, this.controller, + required this.clusterListenable, required this.boundsNotifier, this.minZoom = 0, this.maxZoom = 22, @@ -96,11 +98,13 @@ class _EntryLeafletMapState extends State with TickerProviderSt _subscriptions.add(avesMapController.moveCommands.listen((event) => _moveTo(event.latLng))); } _subscriptions.add(_leafletMapController.mapEventStream.listen((event) => _updateVisibleRegion())); - boundsNotifier.addListener(_onBoundsChange); + widget.clusterListenable.addListener(_updateMarkers); + widget.boundsNotifier.addListener(_onBoundsChange); } void _unregisterWidget(EntryLeafletMap widget) { - boundsNotifier.removeListener(_onBoundsChange); + widget.clusterListenable.removeListener(_updateMarkers); + widget.boundsNotifier.removeListener(_onBoundsChange); _subscriptions ..forEach((sub) => sub.cancel()) ..clear(); @@ -216,6 +220,10 @@ class _EntryLeafletMapState extends State with TickerProviderSt void _onIdle() { if (!mounted) return; widget.controller?.notifyIdle(bounds); + _updateMarkers(); + } + + void _updateMarkers() { setState(() => _geoEntryByMarkerKey = widget.markerClusterBuilder()); } diff --git a/lib/widgets/map/map_page.dart b/lib/widgets/map/map_page.dart index e4c6d2140..b7a7df05a 100644 --- a/lib/widgets/map/map_page.dart +++ b/lib/widgets/map/map_page.dart @@ -221,6 +221,7 @@ class _MapPageContentState extends State with SingleTickerProvid // key is expected by test driver key: const Key('map_view'), controller: _mapController, + collectionListenable: openingCollection, entries: openingCollection.sortedEntries, initialEntry: widget.initialEntry, isAnimatingNotifier: _isPageAnimatingNotifier, @@ -255,15 +256,21 @@ class _MapPageContentState extends State with SingleTickerProvid builder: (context, mqWidth, child) => ValueListenableBuilder( valueListenable: _regionCollectionNotifier, builder: (context, regionCollection, child) { - final regionEntries = regionCollection?.sortedEntries ?? []; - return ThumbnailScroller( - availableWidth: mqWidth, - entryCount: regionEntries.length, - entryBuilder: (index) => index < regionEntries.length ? regionEntries[index] : null, - indexNotifier: _selectedIndexNotifier, - onTap: _onThumbnailTap, - heroTagger: (entry) => Object.hashAll([regionCollection?.id, entry.uri]), - highlightable: true, + return AnimatedBuilder( + // update when entries are added/removed + animation: regionCollection ?? ChangeNotifier(), + builder: (context, child) { + final regionEntries = regionCollection?.sortedEntries ?? []; + return ThumbnailScroller( + availableWidth: mqWidth, + entryCount: regionEntries.length, + entryBuilder: (index) => index < regionEntries.length ? regionEntries[index] : null, + indexNotifier: _selectedIndexNotifier, + onTap: _onThumbnailTap, + heroTagger: (entry) => Object.hashAll([regionCollection?.id, entry.uri]), + highlightable: true, + ); + }, ); }, ), @@ -295,14 +302,11 @@ class _MapPageContentState extends State with SingleTickerProvid selectedEntry = selectedIndex != null && selectedIndex < regionEntries.length ? regionEntries[selectedIndex] : null; } - _regionCollectionNotifier.value = CollectionLens( - source: openingCollection.source, - listenToSource: false, - fixedSelection: openingCollection.sortedEntries, - filters: [ + _regionCollectionNotifier.value = openingCollection.copyWith( + filters: { ...openingCollection.filters.whereNot((v) => v is CoordinateFilter), CoordinateFilter(bounds.sw, bounds.ne), - ], + }, ); // get entries from the new collection, so the entry order is the same @@ -352,7 +356,9 @@ class _MapPageContentState extends State with SingleTickerProvid settings: const RouteSettings(name: EntryViewerPage.routeName), pageBuilder: (context, a, sa) { return EntryViewerPage( - collection: regionCollection, + collection: regionCollection?.copyWith( + listenToSource: false, + ), initialEntry: initialEntry, ); }, diff --git a/lib/widgets/viewer/info/location_section.dart b/lib/widgets/viewer/info/location_section.dart index 3272d3e21..9edcbf933 100644 --- a/lib/widgets/viewer/info/location_section.dart +++ b/lib/widgets/viewer/info/location_section.dart @@ -126,8 +126,8 @@ class _LocationSectionState extends State { MaterialPageRoute( settings: const RouteSettings(name: MapPage.routeName), builder: (context) => MapPage( - collection: CollectionLens( - source: baseCollection.source, + collection: baseCollection.copyWith( + listenToSource: true, fixedSelection: baseCollection.sortedEntries.where((entry) => entry.hasGps).toList(), ), initialEntry: entry,