From c4f084f4a50974285b102e70d33a0d259925c881 Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Thu, 9 Jul 2026 02:22:35 +0600 Subject: [PATCH] fix(mobile): play videos from search results and folder view (#29693) * fix(mobile): play videos from search results and folder view * pr suggestion --------- Co-authored-by: Alex --- .../domain/models/asset/base_asset.model.dart | 11 +++++ .../asset_viewer/asset_page.widget.dart | 4 +- .../test/domain/models/base_asset_test.dart | 46 +++++++++++++++++++ 3 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 mobile/test/domain/models/base_asset_test.dart diff --git a/mobile/lib/domain/models/asset/base_asset.model.dart b/mobile/lib/domain/models/asset/base_asset.model.dart index b458a64a7f..3397dbc6bd 100644 --- a/mobile/lib/domain/models/asset/base_asset.model.dart +++ b/mobile/lib/domain/models/asset/base_asset.model.dart @@ -61,6 +61,17 @@ sealed class BaseAsset { bool get isLocalOnly => storage == AssetState.local; bool get isRemoteOnly => storage == AssetState.remote; + // Same asset even if localId is known on one side but not the other (heroTag isn't stable then) + bool refersToSameAsset(BaseAsset other) { + if (remoteId != null && other.remoteId != null) { + return remoteId == other.remoteId; + } + if (localId != null && other.localId != null) { + return localId == other.localId; + } + return checksum != null && checksum == other.checksum; + } + bool get isEditable => false; // Overridden in subclasses diff --git a/mobile/lib/presentation/widgets/asset_viewer/asset_page.widget.dart b/mobile/lib/presentation/widgets/asset_viewer/asset_page.widget.dart index 6c974376a1..96331477dd 100644 --- a/mobile/lib/presentation/widgets/asset_viewer/asset_page.widget.dart +++ b/mobile/lib/presentation/widgets/asset_viewer/asset_page.widget.dart @@ -395,7 +395,7 @@ class _AssetPageState extends ConsumerState { @override Widget build(BuildContext context) { - final currentHeroTag = ref.watch(assetViewerProvider.select((s) => s.currentAsset?.heroTag)); + final currentAsset = ref.watch(assetViewerProvider.select((s) => s.currentAsset)); _showingDetails = ref.watch(assetViewerProvider.select((s) => s.showingDetails)); final stackIndex = ref.watch(assetViewerProvider.select((s) => s.stackIndex)); final isPlayingMotionVideo = ref.watch(isPlayingMotionVideoProvider); @@ -414,7 +414,7 @@ class _AssetPageState extends ConsumerState { displayAsset = stackChildren.elementAt(stackIndex); } - final isCurrent = currentHeroTag == displayAsset.heroTag; + final isCurrent = currentAsset != null && currentAsset.refersToSameAsset(displayAsset); final viewportWidth = MediaQuery.widthOf(context); final viewportHeight = MediaQuery.heightOf(context); diff --git a/mobile/test/domain/models/base_asset_test.dart b/mobile/test/domain/models/base_asset_test.dart new file mode 100644 index 0000000000..4c2eb3ca38 --- /dev/null +++ b/mobile/test/domain/models/base_asset_test.dart @@ -0,0 +1,46 @@ +import 'package:flutter_test/flutter_test.dart'; + +import '../../unit/factories/local_asset_factory.dart'; +import '../../unit/factories/remote_asset_factory.dart'; + +void main() { + group('BaseAsset.refersToSameAsset', () { + test('search/folder copy (localId null) matches the merged DB copy (localId set)', () { + // #29472: search and folder assets arrive with localId null, then the viewer + // watches the DB copy which fills localId. heroTag embeds localId, so it + // diverges for the same asset and isCurrent used to go false. + final searchCopy = RemoteAssetFactory.create(id: 'asset-1'); + final mergedCopy = searchCopy.copyWith(localId: 'local-1'); + + expect(searchCopy.localId, isNull); + expect(mergedCopy.localId, 'local-1'); + expect(searchCopy.heroTag, isNot(mergedCopy.heroTag)); + + expect(searchCopy.refersToSameAsset(mergedCopy), isTrue); + expect(mergedCopy.refersToSameAsset(searchCopy), isTrue); + }); + + test('different remote assets are not the same', () { + final a = RemoteAssetFactory.create(id: 'asset-1'); + final b = RemoteAssetFactory.create(id: 'asset-2'); + + expect(a.refersToSameAsset(b), isFalse); + }); + + test('same checksum but different remote ids are not the same (duplicate files)', () { + final a = RemoteAssetFactory.create(id: 'asset-1'); + final b = RemoteAssetFactory.create(id: 'asset-2').copyWith(checksum: a.checksum); + + expect(a.checksum, b.checksum); + expect(a.refersToSameAsset(b), isFalse); + }); + + test('falls back to checksum when only one side has an id (local-only vs remote-only)', () { + final remoteOnly = RemoteAssetFactory.create(id: 'asset-1'); + final localOnly = LocalAssetFactory.create(id: 'local-1').copyWith(checksum: remoteOnly.checksum); + + expect(remoteOnly.refersToSameAsset(localOnly), isTrue); + expect(localOnly.refersToSameAsset(remoteOnly), isTrue); + }); + }); +}