Reconcile asset selection with visible items #8296
Conversation
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 01/24/2026, 04:04:42 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Tests:
|
dosubot
bot
added
the
size:L
There was a problem hiding this comment.
Pull request overview
Tracks asset selection anchor by asset id and introduces reconciliation logic intended to keep selection/anchor aligned with the currently visible asset list (plus a small queue overlay alignment update).
Changes:
- Add
lastSelectedAssetIdto the asset selection Pinia store and clear it with selection resets. - Add anchor-by-id helpers and
reconcileSelection(assets)to prune selection and resync anchor index. - Add Vitest coverage for prune/reorder/anchor-clear scenarios; update queue overlay to set anchor id when focusing an asset.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/platform/assets/composables/useAssetSelectionStore.ts | Adds lastSelectedAssetId state + setter and resets it on clearSelection(). |
| src/platform/assets/composables/useAssetSelection.ts | Adds anchor-by-id tracking and reconcileSelection() API for pruning/resyncing selection/anchor. |
| src/platform/assets/composables/useAssetSelection.test.ts | Adds tests covering pruning, reorder anchor resync, and anchor clearing. |
| src/components/queue/QueueProgressOverlay.vue | Ensures queue “focus asset” selection also sets lastSelectedAssetId. |
| assetSelectionStore.setSelection([assetId]) | ||
| assetSelectionStore.setLastSelectedAssetId(assetId) | ||
| } |
There was a problem hiding this comment.
focusAssetInSidebar sets setSelection([assetId]) and now sets lastSelectedAssetId, but it leaves lastSelectedIndex unchanged. Since shift-range selection is keyed off lastSelectedIndex, this can produce an incorrect range anchored to a stale index after selecting from the queue overlay. Consider also resetting lastSelectedIndex (or setting it to the focused asset’s index when available) to keep the anchor state consistent.
| const assets: AssetItem[] = [ | ||
| { id: 'a', name: 'a.png', tags: [] }, | ||
| { id: 'b', name: 'b.png', tags: [] } | ||
| ] |
There was a problem hiding this comment.
The test fixtures typed as AssetItem only provide { id, name, tags }, but AssetItem (from assetSchema) includes other required fields (e.g. asset_hash, mime_type, etc.). If the repo runs TypeScript typechecking in CI, this test file will fail to compile. Suggest using an existing AssetItem factory (or a local helper) that populates the required fields, or loosening the fixture typing (e.g. Partial<AssetItem> where appropriate).
| function reconcileSelection(assets: AssetItem[]) { | ||
| if (selectionStore.selectedAssetIds.size === 0) { | ||
| return | ||
| } |
There was a problem hiding this comment.
reconcileSelection is implemented and returned from the composable, but it isn’t called anywhere in the codebase (search shows only this file + its new test). Without wiring it to changes in the visible asset list (e.g., a watcher where displayAssets is derived), selection won’t actually be pruned/re-anchored at runtime as described in the PR summary.
|
Replaced by #8283 |
2 participants
Reconcile asset selection and anchor state with visible assets.
Summary
Track selection anchor by asset id and prune selection when assets are hidden or reordered so list/gallery actions stay aligned.
Changes
useAssetSelection, plus coverage and queue overlay alignment.Review Focus
Screenshots (if applicable)
N/A
┆Issue is synchronized with this Notion page by Unito