diff --git a/docs/superpowers/specs/2026-08-10-965-space-album-add-parity-design.md b/docs/superpowers/specs/2026-08-10-965-space-album-add-parity-design.md new file mode 100644 index 0000000000000..c4cb545d7c406 --- /dev/null +++ b/docs/superpowers/specs/2026-08-10-965-space-album-add-parity-design.md @@ -0,0 +1,236 @@ +# Adding photos to a specific Shared Space album, from every entry point (#965) + +Status: draft 2026-08-10. Closes the entry-point gap left by +`2026-07-25-space-add-to-collection-design.md` (web `CollectionPickerModal`) and +`2026-07-26-mobile-spaces-ux-design.md` (mobile `CollectionPicker`). + +## Problem + +Whether "add this photo to that Shared Space album" is possible depends on which screen the user +started from, and the two clients disagree about which screens work. + +| Entry point | Personal album | Space pool | **Album inside a space** | +| ---------------------------------------- | -------------- | ---------- | ------------------------ | +| Web — any surface (one shared picker) | yes | yes | **no** | +| Mobile — main timeline / search / person | yes | yes | yes | +| Mobile — inside a personal album | yes | **no** | **no** | +| Mobile — favorites, archive, local album | yes | **no** | **no** | +| Mobile — asset viewer `+` → Album | yes | **no** | **no** | + +Two independent causes: + +1. **Web never lists space-linked albums at all.** `CollectionPickerModal` loads personal albums + (`getAllAlbums`) plus writable spaces (`getAllSpaces`) and stops there. The only place it lists a + space's linked albums is `restrictToSpaceId` mode — reached solely from a space surface with a + non-owned selection (#764 contribution). So the target simply does not exist in the normal picker, + on any web surface. +2. **Mobile has two different pickers and only one of them knows about spaces.** The fork's + `CollectionPicker` (album selector **+** `SpaceCollectionSection`) is wired into + `general_bottom_sheet` and `space_bottom_sheet`; every other add-to-collection surface still + mounts upstream's bare `AlbumSelector`. + +Note that the web gap is uniform — every web surface is equally broken — while the mobile gap is +per-surface. That is why the issue reads as "inconsistent depending on entry point" on mobile and +"missing everywhere" on web. + +## What the server already permits — no server change + +`Permission.AlbumAssetCreate` has a `checkSpaceLinkedAlbumAccess` arm +(`server/src/utils/access.ts:195`, `access.repository.ts:147`): **every album linked to a space where +the caller is Owner or Editor** grants add-permission, even when the caller is neither album owner +nor album user. `POST /albums/:id/assets` is therefore the one and only call needed; a space-linked +album is dispatched exactly like a personal album. + +`GET /shared-spaces/:id/albums` (`SharedSpaceRead`) lists a space's linked albums, and +`GET /shared-spaces` already returns `albumCount` per space (`shared-space.service.ts:143`), which is +what lets a client know a space is expandable without fetching its albums. + +So this is a pure client change on both platforms. + +## Design + +### Guiding rule: one picker per platform, same shape on both + +The fix is not "add space albums to N places". It is "there is exactly one add-to-collection picker +per platform, and it offers albums, spaces, and space albums". Web already has one picker used +everywhere, so web needs only the missing rows. Mobile has the right picker already built — it just +is not mounted on every surface. + +### Row shape — accordion, mirroring mobile + +Mobile's `SpaceCollectionSection` is the reference: a space row with linked albums is expandable; +expanding reveals an "Add to space" child (the pool) plus one child row per linked album. At most one +space is expanded at a time. Web adopts the same shape rather than a flat "Space › Album" list, +because: + +- it keeps the picker short when a user has many spaces with many albums; +- it avoids fanning out `getSharedSpaceAlbums` for every space on modal open — the call happens once, + lazily, when a space is expanded, and is cached for the life of the modal; +- it is the interaction users already know from mobile. + +A space with `albumCount === 0` stays a plain row whose click adds to the pool — unchanged from +today, and identical to mobile. + +**Accepted wart (web):** a space can appear twice, once under `RECENT` and once under `ALL`. +Expansion is keyed by space id, so both occurrences expand together and render the same children. +The alternative — expandable only in `ALL` — would give the same visual row two different click +behaviours, which is worse. + +### Search + +Search filters **top-level rows only**, on both platforms: album names/descriptions and space +names. Children of an expanded space are not filtered. This is mobile's current behaviour and is kept +verbatim on web so the two stay in step. Making search reach into space albums requires eagerly +loading every space's albums (web: N requests; mobile: N live Drift subscriptions, which +`SpaceCollectionSection` deliberately bounds to one) and is out of scope. + +### Web changes + +`web/src/lib/components/shared-components/collection-selection/collection-selection-utils.ts` + +- `CollectionModalRow` gains `expandable?`, `expanded?`, `indented?`. +- New row type `SPACE_POOL_CHILD` — selectable, carries the space collection, rendered indented + under an expanded space row. Added to `isSelectableRowType` so keyboard nav counts it. +- `toModalRows` gains `expandedSpaceId?` and `expandedSpaceAlbums?: PickerCollection[]`. After + pushing a space row whose id matches `expandedSpaceId` it pushes: the pool child, then one indented + `COLLECTION_ITEM` per linked album, or a `MESSAGE` row (`no_albums_in_space_yet`) when the space has + none. `expandedSpaceAlbums === undefined` means the fetch is still in flight and is deliberately + distinct from `[]` — only the pool child renders, so "this space has no albums yet" never flashes + before the answer is known. +- Children are pushed inside `pushItem` so the running `index` — and therefore arrow-key order — + stays a single flat sequence over visible selectable rows. + +`web/src/lib/modals/CollectionPickerModal.svelte` + +- `expandedSpaceId = $state(null)` and + `spaceAlbumCache = $state>({})`. +- Clicking a space row: `albumCount > 0` → toggle expansion (fetching + caching + `getSharedSpaceAlbums` on first expand, `handleError` on failure and collapse); otherwise → select + the pool, as today. +- Multi-select on a space row still means the pool. Space-album children participate in multi-select + like any album row. +- Restricted mode (`restrictToSpaceId`) is untouched — it already lists exactly one space's albums + and never lists spaces. + +`web/src/lib/components/shared-components/collection-selection/space-list-item.svelte` + +- New `expandable` / `expanded` props render a chevron and set `aria-expanded`. + +Dispatch (`collection.service.ts`) needs **no change**: a space-linked album arrives as +`{ kind: 'album' }` and goes through `addAssetsToAlbums`, which is `POST /albums/:id/assets` — the +endpoint that carries the space-linked permission arm. + +### Mobile changes + +Replace the bare `AlbumSelector` with `CollectionPicker` on every add-to-collection surface: + +| File | Surface | +| ---------------------------------------------------- | ------------------------- | +| `bottom_sheet/remote_album_bottom_sheet.widget.dart` | selection inside an album | +| `bottom_sheet/favorite_bottom_sheet.widget.dart` | favorites | +| `bottom_sheet/archive_bottom_sheet.widget.dart` | archive | +| `bottom_sheet/local_album_bottom_sheet.widget.dart` | on-device album | +| `action_buttons/add_action_button.widget.dart` | asset viewer `+` → Album | + +To make that possible `CollectionPicker` gains three things: + +- **`source` (`ActionSource`, default `timeline`)** — the asset viewer dispatches against + `ActionSource.viewer`, and `_addToAlbum` / `_addToTarget` must pass it through instead of + hard-coding `timeline`. +- **`assets`** — `SpaceCollectionSection` currently reads `multiSelectProvider.selectedAssets` to + decide its notices (non-owned / locked / over-cap). In the asset viewer the multiselect is empty, + which would read as "nothing non-owned" and wrongly offer space targets for someone else's photo. + `CollectionPicker` resolves the asset set from `source` (timeline → multiselect, viewer → + `assetViewerProvider.currentAsset`) and passes it down; `SpaceCollectionSection` takes an optional + `assets` and falls back to the multiselect so its existing tests and callers are unaffected. +- **`onCompleted`** (optional) — the asset viewer needs its existing post-add behaviour preserved: + invalidate `albumsContainingAssetProvider` (the info panel's "Appears in" list) and pop the sheet. + +**Deliberately excluded: `partner_detail_bottom_sheet`** — but not for the reason first given here. + +An earlier draft of this spec claimed a partner's asset "can never reach any space target" and that +"web hides the `+` entirely there". Both were wrong, and checking the code settled it: + +- `Permission.AssetShare` is owner **∪ partner**, not owner-only — `access.ts:127-131` unions + `checkPartnerAccess`. So `POST /shared-spaces/:id/assets` accepts a partner's assets. +- Web's partner route does not consult `getSelectionCapabilities` at all; it renders + `` unconditionally + (`routes/(user)/partners/[userId]/…/+page.svelte:99`). + +The real reason to leave the sheet alone is mobile-side and pre-existing: `selectionHasNonOwned` +(`utils/selection_targets.dart`) treats any asset whose `ownerId` differs from the current user as +unreachable, so mounting the picker on a partner surface would render a Spaces section that is +always collapsed behind a notice — and that notice is itself **stricter than the server**. Relaxing +the rule needs mobile to know which owners are partners, which is a behaviour change to every +surface the rule already governs, not an entry-point fix. Tracked as follow-up; out of scope here. + +Note the same rule now reaches one new surface as a side effect of this change: viewing a partner's +photo in the asset viewer shows the notice where previously there was no Spaces section at all. That +is consistent with the timeline's existing behaviour rather than a new class of bug, but it is the +clearest remaining web/mobile divergence. + +`drift_album.page.dart` also mounts `AlbumSelector`, but as an album **browser** (tap navigates to the +album), not a picker. Out of scope. + +## Known remaining divergences + +Found by reviewing the finished change against the goal of parity. None block #965; all are +pre-existing shapes this change did not create. + +| | Web | Mobile | +| --------------------------------- | ----------------------------------------------------------- | -------------------------------------------- | +| Partner surface | offers spaces and space albums, and the server accepts them | album-only (see above) | +| Space-album page | add-to-collection available | `space_album_bottom_sheet` passes no slivers | +| Album you can edit but do not own | add-to-collection available | gated on `ownsAlbum` | +| Current space as a target | offered, so its own albums are reachable from inside it | filtered out via `excludeSpaceId` | +| Child-album source | live `GET /shared-spaces/:id/albums` | local Drift, so sync-gated | +| Child ordering | server order (`album.createdAt DESC`) | album name ascending | +| Searching a space album by name | no match — children are not searchable | same | + +## Out of scope + +**Selections over `MAX_SPACE_ASSETS_PER_REQUEST` (50 000).** Web already hides every space row above +that cap, with a notice, because `POST /shared-spaces/:id/assets` cannot take the request. Space +albums go through the album endpoint and are not capped, but they are only reachable by expanding a +space row — so above the cap they disappear along with the spaces. Adding a second, uncapped route to +them would complicate the picker for a case that needs a 50 000-asset selection to reach; the notice +already explains why the section is gone. + +**Duplicate rows for an album you own that is linked to a space.** It appears both as a personal +album at the top level and as a child of its space. Mobile has behaved this way since the spaces +section shipped, and the nesting is informative rather than wrong. + +The issue's closing note — extending the Album filter from `All / Has album / Has no album` to +"filter by a specific album" so photos can be found from a Space album surface — is a separate +feature request against the filter system, not an entry-point inconsistency. Tracked separately. + +Issue #966 (album sort options differing between web and mobile inside a Space) is a different +parity bug and is not touched here. + +## Test plan + +TDD, red first, per platform. + +**Web** — `CollectionPickerModal.spec.ts`, `collection-selection-utils` converter spec: + +1. a space with `albumCount > 0` renders as expandable and does **not** immediately fetch its albums; +2. clicking it calls `getSharedSpaceAlbums({ id })` once and renders one child row per linked album + plus the "Add to space" pool child; +3. clicking a space-album child confirms with `{ kind: 'album', id: }`; +4. clicking the pool child confirms with `{ kind: 'space', id: }`; +5. expanding a second space collapses the first, and re-expanding the first does not re-fetch; +6. a space with `albumCount === 0` is not expandable and its click still confirms with the space; +7. a failed `getSharedSpaceAlbums` calls `handleError` and leaves the row collapsed; +8. arrow-key order walks the children in visual order; +9. restricted mode is unchanged (existing suite must stay green). + +**Mobile** — `collection_picker_test.dart`, `space_collection_section_test.dart`, plus one test per +newly-wired surface: + +1. `CollectionPicker` dispatches against the `source` it was given (viewer vs timeline); +2. `CollectionPicker` passes viewer assets to `SpaceCollectionSection`, so a non-owned asset in the + viewer shows the notice and offers no space target; +3. `SpaceCollectionSection` with an explicit `assets` argument ignores the multiselect; +4. `onCompleted` fires after a successful add and not after a failure; +5. each rewired surface renders the collection-picker header (`collection-picker-header`) rather than + a bare `AlbumSelector`. diff --git a/mobile/lib/presentation/widgets/action_buttons/add_action_button.widget.dart b/mobile/lib/presentation/widgets/action_buttons/add_action_button.widget.dart index dc48ed57ecb17..c071a68c14038 100644 --- a/mobile/lib/presentation/widgets/action_buttons/add_action_button.widget.dart +++ b/mobile/lib/presentation/widgets/action_buttons/add_action_button.widget.dart @@ -1,19 +1,19 @@ +import 'dart:async'; + import 'package:easy_localization/easy_localization.dart'; import 'package:flutter/material.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/extensions/build_context_extensions.dart'; -import 'package:immich_mobile/extensions/translate_extensions.dart'; import 'package:immich_mobile/presentation/widgets/action_buttons/base_action_button.widget.dart'; import 'package:immich_mobile/presentation/widgets/action_buttons/unarchive_action_button.widget.dart'; -import 'package:immich_mobile/providers/asset_viewer/asset_viewer.provider.dart'; import 'package:immich_mobile/presentation/widgets/album/album_selector.widget.dart'; -import 'package:immich_mobile/providers/infrastructure/action.provider.dart'; +import 'package:immich_mobile/presentation/widgets/collection/collection_picker.widget.dart'; +import 'package:immich_mobile/providers/asset_viewer/asset_viewer.provider.dart'; import 'package:immich_mobile/providers/infrastructure/album.provider.dart'; import 'package:immich_mobile/providers/routes.provider.dart'; import 'package:immich_mobile/widgets/common/immich_toast.dart'; import 'package:immich_mobile/providers/user.provider.dart'; -import 'package:immich_mobile/domain/models/album/album.model.dart'; import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; import 'package:immich_mobile/constants/enums.dart'; @@ -113,9 +113,16 @@ class _AddActionButtonState extends ConsumerState { return; } + // #965: the same picker every other surface offers. The viewer has no multiselect, so it + // states its source and its one asset explicitly — the spaces section judges ownership + // from that rather than from an empty selection. final List slivers = [ const CreateAlbumButton(), - AlbumSelector(onAlbumSelected: (album) => _addCurrentAssetToAlbum(album)), + CollectionPicker( + source: ActionSource.viewer, + assets: [currentAsset], + onCompleted: () => _onAddCompleted(currentAsset), + ), ]; showModalBottomSheet( @@ -136,51 +143,19 @@ class _AddActionButtonState extends ConsumerState { ); } - Future _addCurrentAssetToAlbum(RemoteAlbum album) async { - final latest = ref.read(assetViewerProvider).currentAsset; - - if (latest == null) { - ImmichToast.show(context: context, msg: "Cannot load asset information.", toastType: ToastType.error); - return; - } - - final result = await ref.read(actionProvider.notifier).addToAlbum(ActionSource.viewer, album); - + /// The picker owns the dispatch and the toasts; the viewer only has to refresh what it + /// shows and get out of the way. + void _onAddCompleted(BaseAsset asset) { + // Guard before touching `ref`: invalidating from a disposed ConsumerState throws. if (!context.mounted) { return; } - - if (!result.success) { - ImmichToast.show(context: context, msg: 'scaffold_body_error_occurred'.tr(), toastType: ToastType.error); - return; - } - - // Only report the failure when nothing was added; if some succeeded we show "added". - if (result.count > 0) { - ImmichToast.show( - context: context, - msg: 'add_to_album_bottom_sheet_added'.tr(namedArgs: {'album': album.name}), - ); - + final remoteId = asset.remoteId; + if (remoteId != null) { // Refresh the "Appears in" list on the asset's info panel. - ref.invalidate(albumsContainingAssetProvider(latest.remoteId!)); - } else if (result.failedCount > 0) { - ImmichToast.show( - context: context, - msg: 'assets_cannot_be_added_to_album_count'.t(context: context, args: {'count': result.failedCount}), - toastType: ToastType.error, - ); - } else { - ImmichToast.show( - context: context, - msg: 'add_to_album_bottom_sheet_already_exists'.tr(namedArgs: {'album': album.name}), - ); - } - - if (!context.mounted) { - return; + ref.invalidate(albumsContainingAssetProvider(remoteId)); } - await Navigator.of(context).maybePop(); + unawaited(Navigator.of(context).maybePop()); } @override diff --git a/mobile/lib/presentation/widgets/bottom_sheet/archive_bottom_sheet.widget.dart b/mobile/lib/presentation/widgets/bottom_sheet/archive_bottom_sheet.widget.dart index 3c9c0c692e424..af2e049061e90 100644 --- a/mobile/lib/presentation/widgets/bottom_sheet/archive_bottom_sheet.widget.dart +++ b/mobile/lib/presentation/widgets/bottom_sheet/archive_bottom_sheet.widget.dart @@ -1,8 +1,6 @@ -import 'package:easy_localization/easy_localization.dart'; import 'package:flutter/material.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/constants/enums.dart'; -import 'package:immich_mobile/domain/models/album/album.model.dart'; import 'package:immich_mobile/presentation/actions/action.widget.dart'; import 'package:immich_mobile/presentation/actions/favorite.action.dart'; import 'package:immich_mobile/presentation/actions/timeline.action.dart'; @@ -18,12 +16,10 @@ import 'package:immich_mobile/presentation/widgets/action_buttons/stack_action_b import 'package:immich_mobile/presentation/widgets/action_buttons/trash_action_button.widget.dart'; import 'package:immich_mobile/presentation/widgets/action_buttons/unarchive_action_button.widget.dart'; import 'package:immich_mobile/presentation/widgets/action_buttons/unstack_action_button.widget.dart'; -import 'package:immich_mobile/presentation/widgets/album/album_selector.widget.dart'; import 'package:immich_mobile/presentation/widgets/bottom_sheet/base_bottom_sheet.widget.dart'; -import 'package:immich_mobile/providers/infrastructure/action.provider.dart'; +import 'package:immich_mobile/presentation/widgets/collection/collection_picker.widget.dart'; import 'package:immich_mobile/providers/server_info.provider.dart'; import 'package:immich_mobile/providers/timeline/multiselect.provider.dart'; -import 'package:immich_mobile/widgets/common/immich_toast.dart'; class ArchiveBottomSheet extends ConsumerStatefulWidget { const ArchiveBottomSheet({super.key}); @@ -52,26 +48,6 @@ class _ArchiveBottomSheetState extends ConsumerState { final multiselect = ref.watch(multiSelectProvider); final isTrashEnable = ref.watch(serverInfoProvider.select((state) => state.serverFeatures.trash)); - Future addToAlbum(RemoteAlbum album) async { - final result = await ref.read(actionProvider.notifier).addToAlbum(ActionSource.timeline, album); - - if (!context.mounted) { - return; - } - - if (!result.success) { - ImmichToast.show(context: context, msg: 'scaffold_body_error_occurred'.tr(), toastType: ToastType.error); - return; - } - - ImmichToast.show( - context: context, - msg: result.count == 0 - ? 'add_to_album_bottom_sheet_already_exists'.tr(namedArgs: {'album': album.name}) - : 'add_to_album_bottom_sheet_added'.tr(namedArgs: {'album': album.name}), - ); - } - Future onKeyboardExpand() { return sheetController.animateTo(0.85, duration: const Duration(milliseconds: 200), curve: Curves.easeInOut); } @@ -102,10 +78,9 @@ class _ArchiveBottomSheetState extends ConsumerState { ], if (multiselect.hasMerged) const DeleteLocalActionButton(source: ActionSource.timeline), ], - slivers: [ - const AddToAlbumHeader(), - AlbumSelector(onAlbumSelected: addToAlbum, onKeyboardExpanded: onKeyboardExpand), - ], + // #965: the same picker the main timeline offers, so a space album is reachable from + // the archive too. + slivers: [CollectionPicker(onKeyboardExpanded: onKeyboardExpand)], ); } } diff --git a/mobile/lib/presentation/widgets/bottom_sheet/favorite_bottom_sheet.widget.dart b/mobile/lib/presentation/widgets/bottom_sheet/favorite_bottom_sheet.widget.dart index bcb9fc6fe32bf..4c90208f5947f 100644 --- a/mobile/lib/presentation/widgets/bottom_sheet/favorite_bottom_sheet.widget.dart +++ b/mobile/lib/presentation/widgets/bottom_sheet/favorite_bottom_sheet.widget.dart @@ -1,9 +1,6 @@ import 'package:flutter/material.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/constants/enums.dart'; -import 'package:immich_mobile/domain/models/album/album.model.dart'; -import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; -import 'package:immich_mobile/extensions/translate_extensions.dart'; import 'package:immich_mobile/presentation/actions/action.widget.dart'; import 'package:immich_mobile/presentation/actions/favorite.action.dart'; import 'package:immich_mobile/presentation/actions/timeline.action.dart'; @@ -19,12 +16,10 @@ import 'package:immich_mobile/presentation/widgets/action_buttons/share_link_act import 'package:immich_mobile/presentation/widgets/action_buttons/stack_action_button.widget.dart'; import 'package:immich_mobile/presentation/widgets/action_buttons/trash_action_button.widget.dart'; import 'package:immich_mobile/presentation/widgets/action_buttons/unstack_action_button.widget.dart'; -import 'package:immich_mobile/presentation/widgets/album/album_selector.widget.dart'; import 'package:immich_mobile/presentation/widgets/bottom_sheet/base_bottom_sheet.widget.dart'; -import 'package:immich_mobile/providers/infrastructure/album.provider.dart'; +import 'package:immich_mobile/presentation/widgets/collection/collection_picker.widget.dart'; import 'package:immich_mobile/providers/server_info.provider.dart'; import 'package:immich_mobile/providers/timeline/multiselect.provider.dart'; -import 'package:immich_mobile/widgets/common/immich_toast.dart'; class FavoriteBottomSheet extends ConsumerWidget { const FavoriteBottomSheet({super.key}); @@ -34,46 +29,6 @@ class FavoriteBottomSheet extends ConsumerWidget { final multiselect = ref.watch(multiSelectProvider); final isTrashEnable = ref.watch(serverInfoProvider.select((state) => state.serverFeatures.trash)); - Future addAssetsToAlbum(RemoteAlbum album) async { - final selectedAssets = multiselect.selectedAssets; - if (selectedAssets.isEmpty) { - return; - } - - final remoteAssets = selectedAssets.whereType(); - final result = await ref - .read(remoteAlbumProvider.notifier) - .addAssets(album.id, remoteAssets.map((e) => e.id).toList()); - - if (selectedAssets.length != remoteAssets.length) { - ImmichToast.show( - context: context, - msg: 'add_to_album_bottom_sheet_some_local_assets'.t(context: context), - ); - } - - // Only report the failure when nothing was added; if some succeeded we show "added". - if (result.added > 0) { - ImmichToast.show( - context: context, - msg: 'add_to_album_bottom_sheet_added'.t(args: {"album": album.name}), - ); - } else if (result.failed > 0) { - ImmichToast.show( - context: context, - msg: 'assets_cannot_be_added_to_album_count'.t(context: context, args: {'count': result.failed}), - toastType: ToastType.error, - ); - } else { - ImmichToast.show( - context: context, - msg: 'add_to_album_bottom_sheet_already_exists'.t(args: {"album": album.name}), - ); - } - - ref.read(multiSelectProvider.notifier).reset(); - } - final assets = multiselect.selectedAssets.toList(growable: false); final actions = [FavoriteAction(assets: assets)]; @@ -99,9 +54,9 @@ class FavoriteBottomSheet extends ConsumerWidget { ], if (multiselect.hasMerged) const DeleteLocalActionButton(source: ActionSource.timeline), ], - slivers: multiselect.hasRemote - ? [const AddToAlbumHeader(), AlbumSelector(onAlbumSelected: addAssetsToAlbum)] - : [], + // #965: the same picker the main timeline offers, so a space album is reachable from + // favorites too. + slivers: multiselect.hasRemote ? [const CollectionPicker()] : [], ); } } diff --git a/mobile/lib/presentation/widgets/bottom_sheet/local_album_bottom_sheet.widget.dart b/mobile/lib/presentation/widgets/bottom_sheet/local_album_bottom_sheet.widget.dart index ac8c77af03d8d..9671fbfdbe8ff 100644 --- a/mobile/lib/presentation/widgets/bottom_sheet/local_album_bottom_sheet.widget.dart +++ b/mobile/lib/presentation/widgets/bottom_sheet/local_album_bottom_sheet.widget.dart @@ -1,15 +1,11 @@ -import 'package:easy_localization/easy_localization.dart'; import 'package:flutter/material.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/constants/enums.dart'; -import 'package:immich_mobile/domain/models/album/album.model.dart'; import 'package:immich_mobile/presentation/widgets/action_buttons/delete_local_action_button.widget.dart'; import 'package:immich_mobile/presentation/widgets/action_buttons/share_action_button.widget.dart'; import 'package:immich_mobile/presentation/widgets/action_buttons/upload_action_button.widget.dart'; -import 'package:immich_mobile/presentation/widgets/album/album_selector.widget.dart'; import 'package:immich_mobile/presentation/widgets/bottom_sheet/base_bottom_sheet.widget.dart'; -import 'package:immich_mobile/providers/infrastructure/action.provider.dart'; -import 'package:immich_mobile/widgets/common/immich_toast.dart'; +import 'package:immich_mobile/presentation/widgets/collection/collection_picker.widget.dart'; class LocalAlbumBottomSheet extends ConsumerStatefulWidget { const LocalAlbumBottomSheet({super.key}); @@ -35,26 +31,6 @@ class _LocalAlbumBottomSheetState extends ConsumerState { @override Widget build(BuildContext context) { - Future addToAlbum(RemoteAlbum album) async { - final result = await ref.read(actionProvider.notifier).addToAlbum(ActionSource.timeline, album); - - if (!context.mounted) { - return; - } - - if (!result.success) { - ImmichToast.show(context: context, msg: 'scaffold_body_error_occurred'.tr(), toastType: ToastType.error); - return; - } - - ImmichToast.show( - context: context, - msg: result.count == 0 - ? 'add_to_album_bottom_sheet_already_exists'.tr(namedArgs: {'album': album.name}) - : 'add_to_album_bottom_sheet_added'.tr(namedArgs: {'album': album.name}), - ); - } - Future onKeyboardExpand() { return sheetController.animateTo(0.85, duration: const Duration(milliseconds: 200), curve: Curves.easeInOut); } @@ -69,10 +45,9 @@ class _LocalAlbumBottomSheetState extends ConsumerState { DeleteLocalActionButton(source: ActionSource.timeline), UploadActionButton(source: ActionSource.timeline), ], - slivers: [ - const AddToAlbumHeader(), - AlbumSelector(onAlbumSelected: addToAlbum, onKeyboardExpanded: onKeyboardExpand), - ], + // #965: the same picker the main timeline offers. A selection here is local-only, and + // the space paths upload before they add, so a space album is a valid destination. + slivers: [CollectionPicker(onKeyboardExpanded: onKeyboardExpand)], ); } } diff --git a/mobile/lib/presentation/widgets/bottom_sheet/remote_album_bottom_sheet.widget.dart b/mobile/lib/presentation/widgets/bottom_sheet/remote_album_bottom_sheet.widget.dart index a292c1899c749..d0002629e9bfc 100644 --- a/mobile/lib/presentation/widgets/bottom_sheet/remote_album_bottom_sheet.widget.dart +++ b/mobile/lib/presentation/widgets/bottom_sheet/remote_album_bottom_sheet.widget.dart @@ -2,7 +2,6 @@ import 'package:flutter/material.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/constants/enums.dart'; import 'package:immich_mobile/domain/models/album/album.model.dart'; -import 'package:immich_mobile/extensions/translate_extensions.dart'; import 'package:immich_mobile/presentation/actions/action.widget.dart'; import 'package:immich_mobile/presentation/actions/favorite.action.dart'; import 'package:immich_mobile/presentation/actions/timeline.action.dart'; @@ -20,13 +19,11 @@ import 'package:immich_mobile/presentation/widgets/action_buttons/share_link_act import 'package:immich_mobile/presentation/widgets/action_buttons/stack_action_button.widget.dart'; import 'package:immich_mobile/presentation/widgets/action_buttons/trash_action_button.widget.dart'; import 'package:immich_mobile/presentation/widgets/action_buttons/unstack_action_button.widget.dart'; -import 'package:immich_mobile/presentation/widgets/album/album_selector.widget.dart'; import 'package:immich_mobile/presentation/widgets/bottom_sheet/base_bottom_sheet.widget.dart'; -import 'package:immich_mobile/providers/infrastructure/action.provider.dart'; +import 'package:immich_mobile/presentation/widgets/collection/collection_picker.widget.dart'; import 'package:immich_mobile/providers/server_info.provider.dart'; import 'package:immich_mobile/providers/timeline/multiselect.provider.dart'; import 'package:immich_mobile/providers/user.provider.dart'; -import 'package:immich_mobile/widgets/common/immich_toast.dart'; class RemoteAlbumBottomSheet extends ConsumerStatefulWidget { final RemoteAlbum album; @@ -57,30 +54,6 @@ class _RemoteAlbumBottomSheetState extends ConsumerState final isTrashEnable = ref.watch(serverInfoProvider.select((state) => state.serverFeatures.trash)); final ownsAlbum = ref.watch(currentUserProvider)?.id == widget.album.ownerId; - Future addToAlbum(RemoteAlbum album) async { - final result = await ref.read(actionProvider.notifier).addToAlbum(ActionSource.timeline, album); - - if (!context.mounted) { - return; - } - - if (!result.success) { - ImmichToast.show( - context: context, - msg: 'scaffold_body_error_occurred'.t(context: context), - toastType: ToastType.error, - ); - return; - } - - ImmichToast.show( - context: context, - msg: result.count == 0 - ? 'add_to_album_bottom_sheet_already_exists'.t(context: context, args: {"album": album.name}) - : 'add_to_album_bottom_sheet_added'.t(context: context, args: {"album": album.name}), - ); - } - Future onKeyboardExpand() { return sheetController.animateTo(0.85, duration: const Duration(milliseconds: 200), curve: Curves.easeInOut); } @@ -120,9 +93,9 @@ class _RemoteAlbumBottomSheetState extends ConsumerState if (ownsAlbum && multiselect.selectedAssets.length == 1) SetAlbumCoverActionButton(source: ActionSource.timeline, albumId: widget.album.id), ], - slivers: ownsAlbum - ? [const AddToAlbumHeader(), AlbumSelector(onAlbumSelected: addToAlbum, onKeyboardExpanded: onKeyboardExpand)] - : null, + // #965: the same picker the main timeline offers, so a space album is reachable from + // inside an album too — not only from the timeline. + slivers: ownsAlbum ? [CollectionPicker(onKeyboardExpanded: onKeyboardExpand)] : null, ); } } diff --git a/mobile/lib/presentation/widgets/collection/collection_picker.widget.dart b/mobile/lib/presentation/widgets/collection/collection_picker.widget.dart index 7a0882f94cbf6..0de144018b694 100644 --- a/mobile/lib/presentation/widgets/collection/collection_picker.widget.dart +++ b/mobile/lib/presentation/widgets/collection/collection_picker.widget.dart @@ -2,6 +2,7 @@ import 'package:flutter/material.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/constants/enums.dart'; import 'package:immich_mobile/domain/models/album/album.model.dart'; +import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; import 'package:immich_mobile/domain/models/collection_target.dart'; import 'package:immich_mobile/extensions/build_context_extensions.dart'; import 'package:immich_mobile/extensions/translate_extensions.dart'; @@ -18,7 +19,14 @@ import 'package:sliver_tools/sliver_tools.dart'; /// `AddToAlbumHeader` is not reused because it hardcodes the `add_to_album` key; this /// picker supplies its own header so the sheet can honestly say "album or space". class CollectionPicker extends ConsumerStatefulWidget { - const CollectionPicker({super.key, this.excludeSpaceId, this.onKeyboardExpanded}); + const CollectionPicker({ + super.key, + this.excludeSpaceId, + this.onKeyboardExpanded, + this.source = ActionSource.timeline, + this.assets, + this.onCompleted, + }); /// Set on a space's own surface so that space is not offered as a destination for /// its own assets. @@ -26,6 +34,18 @@ class CollectionPicker extends ConsumerStatefulWidget { final Function? onKeyboardExpanded; + /// Where the assets to file come from. The asset viewer dispatches against + /// [ActionSource.viewer]; every multi-select surface uses the default. + final ActionSource source; + + /// The assets being filed, for the spaces section's ownership / cap notices. Omit on a + /// multi-select surface — the section then reads the timeline selection itself. + final Iterable? assets; + + /// Called after an add that succeeded. Surfaces that dismiss themselves (the asset viewer + /// sheet) hook this; a failed add deliberately does not fire it, so the sheet stays open. + final VoidCallback? onCompleted; + @override ConsumerState createState() => _CollectionPickerState(); } @@ -37,7 +57,7 @@ class _CollectionPickerState extends ConsumerState { Future _addToAlbum(RemoteAlbum album) async { if (_isBusy) return; setState(() => _isBusy = true); - final result = await ref.read(actionProvider.notifier).addToAlbum(ActionSource.timeline, album); + final result = await ref.read(actionProvider.notifier).addToAlbum(widget.source, album); if (!mounted) return; setState(() => _isBusy = false); @@ -45,12 +65,22 @@ class _CollectionPickerState extends ConsumerState { _toastError(); return; } + if (result.count == 0 && result.failedCount > 0) { + // Nothing landed and the server said why — "already in this album" would be a lie. + ImmichToast.show( + context: context, + msg: 'assets_cannot_be_added_to_album_count'.t(context: context, args: {'count': result.failedCount}), + toastType: ToastType.error, + ); + return; + } ImmichToast.show( context: context, msg: result.count == 0 ? 'add_to_album_bottom_sheet_already_exists'.t(context: context, args: {'album': album.name}) : 'add_to_album_bottom_sheet_added'.t(context: context, args: {'album': album.name}), ); + widget.onCompleted?.call(); } Future _addToTarget(CollectionTarget target) async { @@ -62,14 +92,14 @@ class _CollectionPickerState extends ConsumerState { final String? successMessage; switch (target) { case AlbumTarget(:final album): - result = await notifier.addToAlbum(ActionSource.timeline, album); + result = await notifier.addToAlbum(widget.source, album); successMessage = null; case SpacePoolTarget(:final space): - result = await notifier.addToSpace(ActionSource.timeline, space); + result = await notifier.addToSpace(widget.source, space); // The pool endpoint is 204 with no body, so this count is the request length. successMessage = 'added_to_space_count'; case SpaceAlbumTarget(:final spaceId, :final album): - result = await notifier.addToSpaceAlbum(ActionSource.timeline, spaceId, album); + result = await notifier.addToSpaceAlbum(widget.source, spaceId, album); // This one IS the server's count, so duplicates are already excluded. successMessage = 'space_album_add_photos_success'; } @@ -88,6 +118,7 @@ class _CollectionPickerState extends ConsumerState { toastType: ToastType.success, ); } + widget.onCompleted?.call(); } void _toastError() { @@ -124,6 +155,7 @@ class _CollectionPickerState extends ConsumerState { excludeSpaceId: widget.excludeSpaceId, isBusy: _isBusy, searchQuery: _searchQuery, + assets: widget.assets, ), ), ], diff --git a/mobile/lib/presentation/widgets/collection/space_collection_section.widget.dart b/mobile/lib/presentation/widgets/collection/space_collection_section.widget.dart index a14bca4553522..0e2fdf8f7e661 100644 --- a/mobile/lib/presentation/widgets/collection/space_collection_section.widget.dart +++ b/mobile/lib/presentation/widgets/collection/space_collection_section.widget.dart @@ -1,8 +1,8 @@ import 'package:flutter/material.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:immich_mobile/constants/collection.dart'; +import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; import 'package:immich_mobile/domain/models/collection_target.dart'; -import 'package:immich_mobile/domain/models/space_album.model.dart'; import 'package:immich_mobile/extensions/build_context_extensions.dart'; import 'package:immich_mobile/extensions/translate_extensions.dart'; import 'package:immich_mobile/providers/infrastructure/space_album.provider.dart'; @@ -27,6 +27,7 @@ class SpaceCollectionSection extends ConsumerStatefulWidget { this.excludeSpaceId, this.isBusy = false, this.searchQuery = '', + this.assets, }); final void Function(CollectionTarget target) onTargetSelected; @@ -34,6 +35,13 @@ class SpaceCollectionSection extends ConsumerStatefulWidget { /// Set on a space's own surface so it is not offered as a destination for its own assets. final String? excludeSpaceId; + /// The assets the picker is about to file, for the ownership / cap notices. + /// + /// Defaults to the timeline multiselect. The asset viewer has no multiselect, so it passes + /// its one asset — falling back to the empty selection there would read as "nothing + /// non-owned" and offer space targets for a photo that can never reach one. + final Iterable? assets; + /// Disables every row while an add is in flight. final bool isBusy; @@ -73,7 +81,8 @@ class _SpaceCollectionSectionState extends ConsumerState Widget build(BuildContext context) { final spacesAsync = ref.watch(sharedSpacesProvider); final userId = ref.watch(currentUserProvider.select((user) => user?.id)); - final selection = ref.watch(multiSelectProvider.select((state) => state.selectedAssets)); + final multiSelection = ref.watch(multiSelectProvider.select((state) => state.selectedAssets)); + final selection = widget.assets ?? multiSelection; final spaces = spacesAsync.valueOrNull; // Offline or still loading: the album half of the picker still works, so stay out of @@ -165,7 +174,10 @@ class _SpaceCollectionSectionState extends ConsumerState List _childrenFor(SharedSpaceResponseDto space) { final albumsAsync = ref.watch(spaceAlbumsProvider(space.id)); - final albums = albumsAsync.valueOrNull ?? const []; + // `null` means the watch has not produced a value yet, which is NOT the same as "this space + // has no albums" — conflating them flashed "no albums yet" on every expand. Web draws the + // same distinction (`expandedSpaceAlbums === undefined`). + final albums = albumsAsync.valueOrNull; return [ ListTile( @@ -176,7 +188,9 @@ class _SpaceCollectionSectionState extends ConsumerState enabled: !widget.isBusy, onTap: widget.isBusy ? null : () => _emit(SpacePoolTarget(space)), ), - if (albums.isEmpty) + if (albums == null) + const SizedBox.shrink() // still loading — say nothing rather than something wrong + else if (albums.isEmpty) Padding( key: Key('space-albums-empty-${space.id}'), padding: const EdgeInsets.only(left: 48, right: 16, bottom: 8), diff --git a/mobile/test/presentation/widgets/bottom_sheet/add_to_collection_surfaces_test.dart b/mobile/test/presentation/widgets/bottom_sheet/add_to_collection_surfaces_test.dart new file mode 100644 index 0000000000000..ebe4b98adba04 --- /dev/null +++ b/mobile/test/presentation/widgets/bottom_sheet/add_to_collection_surfaces_test.dart @@ -0,0 +1,188 @@ +// #965: "add to a Shared Space album" used to depend on which screen you started from — +// only the surfaces mounting the fork's `CollectionPicker` offered spaces at all, and the +// rest mounted upstream's bare `AlbumSelector`. These tests pin the wiring: every +// add-to-collection surface mounts the one picker. +// +// The picker's own behaviour (which spaces, which albums, which dispatch) is covered by +// `collection/collection_picker_test.dart` and `collection/space_collection_section_test.dart`; +// what can silently regress here is a surface being left behind on the album-only selector. +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:immich_mobile/domain/models/album/album.model.dart'; +import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; +import 'package:immich_mobile/domain/models/config/app_config.dart'; +import 'package:immich_mobile/domain/models/user.model.dart'; +import 'package:immich_mobile/domain/services/user.service.dart'; +import 'package:immich_mobile/constants/enums.dart'; +import 'package:immich_mobile/models/albums/album_search.model.dart'; +import 'package:immich_mobile/presentation/widgets/action_buttons/add_action_button.widget.dart'; +import 'package:immich_mobile/presentation/widgets/action_buttons/base_action_button.widget.dart'; +import 'package:immich_mobile/presentation/widgets/bottom_sheet/archive_bottom_sheet.widget.dart'; +import 'package:immich_mobile/presentation/widgets/bottom_sheet/base_bottom_sheet.widget.dart'; +import 'package:immich_mobile/presentation/widgets/bottom_sheet/favorite_bottom_sheet.widget.dart'; +import 'package:immich_mobile/presentation/widgets/bottom_sheet/local_album_bottom_sheet.widget.dart'; +import 'package:immich_mobile/presentation/widgets/bottom_sheet/remote_album_bottom_sheet.widget.dart'; +import 'package:immich_mobile/presentation/widgets/collection/collection_picker.widget.dart'; +import 'package:immich_mobile/providers/asset_viewer/asset_viewer.provider.dart'; +import 'package:immich_mobile/providers/infrastructure/album.provider.dart'; +import 'package:immich_mobile/providers/infrastructure/remote_album.provider.dart'; +import 'package:immich_mobile/providers/infrastructure/settings.provider.dart'; +import 'package:immich_mobile/providers/routes.provider.dart'; +import 'package:immich_mobile/providers/server_info.provider.dart'; +import 'package:immich_mobile/services/server_info.service.dart'; +import 'package:immich_mobile/providers/shared_space.provider.dart'; +import 'package:immich_mobile/providers/timeline/multiselect.provider.dart'; +import 'package:immich_mobile/providers/user.provider.dart'; +import 'package:mocktail/mocktail.dart'; + +import '../../../fixtures/user.stub.dart'; +import '../../../unit/factories/remote_album_factory.dart'; +import '../../../widget_tester_extensions.dart'; + +class _MockUserService extends Mock implements UserService {} + +class _StubCurrentUserNotifier extends CurrentUserProvider { + _StubCurrentUserNotifier(super.service, UserDto? user) { + state = user; + } +} + +/// `AlbumSelector` fires a post-frame `refresh()` against a live `RemoteAlbumService` that +/// this harness has no reason to stand up; the picker composes it, so stub both. +class _StubRemoteAlbumNotifier extends RemoteAlbumNotifier { + @override + RemoteAlbumState build() => const RemoteAlbumState(albums: []); + + @override + Future refresh() async {} + + @override + List searchAlbums( + List albums, + String query, + String? userId, [ + QuickFilterMode filterMode = QuickFilterMode.all, + ]) => albums; +} + +class _MockServerInfoService extends Mock implements ServerInfoService {} + +class _StubAssetViewerNotifier extends AssetViewerStateNotifier { + _StubAssetViewerNotifier(this.asset); + + final BaseAsset asset; + + @override + AssetViewerState build() => AssetViewerState(currentAsset: asset); +} + +void main() { + final user = UserStub.user1; + + RemoteAsset asset(String id) => RemoteAsset( + id: id, + name: id, + ownerId: user.id, + checksum: id, + type: AssetType.image, + createdAt: DateTime(2026, 1, 1), + updatedAt: DateTime(2026, 1, 1), + isEdited: false, + ); + + RemoteAlbum ownedAlbum() => RemoteAlbumFactory.create(ownerId: user.id, ownerName: user.name, assetCount: 1); + + Future pumpSheet(WidgetTester tester, Widget sheet) async { + final userService = _MockUserService(); + when(() => userService.tryGetMyUser()).thenReturn(user); + when(() => userService.watchMyUser()).thenAnswer((_) => const Stream.empty()); + + // The localized helper, not the raw one: these sheets are mostly action buttons, and + // every one of them resolves an i18n key at build time. + await tester.pumpConsumerWidget( + sheet, + overrides: [ + currentUserProvider.overrideWith((ref) => _StubCurrentUserNotifier(userService, user)), + remoteAlbumProvider.overrideWith(() => _StubRemoteAlbumNotifier()), + appConfigProvider.overrideWithValue(const AppConfig()), + sharedSpacesProvider.overrideWith((ref) async => const []), + serverInfoProvider.overrideWith((ref) => ServerInfoNotifier(_MockServerInfoService())), + multiSelectProvider.overrideWith( + () => MultiSelectNotifier(MultiSelectState(selectedAssets: {asset('a')}, lockedSelectionAssets: const {})), + ), + ], + ); + await tester.pump(); + } + + /// Assert on the sliver list the sheet was handed, not on the rendered tree. + /// + /// These sheets open at 0.22–0.4 of the screen. Measured: `find.byType(CollectionPicker)` + /// finds it in the favorites sheet (0.4) but reports nothing for the remote-album (0.22) and + /// archive (0.25) sheets, because the picker sits below the viewport and its sliver is never + /// built. That makes a rendered-tree assertion pass or fail on sheet height rather than on + /// wiring. What the picker renders once built is covered by the picker's own tests. + void expectPickerMounted(WidgetTester tester) { + final slivers = tester.widget(find.byType(BaseBottomSheet)).slivers ?? const []; + // Reverting a surface to `[AddToAlbumHeader(), AlbumSelector(...)]` — upstream's album-only + // picker, which is the bug — empties this. + expect(slivers.whereType(), hasLength(1)); + } + + testWidgets('a selection inside an owned album offers spaces', (tester) async { + await pumpSheet(tester, RemoteAlbumBottomSheet(album: ownedAlbum())); + expectPickerMounted(tester); + }); + + testWidgets('a selection in favorites offers spaces', (tester) async { + await pumpSheet(tester, const FavoriteBottomSheet()); + expectPickerMounted(tester); + }); + + testWidgets('a selection in the archive offers spaces', (tester) async { + await pumpSheet(tester, const ArchiveBottomSheet()); + expectPickerMounted(tester); + }); + + testWidgets('a selection in an on-device album offers spaces', (tester) async { + await pumpSheet(tester, const LocalAlbumBottomSheet()); + expectPickerMounted(tester); + }); + + testWidgets('the asset viewer + button offers spaces, judged against the viewed asset', (tester) async { + final viewed = asset('viewed'); + final userService = _MockUserService(); + when(() => userService.tryGetMyUser()).thenReturn(user); + when(() => userService.watchMyUser()).thenAnswer((_) => const Stream.empty()); + + await tester.pumpConsumerWidget( + const AddActionButton(), + overrides: [ + currentUserProvider.overrideWith((ref) => _StubCurrentUserNotifier(userService, user)), + assetViewerProvider.overrideWith(() => _StubAssetViewerNotifier(viewed)), + // Reads the auto_route stack, which this harness has none of. + inLockedViewProvider.overrideWithValue(false), + remoteAlbumProvider.overrideWith(() => _StubRemoteAlbumNotifier()), + appConfigProvider.overrideWithValue(const AppConfig()), + sharedSpacesProvider.overrideWith((ref) async => const []), + multiSelectProvider.overrideWith( + () => MultiSelectNotifier(const MultiSelectState(selectedAssets: {}, lockedSelectionAssets: {})), + ), + ], + ); + + await tester.tap(find.byType(BaseActionButton).first); + await tester.pumpAndSettle(); + await tester.tap(find.widgetWithText(BaseActionButton, 'Album')); + await tester.pumpAndSettle(); + + final picker = tester + .widget(find.byType(BaseBottomSheet)) + .slivers! + .whereType() + .single; + expect(picker.source, ActionSource.viewer); + // The viewer has no multiselect, so it must state the asset the notices reason about. + expect(picker.assets, [viewed]); + }); +} diff --git a/mobile/test/presentation/widgets/collection/collection_picker_test.dart b/mobile/test/presentation/widgets/collection/collection_picker_test.dart index 0f91ffa833bef..707fcbe9d9025 100644 --- a/mobile/test/presentation/widgets/collection/collection_picker_test.dart +++ b/mobile/test/presentation/widgets/collection/collection_picker_test.dart @@ -1,16 +1,22 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:hooks_riverpod/hooks_riverpod.dart'; +import 'package:immich_mobile/constants/enums.dart'; import 'package:immich_mobile/domain/models/album/album.model.dart'; +import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; import 'package:immich_mobile/domain/models/config/app_config.dart'; +import 'package:immich_mobile/domain/models/space_album.model.dart'; import 'package:immich_mobile/domain/models/user.model.dart'; import 'package:immich_mobile/domain/services/user.service.dart'; import 'package:immich_mobile/models/albums/album_search.model.dart'; import 'package:immich_mobile/presentation/widgets/collection/collection_picker.widget.dart'; import 'package:immich_mobile/presentation/widgets/collection/space_collection_section.widget.dart'; import 'package:immich_mobile/presentation/widgets/album/album_selector.widget.dart'; +import 'package:immich_mobile/providers/infrastructure/action.provider.dart'; import 'package:immich_mobile/providers/infrastructure/album.provider.dart'; import 'package:immich_mobile/providers/infrastructure/remote_album.provider.dart'; import 'package:immich_mobile/providers/infrastructure/settings.provider.dart'; +import 'package:immich_mobile/providers/infrastructure/space_album.provider.dart'; import 'package:immich_mobile/providers/shared_space.provider.dart'; import 'package:immich_mobile/providers/timeline/multiselect.provider.dart'; import 'package:immich_mobile/providers/user.provider.dart'; @@ -54,6 +60,38 @@ class _StubRemoteAlbumNotifier extends RemoteAlbumNotifier { ]) => albums; } +/// Captures which [ActionSource] the picker dispatched against, and lets a test make the +/// dispatch fail, without standing up the real action plumbing. +class _RecordingActionNotifier extends ActionNotifier { + _RecordingActionNotifier({this.succeeds = true}); + + final bool succeeds; + final List albumSources = []; + final List spaceSources = []; + final List<(ActionSource, String, String)> spaceAlbumDispatches = []; + + @override + void build() {} + + @override + Future addToAlbum(ActionSource source, RemoteAlbum album) async { + albumSources.add(source); + return ActionResult(count: succeeds ? 1 : 0, success: succeeds); + } + + @override + Future addToSpace(ActionSource source, SharedSpaceResponseDto space) async { + spaceSources.add(source); + return ActionResult(count: succeeds ? 1 : 0, success: succeeds); + } + + @override + Future addToSpaceAlbum(ActionSource source, String spaceId, SpaceAlbum album) async { + spaceAlbumDispatches.add((source, spaceId, album.id)); + return ActionResult(count: succeeds ? 1 : 0, success: succeeds); + } +} + void main() { SharedSpaceMemberResponseDto member(String userId, SharedSpaceRole role) => SharedSpaceMemberResponseDto( userId: userId, @@ -65,14 +103,23 @@ void main() { showInTimeline: true, ); - SharedSpaceResponseDto space(String id, String name) => SharedSpaceResponseDto( + SharedSpaceResponseDto space(String id, String name, {int albums = 0}) => SharedSpaceResponseDto( id: id, name: name, createdAt: '2026-01-01T00:00:00Z', updatedAt: '2026-01-01T00:00:00Z', createdById: 'someone-else', members: Optional.present([member('user-1', SharedSpaceRole.owner)]), - albumCount: const Optional.present(0), + albumCount: Optional.present(albums), + ); + + SpaceAlbum spaceAlbum(String id, String name) => SpaceAlbum( + id: id, + name: name, + showInTimeline: true, + linkedAt: DateTime(2026, 1, 1), + updatedAt: DateTime(2026, 1, 1), + createdAt: DateTime(2026, 1, 1), ); testWidgets('composes the header, the album selector and the spaces section, in that order', (tester) async { @@ -147,4 +194,191 @@ void main() { expect(find.byKey(const Key('space-row-s2')), findsOneWidget); }); + + // #965: the same picker is now mounted from surfaces that have no timeline multiselect — + // the asset viewer above all — so the source it dispatches against and the assets it + // reasons about both have to be things the caller can state. + group('mounted outside the timeline', () { + RemoteAsset asset(String id, {String ownerId = 'user-1'}) => RemoteAsset( + id: id, + name: id, + ownerId: ownerId, + checksum: id, + type: AssetType.image, + createdAt: DateTime(2026, 1, 1), + updatedAt: DateTime(2026, 1, 1), + isEdited: false, + ); + + RemoteAlbum personalAlbum() => RemoteAlbum( + id: 'pa1', + name: 'Personal', + ownerId: 'user-1', + ownerName: 'user-1', + description: '', + createdAt: DateTime(2026, 1, 1), + updatedAt: DateTime(2026, 1, 1), + isActivityEnabled: false, + order: AlbumAssetOrder.desc, + assetCount: 0, + isShared: false, + ); + + Future pumpPicker( + WidgetTester tester, { + required Widget picker, + List spaces = const [], + Map> spaceAlbums = const {}, + List extraOverrides = const [], + }) async { + final userService = _MockUserService(); + final user = UserStub.user1; // id: 'user-1' + when(() => userService.tryGetMyUser()).thenReturn(user); + when(() => userService.watchMyUser()).thenAnswer((_) => const Stream.empty()); + + await tester.pumpConsumerWidgetRaw( + CustomScrollView(slivers: [picker]), + overrides: [ + currentUserProvider.overrideWith((ref) => _StubCurrentUserNotifier(userService, user)), + remoteAlbumProvider.overrideWith(() => _StubRemoteAlbumNotifier()), + appConfigProvider.overrideWithValue(const AppConfig()), + sharedSpacesProvider.overrideWith((ref) async => spaces), + multiSelectProvider.overrideWith( + () => MultiSelectNotifier(const MultiSelectState(selectedAssets: {}, lockedSelectionAssets: {})), + ), + for (final entry in spaceAlbums.entries) + spaceAlbumsProvider(entry.key).overrideWith((ref) => Stream.value(entry.value)), + ...extraOverrides, + ], + ); + await tester.pump(); + } + + /// `ImmichToast` schedules a 3s fluttertoast Timer outside the frame scheduler, so a + /// plain `pumpAndSettle()` leaves it pending and teardown fails with "A Timer is still + /// pending". Pump past its lifetime instead. + Future tapSpaceRow(WidgetTester tester, String id) async { + await tester.tap(find.byKey(Key('space-row-$id'))); + await tester.pumpAndSettle(); + await tester.pump(const Duration(seconds: 4)); + await tester.pumpAndSettle(); + } + + testWidgets('judges space targets by the assets it was given, not the empty multiselect', (tester) async { + await pumpPicker( + tester, + picker: CollectionPicker(assets: [asset('a', ownerId: 'someone-else')]), + spaces: [space('s1', 'Family')], + ); + + expect(find.byKey(const Key('space-row-s1')), findsNothing); + expect(find.byKey(const Key('space-collection-notice')), findsOneWidget); + }); + + testWidgets('dispatches the space pool against the source it was given', (tester) async { + final notifier = _RecordingActionNotifier(); + await pumpPicker( + tester, + picker: CollectionPicker(source: ActionSource.viewer, assets: [asset('a')]), + spaces: [space('s1', 'Family')], + extraOverrides: [actionProvider.overrideWith(() => notifier)], + ); + + await tapSpaceRow(tester, 's1'); + + expect(notifier.spaceSources, [ActionSource.viewer]); + }); + + testWidgets('still defaults to the timeline source', (tester) async { + final notifier = _RecordingActionNotifier(); + await pumpPicker( + tester, + picker: const CollectionPicker(), + spaces: [space('s1', 'Family')], + extraOverrides: [actionProvider.overrideWith(() => notifier)], + ); + + await tapSpaceRow(tester, 's1'); + + expect(notifier.spaceSources, [ActionSource.timeline]); + }); + + testWidgets('reports completion only when the add succeeded', (tester) async { + var completions = 0; + final notifier = _RecordingActionNotifier(succeeds: true); + await pumpPicker( + tester, + picker: CollectionPicker(onCompleted: () => completions++), + spaces: [space('s1', 'Family')], + extraOverrides: [actionProvider.overrideWith(() => notifier)], + ); + + await tapSpaceRow(tester, 's1'); + + expect(completions, 1); + }); + + // The literal subject of #965: reaching an album *inside* a space. Nothing else in this + // suite exercises `addToSpaceAlbum`, so without this the dispatch could break unnoticed. + testWidgets('dispatches a space album with its source, owning space and album id', (tester) async { + final notifier = _RecordingActionNotifier(); + await pumpPicker( + tester, + picker: CollectionPicker(source: ActionSource.viewer, assets: [asset('a')]), + spaces: [space('s1', 'Family', albums: 1)], + spaceAlbums: { + 's1': [spaceAlbum('sa1', 'Holiday')], + }, + extraOverrides: [actionProvider.overrideWith(() => notifier)], + ); + + await tester.tap(find.byKey(const Key('space-row-s1'))); // expands, does not dispatch + await tester.pumpAndSettle(); + await tester.tap(find.byKey(const Key('space-album-child-sa1'))); + await tester.pumpAndSettle(); + await tester.pump(const Duration(seconds: 4)); + await tester.pumpAndSettle(); + + expect(notifier.spaceAlbumDispatches, [(ActionSource.viewer, 's1', 'sa1')]); + expect(notifier.spaceSources, isEmpty, reason: 'an album child must not hit the space pool'); + }); + + // `_addToAlbum` is reached through `AlbumSelector`'s callback. Driving it at that seam keeps + // the test off upstream's list rendering while still running the fork's own dispatch: a + // regression to a hard-coded `timeline` source here would make the asset viewer a silent + // no-op, because the sheet's multiselect is empty. + testWidgets('dispatches a personal album against the source it was given', (tester) async { + final notifier = _RecordingActionNotifier(); + var completions = 0; + await pumpPicker( + tester, + picker: CollectionPicker(source: ActionSource.viewer, assets: [asset('a')], onCompleted: () => completions++), + extraOverrides: [actionProvider.overrideWith(() => notifier)], + ); + + // The callback is void-returning (fire-and-forget), so pump for the dispatch instead. + tester.widget(find.byType(AlbumSelector)).onAlbumSelected(personalAlbum()); + await tester.pumpAndSettle(); + await tester.pump(const Duration(seconds: 4)); + await tester.pumpAndSettle(); + + expect(notifier.albumSources, [ActionSource.viewer]); + expect(completions, 1); + }); + + testWidgets('does not report completion when the add failed', (tester) async { + var completions = 0; + final notifier = _RecordingActionNotifier(succeeds: false); + await pumpPicker( + tester, + picker: CollectionPicker(onCompleted: () => completions++), + spaces: [space('s1', 'Family')], + extraOverrides: [actionProvider.overrideWith(() => notifier)], + ); + + await tapSpaceRow(tester, 's1'); + + expect(completions, 0, reason: 'the sheet must stay open so the user can retry'); + }); + }); } diff --git a/mobile/test/presentation/widgets/collection/space_collection_section_test.dart b/mobile/test/presentation/widgets/collection/space_collection_section_test.dart index cf57e54ebaae4..0a45d8b406eb3 100644 --- a/mobile/test/presentation/widgets/collection/space_collection_section_test.dart +++ b/mobile/test/presentation/widgets/collection/space_collection_section_test.dart @@ -89,10 +89,12 @@ void main() { required List spaces, Map> albums = const {}, List? selection, + List? assets, String? excludeSpaceId, String? userId = 'user-1', String searchQuery = '', bool raw = false, + Set pendingAlbumSpaceIds = const {}, }) async { final targets = []; final overrides = [ @@ -109,12 +111,17 @@ void main() { ), ), for (final entry in albums.entries) - spaceAlbumsProvider(entry.key).overrideWith((ref) => Stream.value(entry.value)), + if (!pendingAlbumSpaceIds.contains(entry.key)) + spaceAlbumsProvider(entry.key).overrideWith((ref) => Stream.value(entry.value)), + // A stream that never emits — the provider stays in its loading state. + for (final spaceId in pendingAlbumSpaceIds) + spaceAlbumsProvider(spaceId).overrideWith((ref) => const Stream>.empty()), ]; final widget = SpaceCollectionSection( onTargetSelected: targets.add, excludeSpaceId: excludeSpaceId, searchQuery: searchQuery, + assets: assets, ); if (raw) { await tester.pumpConsumerWidgetRaw(widget, overrides: overrides); @@ -247,6 +254,23 @@ void main() { expect(targets, hasLength(1)); }); + // "This space has no albums yet" must not flash before the Drift watch has answered — it is + // a claim about the space, and while loading we do not know it. Web draws the same line. + testWidgets('an expanded space says nothing about its albums until the watch answers', (tester) async { + await pump( + tester, + spaces: [space('s1', albums: 2)], + albums: {'s1': const []}, // no stream value pumped for s1 below + pendingAlbumSpaceIds: {'s1'}, + ); + + await tester.tap(find.byKey(const Key('space-row-s1'))); + await tester.pumpAndSettle(); + + expect(find.byKey(const Key('space-pool-child-s1')), findsOneWidget, reason: 'the pool is always reachable'); + expect(find.byKey(const Key('space-albums-empty-s1')), findsNothing); + }); + testWidgets('a double-tap on a plain row emits exactly one target', (tester) async { final targets = await pump(tester, spaces: [space('s1', albums: 0)]); @@ -309,6 +333,35 @@ void main() { expect(find.byKey(const Key('space-row-s1')), findsNothing); }); + // #965: the asset viewer has no multiselect, so it hands the section the one asset the + // viewer is on. Falling back to the (empty) multiselect there would read as "nothing + // non-owned" and offer space targets for a photo that can never reach one. + testWidgets('an explicit asset list is used instead of the multiselect', (tester) async { + await pump( + tester, + spaces: [space('s1')], + selection: [asset('a')], // owned — on its own this would show the rows + assets: [asset('b', ownerId: 'other')], + ); + + expect( + find.text("Your selection includes photos owned by other members, so it can't be added to a space."), + findsOneWidget, + ); + expect(find.byKey(const Key('space-row-s1')), findsNothing); + }); + + testWidgets('an explicit owned asset list still offers the rows', (tester) async { + await pump( + tester, + spaces: [space('s1')], + selection: [asset('a', ownerId: 'other')], // ignored + assets: [asset('b')], + ); + + expect(find.byKey(const Key('space-row-s1')), findsOneWidget); + }); + testWidgets('an unknown current user hides the rows (fail closed)', (tester) async { await pump(tester, spaces: [space('s1')], userId: null); diff --git a/web/src/lib/components/shared-components/collection-selection/collection-selection-utils.spec.ts b/web/src/lib/components/shared-components/collection-selection/collection-selection-utils.spec.ts index 11454f7b57ebf..d46e684793d3b 100644 --- a/web/src/lib/components/shared-components/collection-selection/collection-selection-utils.spec.ts +++ b/web/src/lib/components/shared-components/collection-selection/collection-selection-utils.spec.ts @@ -201,6 +201,7 @@ describe('CollectionModalRowConverter', () => { expect(isSelectableRowType(CollectionModalRowType.NEW_ALBUM)).toBe(true); expect(isSelectableRowType(CollectionModalRowType.NEW_SPACE)).toBe(true); expect(isSelectableRowType(CollectionModalRowType.COLLECTION_ITEM)).toBe(true); + expect(isSelectableRowType(CollectionModalRowType.SPACE_POOL_CHILD)).toBe(true); expect(isSelectableRowType(CollectionModalRowType.SECTION)).toBe(false); expect(isSelectableRowType(CollectionModalRowType.MESSAGE)).toBe(false); }); @@ -214,6 +215,131 @@ describe('CollectionModalRowConverter', () => { expect(items[0].collection!.kind).toBe('album'); }); + // #965: a space row with linked albums expands into "Add to space" plus one row per linked + // album, mirroring mobile's SpaceCollectionSection. A linked album is an ordinary `album` + // collection, so the dispatch (POST /albums/:id/assets) is unchanged. + describe('space expansion (#965)', () => { + const withAlbums = (id: string, name: string, albumCount: number) => s(id, name, { albumCount }); + const items = (rows: ReturnType) => + rows.filter((r) => r.type === CollectionModalRowType.COLLECTION_ITEM); + + it('marks a space with linked albums expandable, and one without not', () => { + const all = [withAlbums('s1', 'Family', 2), s('s2', 'Empty', { albumCount: 0 }), s('s3', 'Unknown')]; + const rows = items(conv.toModalRows('', [], all, -1, [], opts)); + expect(rows.find((r) => r.collection!.id === 's1')!.expandable).toBe(true); + expect(rows.find((r) => r.collection!.id === 's2')!.expandable).toBe(false); + // albumCount absent (older server) — treated as "nothing to expand into". + expect(rows.find((r) => r.collection!.id === 's3')!.expandable).toBe(false); + }); + + it('emits no children until the space is the expanded one', () => { + const all = [withAlbums('s1', 'Family', 2)]; + const rows = conv.toModalRows('', [], all, -1, [], opts); + expect(rows.some((r) => r.type === CollectionModalRowType.SPACE_POOL_CHILD)).toBe(false); + expect(rows.find((r) => r.collection?.id === 's1')!.expanded).toBe(false); + }); + + it('expands into the pool child then one indented album row per linked album', () => { + const all = [withAlbums('s1', 'Family', 2)]; + const rows = conv.toModalRows('', [], all, -1, [], { + ...opts, + expandedSpaceId: 's1', + expandedSpaceAlbums: [a('sa1', 'Holiday'), a('sa2', 'Birthday')], + }); + const kinds = rows + .filter( + (r) => + r.type === CollectionModalRowType.COLLECTION_ITEM || r.type === CollectionModalRowType.SPACE_POOL_CHILD, + ) + .map((r) => `${r.type}:${r.collection!.id}`); + expect(kinds).toEqual([ + `${CollectionModalRowType.COLLECTION_ITEM}:s1`, + `${CollectionModalRowType.SPACE_POOL_CHILD}:s1`, + `${CollectionModalRowType.COLLECTION_ITEM}:sa1`, + `${CollectionModalRowType.COLLECTION_ITEM}:sa2`, + ]); + expect(rows.find((r) => r.collection?.id === 's1')!.expanded).toBe(true); + // The children are the ones that carry the indent, not the space row itself. + expect(rows.find((r) => r.type === CollectionModalRowType.SPACE_POOL_CHILD)!.indented).toBe(true); + expect(rows.find((r) => r.collection?.id === 'sa1')!.indented).toBe(true); + expect(rows.find((r) => r.collection?.id === 's1')!.indented).toBeFalsy(); + }); + + it('emits only the pool child while the albums are still loading', () => { + const all = [withAlbums('s1', 'Family', 2)]; + // `expandedSpaceAlbums` undefined == request in flight. Showing "no albums yet" here would + // be a lie that flashes on every expand. + const rows = conv.toModalRows('', [], all, -1, [], { ...opts, expandedSpaceId: 's1' }); + expect(rows.some((r) => r.type === CollectionModalRowType.SPACE_POOL_CHILD)).toBe(true); + expect(rows.some((r) => r.type === CollectionModalRowType.MESSAGE)).toBe(false); + expect(items(rows)).toHaveLength(1); // the space row only + }); + + it('explains an expanded space that turned out to have no linked albums', () => { + const all = [withAlbums('s1', 'Family', 2)]; + const rows = conv.toModalRows('', [], all, -1, [], { + ...opts, + expandedSpaceId: 's1', + expandedSpaceAlbums: [], + }); + const message = rows.find((r) => r.type === CollectionModalRowType.MESSAGE); + expect(message!.text).toBe('no_albums_in_space_yet'); + expect(message!.indented).toBe(true); + }); + + it('keeps arrow-key order flat: children take the indices right after their space', () => { + const all = [a('a1', 'Aardvark'), withAlbums('s1', 'Family', 1)]; + const selectedAt = (i: number) => + conv.toModalRows('', [], all, i, [], { + ...opts, + expandedSpaceId: 's1', + expandedSpaceAlbums: [a('sa1', 'Holiday')], + }); + // NewAlbum(0) NewSpace(1) a1(2) s1(3) pool(4) sa1(5) + expect(selectedAt(2).find((r) => r.selected && r.collection)!.collection!.id).toBe('a1'); + expect(selectedAt(3).find((r) => r.selected && r.collection)!.collection!.id).toBe('s1'); + const pool = selectedAt(4).find((r) => r.selected)!; + expect(pool.type).toBe(CollectionModalRowType.SPACE_POOL_CHILD); + expect(selectedAt(5).find((r) => r.selected && r.collection)!.collection!.id).toBe('sa1'); + }); + + it('multi-selects a linked album by its own key, and the pool by the space key', () => { + const all = [withAlbums('s1', 'Family', 1)]; + const rows = conv.toModalRows('', [], all, -1, ['album:sa1'], { + ...opts, + expandedSpaceId: 's1', + expandedSpaceAlbums: [a('sa1', 'Holiday')], + }); + expect(rows.find((r) => r.collection?.id === 'sa1')!.multiSelected).toBe(true); + expect(rows.find((r) => r.type === CollectionModalRowType.SPACE_POOL_CHILD)!.multiSelected).toBe(false); + }); + + it('renders the children under every occurrence of the space, RECENT included', () => { + const recent = [withAlbums('s1', 'Family', 1)]; + const all = [withAlbums('s1', 'Family', 1)]; + const rows = conv.toModalRows('', recent, all, -1, [], { + ...opts, + expandedSpaceId: 's1', + expandedSpaceAlbums: [a('sa1', 'Holiday')], + }); + // Same row, same affordance, in both sections — rather than the same row behaving + // differently depending on which section it was rendered in. + expect(rows.filter((r) => r.type === CollectionModalRowType.SPACE_POOL_CHILD)).toHaveLength(2); + expect(rows.filter((r) => r.collection?.id === 'sa1')).toHaveLength(2); + }); + + it('emits nothing space-related when spaces are hidden, even if a space is marked expanded', () => { + const all = [a('a1', 'A'), withAlbums('s1', 'Family', 1)]; + const rows = conv.toModalRows('', [], all, -1, [], { + showSpaces: false, + expandedSpaceId: 's1', + expandedSpaceAlbums: [a('sa1', 'Holiday')], + }); + expect(rows.some((r) => r.type === CollectionModalRowType.SPACE_POOL_CHILD)).toBe(false); + expect(items(rows).map((r) => r.collection!.id)).toEqual(['a1']); + }); + }); + // Restricted (space-contribution) mode: no create rows, and both empty-state messages // are overridable so they never name a collection type that was not on offer. describe('allowCreate / message overrides', () => { diff --git a/web/src/lib/components/shared-components/collection-selection/collection-selection-utils.ts b/web/src/lib/components/shared-components/collection-selection/collection-selection-utils.ts index e3e2fe8291266..aea5c522706bd 100644 --- a/web/src/lib/components/shared-components/collection-selection/collection-selection-utils.ts +++ b/web/src/lib/components/shared-components/collection-selection/collection-selection-utils.ts @@ -71,6 +71,8 @@ export enum CollectionModalRowType { SECTION = 'section', MESSAGE = 'message', COLLECTION_ITEM = 'collectionItem', + /** "Add to space" — the space's own pool, offered as a child of an expanded space row (#965). */ + SPACE_POOL_CHILD = 'spacePoolChild', } export type CollectionModalRow = { @@ -79,12 +81,31 @@ export type CollectionModalRow = { multiSelected?: boolean; text?: string; collection?: PickerCollection; + /** Space rows only: the space has linked albums, so clicking it toggles instead of selecting. */ + expandable?: boolean; + /** Space rows only: this space is the currently-expanded one. */ + expanded?: boolean; + /** A child of an expanded space row — the pool, a linked album, or the empty-state message. */ + indented?: boolean; }; export const isSelectableRowType = (type: CollectionModalRowType): boolean => - [CollectionModalRowType.NEW_ALBUM, CollectionModalRowType.NEW_SPACE, CollectionModalRowType.COLLECTION_ITEM].includes( - type, - ); + [ + CollectionModalRowType.NEW_ALBUM, + CollectionModalRowType.NEW_SPACE, + CollectionModalRowType.COLLECTION_ITEM, + CollectionModalRowType.SPACE_POOL_CHILD, + ].includes(type); + +/** + * Whether a space row can expand into linked-album children. + * + * `albumCount` comes back on every `GET /shared-spaces` row, so this needs no extra request — + * that is the whole point of the accordion: a space's albums are fetched only once the user + * asks for them. An absent count (older server) reads as "nothing to expand into". + */ +const isExpandableSpace = (collection: PickerCollection): boolean => + collection.kind === 'space' && (collection.space.albumCount ?? 0) > 0; export class CollectionModalRowConverter { toModalRows( @@ -93,7 +114,19 @@ export class CollectionModalRowConverter { all: PickerCollection[], selectedRowIndex: number, multiSelectedKeys: string[], - options: { showSpaces: boolean; allowCreate?: boolean; emptyText?: string; noMatchText?: string }, + options: { + showSpaces: boolean; + allowCreate?: boolean; + emptyText?: string; + noMatchText?: string; + /** The one space currently expanded into its linked albums, if any (#965). */ + expandedSpaceId?: string | null; + /** + * The expanded space's linked albums. `undefined` means the request is still in flight — + * distinct from `[]`, which means the space genuinely has none. + */ + expandedSpaceAlbums?: PickerCollection[]; + }, ): CollectionModalRow[] { const $t = get(t); // Restricted mode passes allowCreate:false — a freshly created album is not linked to the @@ -124,14 +157,45 @@ export class CollectionModalRowConverter { } let index = createCount; + const pushSelectable = (row: CollectionModalRow) => { + rows.push({ ...row, selected: selectedRowIndex === index }); + index++; + }; const pushItem = (c: PickerCollection) => { - rows.push({ + pushSelectable({ type: CollectionModalRowType.COLLECTION_ITEM, - selected: selectedRowIndex === index, multiSelected: multiSelectedKeys.includes(collectionKey(c)), collection: c, + expandable: isExpandableSpace(c), + expanded: c.kind === 'space' && c.id === options.expandedSpaceId, }); - index++; + if (c.kind !== 'space' || c.id !== options.expandedSpaceId) { + return; + } + // The pool keeps the space's own collection key, so ticking the parent row and ticking + // "Add to space" are the same multi-select — they name the same destination. + pushSelectable({ + type: CollectionModalRowType.SPACE_POOL_CHILD, + multiSelected: multiSelectedKeys.includes(collectionKey(c)), + collection: c, + indented: true, + }); + const linked = options.expandedSpaceAlbums; + if (linked === undefined) { + return; // still loading — anything else here would be a claim we cannot back yet + } + if (linked.length === 0) { + rows.push({ type: CollectionModalRowType.MESSAGE, text: $t('no_albums_in_space_yet'), indented: true }); + return; + } + for (const child of linked) { + pushSelectable({ + type: CollectionModalRowType.COLLECTION_ITEM, + multiSelected: multiSelectedKeys.includes(collectionKey(child)), + collection: child, + indented: true, + }); + } }; if (recentToShow.length > 0) { diff --git a/web/src/lib/components/shared-components/collection-selection/space-list-item.svelte b/web/src/lib/components/shared-components/collection-selection/space-list-item.svelte index eb9b875398418..32204a9eb0444 100644 --- a/web/src/lib/components/shared-components/collection-selection/space-list-item.svelte +++ b/web/src/lib/components/shared-components/collection-selection/space-list-item.svelte @@ -6,7 +6,7 @@ import { normalizeSearchString } from '$lib/utils/string-utils'; import type { SharedSpaceResponseDto } from '@immich/sdk'; import { Icon } from '@immich/ui'; - import { mdiAccountMultipleOutline, mdiCheckCircle } from '@mdi/js'; + import { mdiAccountMultipleOutline, mdiCheckCircle, mdiChevronDown, mdiChevronUp } from '@mdi/js'; import type { Action } from 'svelte/action'; import { t } from 'svelte-i18n'; @@ -15,6 +15,9 @@ searchQuery?: string; selected: boolean; multiSelected?: boolean; + /** The space has linked albums, so clicking the row opens them instead of picking the pool. */ + expandable?: boolean; + expanded?: boolean; onSpaceClick: () => void; onMultiSelect: () => void; } @@ -24,6 +27,8 @@ searchQuery = '', selected = false, multiSelected = false, + expandable = false, + expanded = false, onSpaceClick, onMultiSelect, }: Props = $props(); @@ -86,6 +91,7 @@ class:dark:bg-gray-700={selected} use:longPress={{ onLongPress: () => handleMultiSelectClicked() }} data-testid="space-row" + aria-expanded={expandable ? expanded : undefined} > @@ -110,6 +116,13 @@ {/if} + {#if expandable} + + + + + {/if} {#if mouseOver || multiSelected} diff --git a/web/src/lib/components/shared-components/collection-selection/space-pool-list-item.svelte b/web/src/lib/components/shared-components/collection-selection/space-pool-list-item.svelte new file mode 100644 index 0000000000000..68f9c87cd8ee7 --- /dev/null +++ b/web/src/lib/components/shared-components/collection-selection/space-pool-list-item.svelte @@ -0,0 +1,88 @@ + + +
{ + if (!usingMobileDevice) { + mouseOver = true; + } + }} + onmouseleave={() => (mouseOver = false)} +> + + + {#if mouseOver || multiSelected} + + {/if} +
diff --git a/web/src/lib/modals/CollectionPickerModal.spec.ts b/web/src/lib/modals/CollectionPickerModal.spec.ts index d2ece5949be30..059469f77c91a 100644 --- a/web/src/lib/modals/CollectionPickerModal.spec.ts +++ b/web/src/lib/modals/CollectionPickerModal.spec.ts @@ -135,6 +135,231 @@ describe('CollectionPickerModal', () => { }); }); +// --------------------------------------------------------------------------- +// #965: a space with linked albums expands into "Add to space" plus one row per +// linked album, so a specific space album is reachable from every surface — the +// same accordion mobile's SpaceCollectionSection already offers. +// --------------------------------------------------------------------------- + +describe('CollectionPickerModal — expanding a space into its albums', () => { + const spaceWithAlbums = (id: string, name: string, albumCount: number) => + ({ ...space(id, name), albumCount }) as unknown as SharedSpaceResponseDto; + const linkedAlbum = (id: string, name: string) => + ({ + id, + albumName: name, + assetCount: 2, + albumThumbnailAssetId: null, + shared: true, + updatedAt: '2024-01-01T00:00:00Z', + ownerId: 'someone-else', + showInTimeline: true, + addedById: 'me', + linkedAt: '2024-01-01T00:00:00Z', + }) as never; + + // A space shows up in both RECENT and All, so every row lookup takes the first occurrence. + const expandSpace = async (id: string) => { + const rows = await screen.findAllByTestId(`row-space-${id}`); + await fireEvent.click(within(rows[0]).getByTestId('space-row')); + }; + + it('marks a space with linked albums expandable without fetching them up front', async () => { + sdkMock.getAllSpaces.mockResolvedValue([spaceWithAlbums('s1', 'Family', 2)]); + render(CollectionPickerModal, { assetCount: 3, onClose: vi.fn() }); + + const rows = await screen.findAllByTestId('row-space-s1'); + expect(within(rows[0]).getByTestId('space-row').getAttribute('aria-expanded')).toBe('false'); + // The whole point of the accordion: albumCount already says it is expandable. + expect(sdkMock.getSharedSpaceAlbums).not.toHaveBeenCalled(); + }); + + it('fetches and lists the linked albums plus the pool child on expand', async () => { + sdkMock.getAllSpaces.mockResolvedValue([spaceWithAlbums('s1', 'Family', 2)]); + sdkMock.getSharedSpaceAlbums.mockResolvedValue([linkedAlbum('sa1', 'Holiday'), linkedAlbum('sa2', 'Birthday')]); + render(CollectionPickerModal, { assetCount: 3, onClose: vi.fn() }); + + await expandSpace('s1'); + + await waitFor(() => expect(screen.getAllByTestId('row-album-sa1').length).toBeGreaterThan(0)); + expect(sdkMock.getSharedSpaceAlbums).toHaveBeenCalledWith({ id: 's1' }); + expect(screen.getAllByTestId('row-album-sa2').length).toBeGreaterThan(0); + expect(screen.getAllByTestId('space-pool-child-s1').length).toBeGreaterThan(0); + }); + + it('confirms with the linked album when one is chosen', async () => { + const onClose = vi.fn(); + sdkMock.getAllSpaces.mockResolvedValue([spaceWithAlbums('s1', 'Family', 1)]); + sdkMock.getSharedSpaceAlbums.mockResolvedValue([linkedAlbum('sa1', 'Holiday')]); + render(CollectionPickerModal, { assetCount: 3, onClose }); + + await expandSpace('s1'); + const albumRows = await screen.findAllByTestId('row-album-sa1'); + await fireEvent.click(within(albumRows[0]).getByRole('button', { name: /Holiday/ })); + + expect(onClose).toHaveBeenCalledWith([expect.objectContaining({ kind: 'album', id: 'sa1' })]); + }); + + it('confirms with the space itself when the pool child is chosen', async () => { + const onClose = vi.fn(); + sdkMock.getAllSpaces.mockResolvedValue([spaceWithAlbums('s1', 'Family', 1)]); + sdkMock.getSharedSpaceAlbums.mockResolvedValue([linkedAlbum('sa1', 'Holiday')]); + render(CollectionPickerModal, { assetCount: 3, onClose }); + + await expandSpace('s1'); + const poolRows = await screen.findAllByTestId('space-pool-child-s1'); + await fireEvent.click(poolRows[0]); + + expect(onClose).toHaveBeenCalledWith([expect.objectContaining({ kind: 'space', id: 's1' })]); + }); + + it('keeps one space open at a time and does not re-fetch a space it already loaded', async () => { + sdkMock.getAllSpaces.mockResolvedValue([spaceWithAlbums('s1', 'Family', 1), spaceWithAlbums('s2', 'Friends', 1)]); + sdkMock.getSharedSpaceAlbums.mockImplementation(({ id }: { id: string }) => + Promise.resolve(id === 's1' ? [linkedAlbum('sa1', 'Holiday')] : [linkedAlbum('sa2', 'Birthday')]), + ); + render(CollectionPickerModal, { assetCount: 3, onClose: vi.fn() }); + + await expandSpace('s1'); + await waitFor(() => expect(screen.getAllByTestId('row-album-sa1').length).toBeGreaterThan(0)); + + await expandSpace('s2'); + await waitFor(() => expect(screen.getAllByTestId('row-album-sa2').length).toBeGreaterThan(0)); + expect(screen.queryAllByTestId('row-album-sa1')).toHaveLength(0); + + await expandSpace('s1'); + await waitFor(() => expect(screen.getAllByTestId('row-album-sa1').length).toBeGreaterThan(0)); + expect(sdkMock.getSharedSpaceAlbums).toHaveBeenCalledTimes(2); // s1 and s2, once each + }); + + it('collapses again on a second click', async () => { + sdkMock.getAllSpaces.mockResolvedValue([spaceWithAlbums('s1', 'Family', 1)]); + sdkMock.getSharedSpaceAlbums.mockResolvedValue([linkedAlbum('sa1', 'Holiday')]); + render(CollectionPickerModal, { assetCount: 3, onClose: vi.fn() }); + + await expandSpace('s1'); + await waitFor(() => expect(screen.getAllByTestId('row-album-sa1').length).toBeGreaterThan(0)); + await expandSpace('s1'); + await waitFor(() => expect(screen.queryAllByTestId('row-album-sa1')).toHaveLength(0)); + }); + + it('still adds straight to the pool for a space with no linked albums', async () => { + const onClose = vi.fn(); + sdkMock.getAllSpaces.mockResolvedValue([spaceWithAlbums('s1', 'Family', 0)]); + render(CollectionPickerModal, { assetCount: 3, onClose }); + + await expandSpace('s1'); + + expect(sdkMock.getSharedSpaceAlbums).not.toHaveBeenCalled(); + expect(onClose).toHaveBeenCalledWith([expect.objectContaining({ kind: 'space', id: 's1' })]); + }); + + // The caret is an index into a row list that changes shape when a space opens. Clearing it + // (or leaving it pointing at a shifted row) stranded a keyboard user: they could open a space + // and then had no way to arrow into the children they had just revealed. + it('keeps the keyboard caret on the space row across expand and collapse', async () => { + const onClose = vi.fn(); + sdkMock.getAllSpaces.mockResolvedValue([spaceWithAlbums('s1', 'Family', 1)]); + sdkMock.getSharedSpaceAlbums.mockResolvedValue([linkedAlbum('sa1', 'Holiday')]); + render(CollectionPickerModal, { assetCount: 3, onClose }); + await screen.findAllByTestId('row-space-s1'); + const search = screen.getByPlaceholderText('search'); + + // NewAlbum, NewSpace, then the RECENT occurrence of the space. + await fireEvent.keyDown(search, { key: 'ArrowDown' }); + await fireEvent.keyDown(search, { key: 'ArrowDown' }); + await fireEvent.keyDown(search, { key: 'ArrowDown' }); + await fireEvent.keyDown(search, { key: 'Enter' }); // expand + await waitFor(() => expect(screen.getAllByTestId('row-album-sa1').length).toBeGreaterThan(0)); + + // The very next ArrowDown must reach the pool child, not jump back to the top of the list. + await fireEvent.keyDown(search, { key: 'ArrowDown' }); + await fireEvent.keyDown(search, { key: 'Enter' }); + expect(onClose).toHaveBeenCalledWith([expect.objectContaining({ kind: 'space', id: 's1' })]); + }); + + it('reports a failed album load and leaves the row collapsed', async () => { + sdkMock.getAllSpaces.mockResolvedValue([spaceWithAlbums('s1', 'Family', 2)]); + sdkMock.getSharedSpaceAlbums.mockRejectedValue(new Error('boom')); + render(CollectionPickerModal, { assetCount: 3, onClose: vi.fn() }); + + await expandSpace('s1'); + + await waitFor(() => expect(mockHandleError).toHaveBeenCalledOnce()); + const rows = screen.getAllByTestId('row-space-s1'); + expect(within(rows[0]).getByTestId('space-row').getAttribute('aria-expanded')).toBe('false'); + expect(screen.queryAllByTestId('space-pool-child-s1')).toHaveLength(0); + }); + + // A space-linked album owned by another member has no `album_user` row for the caller, so + // `getAllAlbums` never returns it — it exists only in the expand-time cache. Resolving + // multi-select keys against the album list alone dropped it and closed the modal as a cancel. + it('multi-selects a linked album and actually submits it', async () => { + const onClose = vi.fn(); + sdkMock.getAllSpaces.mockResolvedValue([spaceWithAlbums('s1', 'Family', 1)]); + sdkMock.getSharedSpaceAlbums.mockResolvedValue([linkedAlbum('sa1', 'Holiday')]); + render(CollectionPickerModal, { assetCount: 3, onClose }); + + await expandSpace('s1'); + const albumRows = await screen.findAllByTestId('row-album-sa1'); + const albumRow = albumRows[0]; + await fireEvent.mouseEnter(within(albumRow).getByRole('group')); + await fireEvent.click(within(albumRow).getByRole('checkbox')); + await fireEvent.click(await screen.findByTestId('add-collections-button')); + + expect(onClose).toHaveBeenCalledWith([expect.objectContaining({ kind: 'album', id: 'sa1' })]); + }); + + it('multi-selects the pool child, and shows the tick on it', async () => { + const onClose = vi.fn(); + sdkMock.getAllSpaces.mockResolvedValue([spaceWithAlbums('s1', 'Family', 1)]); + sdkMock.getSharedSpaceAlbums.mockResolvedValue([linkedAlbum('sa1', 'Holiday')]); + render(CollectionPickerModal, { assetCount: 3, onClose }); + + await expandSpace('s1'); + const poolChildren = await screen.findAllByTestId('space-pool-child-s1'); + const poolRow = poolChildren[0].closest('[role="group"]') as HTMLElement; + await fireEvent.mouseEnter(poolRow); + await fireEvent.click(within(poolRow).getByRole('checkbox')); + + // The pool carries the space's own key, so the tick must be visible on the child too — + // not merely computed in the row model. + expect(within(poolRow).getByRole('checkbox').getAttribute('aria-checked')).toBe('true'); + await fireEvent.click(await screen.findByTestId('add-collections-button')); + expect(onClose).toHaveBeenCalledWith([expect.objectContaining({ kind: 'space', id: 's1' })]); + }); + + it('does not fire a second request when re-expanded while the first is still in flight', async () => { + sdkMock.getAllSpaces.mockResolvedValue([spaceWithAlbums('s1', 'Family', 1)]); + let resolveFetch: (albums: never[]) => void = () => {}; + sdkMock.getSharedSpaceAlbums.mockReturnValue( + new Promise((resolve) => { + resolveFetch = resolve; + }), + ); + render(CollectionPickerModal, { assetCount: 3, onClose: vi.fn() }); + + await expandSpace('s1'); // fetch starts + await expandSpace('s1'); // collapse + await expandSpace('s1'); // re-expand before the first response lands + resolveFetch([linkedAlbum('sa1', 'Holiday')]); + + await waitFor(() => expect(screen.getAllByTestId('row-album-sa1').length).toBeGreaterThan(0)); + expect(sdkMock.getSharedSpaceAlbums).toHaveBeenCalledTimes(1); + }); + + it('says so when an expanded space turns out to have no linked albums', async () => { + sdkMock.getAllSpaces.mockResolvedValue([spaceWithAlbums('s1', 'Family', 2)]); + sdkMock.getSharedSpaceAlbums.mockResolvedValue([]); + render(CollectionPickerModal, { assetCount: 3, onClose: vi.fn() }); + + await expandSpace('s1'); + + // Raw i18n key in unit tests. + await waitFor(() => expect(screen.getAllByText('no_albums_in_space_yet').length).toBeGreaterThan(0)); + }); +}); + // --------------------------------------------------------------------------- // Restricted mode: the selection contains assets the user does not own, so the // only targets that can accept the whole selection are albums linked to THIS diff --git a/web/src/lib/modals/CollectionPickerModal.svelte b/web/src/lib/modals/CollectionPickerModal.svelte index 597af4fd1ee8f..1c02ccd425b3b 100644 --- a/web/src/lib/modals/CollectionPickerModal.svelte +++ b/web/src/lib/modals/CollectionPickerModal.svelte @@ -12,10 +12,12 @@ isWritableSpace, pickRecent, spaceToCollection, + type CollectionModalRow, type PickerCollection, } from '$lib/components/shared-components/collection-selection/collection-selection-utils'; import NewSpaceListItem from '$lib/components/shared-components/collection-selection/new-space-list-item.svelte'; import SpaceListItem from '$lib/components/shared-components/collection-selection/space-list-item.svelte'; + import SpacePoolListItem from '$lib/components/shared-components/collection-selection/space-pool-list-item.svelte'; import { MAX_SPACE_ASSETS_PER_REQUEST } from '$lib/constants'; import { authManager } from '$lib/managers/auth-manager.svelte'; import { eventManager } from '$lib/managers/event-manager.svelte'; @@ -32,6 +34,7 @@ import { Button, Icon, Modal, ModalBody, ModalFooter, Text } from '@immich/ui'; import { mdiImageMultipleOutline, mdiInformationOutline, mdiKeyboardReturn } from '@mdi/js'; import { onMount } from 'svelte'; + import { SvelteSet } from 'svelte/reactivity'; import { t } from 'svelte-i18n'; interface Props { @@ -69,6 +72,16 @@ const recentCollections = $derived(pickRecent(allCollections, 3)); + // #965: the linked albums of the one expanded space. Fetched lazily on expand and kept for + // the life of the modal, so re-opening a space costs nothing and opening the picker still + // costs exactly two requests however many spaces the user is in. + let expandedSpaceId = $state(null); + let spaceAlbumCache = $state>({}); + // Guards duplicate requests; nothing renders from it. `SvelteSet` rather than a plain `Set` + // because svelte/prefer-svelte-reactivity forbids mutable built-in Sets in components. + const spaceAlbumsInFlight = new SvelteSet(); + const expandedSpaceAlbums = $derived(expandedSpaceId === null ? undefined : spaceAlbumCache[expandedSpaceId]); + const converter = new CollectionModalRowConverter(); const rows = $derived( converter.toModalRows(search, recentCollections, allCollections, selectedRowIndex, multiSelectedKeys, { @@ -78,6 +91,8 @@ // would name a collection type that was never on offer. emptyText: restricted ? $t('no_albums_in_space_yet') : undefined, noMatchText: restricted ? $t('no_albums_found') : undefined, + expandedSpaceId, + expandedSpaceAlbums, }), ); const selectableRowCount = $derived(rows.filter((row) => isSelectableRowType(row.type)).length); @@ -111,16 +126,93 @@ // `SharedSpaceLinkedAlbumDto` is `AlbumResponseDto` minus `albumUsers` (plus link metadata), // so shim the missing field back in for AlbumListItem. The membership list is unused here. - const loadSpaceAlbums = async (spaceId: string) => { + const fetchSpaceAlbums = async (spaceId: string): Promise => { const linked = await getSharedSpaceAlbums({ id: spaceId }); - albums = linked.map((album) => ({ ...album, albumUsers: [] }) as AlbumResponseDto); + return linked.map((album) => ({ ...album, albumUsers: [] }) as AlbumResponseDto); + }; + + const loadSpaceAlbums = async (spaceId: string) => { + albums = await fetchSpaceAlbums(spaceId); + }; + + /** + * Open a space's linked albums, or close them again. + * + * Accordion, like mobile: at most one space is open, so the row list stays short and only + * one space's albums are ever in memory. A failed fetch collapses the row rather than + * leaving it stuck open on a spinner that will never resolve. + */ + const toggleSpaceExpansion = async (collection: PickerCollection) => { + if (collection.kind !== 'space') { + return; + } + expandedSpaceId = expandedSpaceId === collection.id ? null : collection.id; + // Toggling inserts or removes rows, so every index after this space shifts. Re-anchor the + // caret on the space row itself — leaving it where it was would point at a different row, + // and clearing it would strand a keyboard user who has to walk the list again to reach the + // children they just revealed. + reanchorCaretOnSpace(collection.id); + if (expandedSpaceId !== collection.id || Object.hasOwn(spaceAlbumCache, collection.id)) { + return; // collapsed, or already fetched once this modal was opened + } + if (spaceAlbumsInFlight.has(collection.id)) { + return; // a collapse/re-expand while the first request is still out + } + spaceAlbumsInFlight.add(collection.id); + try { + const linked = await fetchSpaceAlbums(collection.id); + spaceAlbumCache[collection.id] = linked.map((album) => albumToCollection(album)); + } catch (error) { + handleError(error, $t('errors.unable_to_load_albums')); + if (expandedSpaceId === collection.id) { + expandedSpaceId = null; + } + } finally { + spaceAlbumsInFlight.delete(collection.id); + } + }; + + /** + * Put the arrow-key caret back on a space row after its children appeared or disappeared. + * + * Only when the caret was already in use — a mouse user who clicks a row should not suddenly + * acquire a keyboard selection highlight. `rows` is `$derived`, so reading it here sees the + * post-toggle list. + */ + const reanchorCaretOnSpace = (spaceId: string) => { + if (selectedRowIndex === -1) { + return; + } + let index = -1; + for (const row of rows) { + if (!isSelectableRowType(row.type)) { + continue; + } + index++; + if (row.type === CollectionModalRowType.COLLECTION_ITEM && row.collection?.id === spaceId) { + selectedRowIndex = index; + return; + } + } + selectedRowIndex = -1; }; const loadSpaces = async () => { spaces = await getAllSpaces(); }; - const findByKey = (key: string) => allCollections.find((collection) => collectionKey(collection) === key); + /** + * Resolve a multi-select key back to its collection. + * + * Must search the fetched space albums too, not just `allCollections`: a space-linked album + * owned by another member has no `album_user` row for the caller, so `getAllAlbums` never + * returns it — which is precisely the #965 case. Missing it here made `submitMulti` resolve + * the key to `undefined`, drop it, and close the modal as if the user had cancelled. + */ + const findByKey = (key: string) => + [...allCollections, ...Object.values(spaceAlbumCache).flat()].find( + (collection) => collectionKey(collection) === key, + ); const toggleMultiSelect = (collection?: PickerCollection) => { const target = collection ?? rows.find((row) => row.selected)?.collection; @@ -144,6 +236,13 @@ onClose([collection]); }; + /** + * What clicking a space row's body does. An expandable one opens instead of picking — its + * pool stays reachable as the "Add to space" child, and via the row's own checkbox. + */ + const handleSpaceClick = (row: CollectionModalRow, collection: PickerCollection) => + row.expandable ? void toggleSpaceExpansion(collection) : handleCollectionClick(collection); + const submitMulti = () => { const selected = multiSelectedKeys .map((key) => findByKey(key)) @@ -190,7 +289,12 @@ } break; } - case CollectionModalRowType.COLLECTION_ITEM: { + case CollectionModalRowType.COLLECTION_ITEM: + case CollectionModalRowType.SPACE_POOL_CHILD: { + if (item.expandable && item.collection) { + await toggleSpaceExpansion(item.collection); + return; // toggling re-anchored the caret on the space row; don't clear it below + } if (multiSelectActive) { submitMulti(); } else if (item.collection) { @@ -286,10 +390,20 @@ {:else if row.type === CollectionModalRowType.SECTION}

{row.text}

{:else if row.type === CollectionModalRowType.MESSAGE} -

{row.text}

+ +

{row.text}

+ {:else if row.type === CollectionModalRowType.SPACE_POOL_CHILD && row.collection} + {@const collection = row.collection} + handleCollectionClick(collection)} + onMultiSelect={() => toggleMultiSelect(collection)} + /> {:else if row.type === CollectionModalRowType.COLLECTION_ITEM && row.collection} {@const collection = row.collection} -
+
{#if collection.kind === 'album'} handleCollectionClick(collection)} + onSpaceClick={() => handleSpaceClick(row, collection)} onMultiSelect={() => toggleMultiSelect(collection)} /> {/if}