From d077d44540c30e0e2a6c10effe629574b557342e Mon Sep 17 00:00:00 2001 From: "Calum H." Date: Thu, 21 May 2026 23:03:35 +0100 Subject: [PATCH] fix: content tab uniqueness regression (#6156) * fix: content tab uniqueness regression Closes: #6154 * fix: further regressions * fix: lint * fix: lint --- apps/app-frontend/src/pages/instance/Mods.vue | 19 ++++++++--- .../app-lib/src/state/instances/content.rs | 6 ++-- packages/ui/src/composables/virtual-scroll.ts | 8 +++-- .../components/ContentSelectionBar.vue | 10 ++++-- .../composables/content-selection.ts | 26 +++++++++------ .../src/layouts/shared/content-tab/layout.vue | 32 ++++++++++++------- .../content-tab/providers/content-manager.ts | 3 ++ .../wrapped/hosting/manage/content.vue | 9 ++++-- 8 files changed, 77 insertions(+), 36 deletions(-) diff --git a/apps/app-frontend/src/pages/instance/Mods.vue b/apps/app-frontend/src/pages/instance/Mods.vue index f78cf6430..f64101f56 100644 --- a/apps/app-frontend/src/pages/instance/Mods.vue +++ b/apps/app-frontend/src/pages/instance/Mods.vue @@ -230,8 +230,12 @@ function fileNameFromPath(path: string) { return path.split('/').pop() ?? path } +function getContentItemId(item: ContentItem | null | undefined) { + return item?.file_path ?? item?.file_name ?? item?.id ?? '' +} + function getContentOperationKeys(item: ContentItem) { - return [item.id, item.file_path, item.file_name, item.project?.id, item.version?.id].filter( + return [getContentItemId(item), item.file_path, item.file_name].filter( (key): key is string => !!key, ) } @@ -478,10 +482,11 @@ async function switchProjectVersion(mod: ContentItem, version: Labrinth.Versions } async function handleUpdate(id: string) { - const item = projects.value.find((p) => p.id === id) + const item = projects.value.find((p) => getContentItemId(p) === id) if (!item?.has_update || !item.project?.id || !item.version?.id) return const requestId = beginUpdateRequest() + const itemId = getContentItemId(item) debug('handleUpdate triggered', { fileName: item.file_name, @@ -542,7 +547,8 @@ async function handleUpdate(id: string) { return handleError(e) })) as Labrinth.Versions.v2.Version[] | null - if (!isActiveUpdateRequest(requestId) || updatingProject.value?.id !== item.id) return + if (!isActiveUpdateRequest(requestId) || getContentItemId(updatingProject.value) !== itemId) + return loadingVersions.value = false @@ -595,6 +601,7 @@ async function handleSwitchVersion(item: ContentItem) { if (!item.project?.id || !item.version?.id) return const requestId = beginUpdateRequest() + const itemId = getContentItemId(item) updatingModpack.value = false updatingProject.value = item @@ -610,7 +617,8 @@ async function handleSwitchVersion(item: ContentItem) { return handleError(e) })) as Labrinth.Versions.v2.Version[] | null - if (!isActiveUpdateRequest(requestId) || updatingProject.value?.id !== item.id) return + if (!isActiveUpdateRequest(requestId) || getContentItemId(updatingProject.value) !== itemId) + return loadingVersions.value = false @@ -1055,8 +1063,9 @@ provideContentManager({ showContentHint, dismissContentHint, shareItems: handleShareItems, + getItemId: getContentItemId, mapToTableItem: (item: ContentItem) => ({ - id: item.id, + id: getContentItemId(item), project: item.project ?? { id: item.file_name, slug: null, diff --git a/packages/app-lib/src/state/instances/content.rs b/packages/app-lib/src/state/instances/content.rs index 4e7e132a8..c4a5252e4 100644 --- a/packages/app-lib/src/state/instances/content.rs +++ b/packages/app-lib/src/state/instances/content.rs @@ -32,12 +32,12 @@ use std::io::Cursor; /// Content item with rich metadata for frontend display #[derive(Debug, Serialize, Deserialize, Clone)] pub struct ContentItem { - /// Unique identifier (the file name) + /// Display file name. pub file_name: String, /// Relative path to the file within the profile pub file_path: String, - /// Stable frontend identifier (SHA1 hash of file content, survives renames). - /// Not a project or version ID. + /// SHA1 hash of file content. Stable across renames, but not unique when + /// duplicate files have identical contents. pub id: String, /// File size in bytes pub size: u64, diff --git a/packages/ui/src/composables/virtual-scroll.ts b/packages/ui/src/composables/virtual-scroll.ts index 6b641efd4..5cfa12ad6 100644 --- a/packages/ui/src/composables/virtual-scroll.ts +++ b/packages/ui/src/composables/virtual-scroll.ts @@ -84,9 +84,13 @@ export function useVirtualScroll(items: Ref, options: VirtualScrollOptio const start = Math.floor(relativeScrollTop / itemHeight) const visibleCount = Math.ceil(viewportHeight.value / itemHeight) + const rangeSize = visibleCount + bufferSize * 2 - const rangeStart = Math.max(0, start - bufferSize) - const rangeEnd = Math.min(items.value.length, start + visibleCount + bufferSize * 2) + const rangeStart = Math.min( + Math.max(0, start - bufferSize), + Math.max(0, items.value.length - rangeSize), + ) + const rangeEnd = Math.min(items.value.length, rangeStart + rangeSize) return { start: rangeStart, diff --git a/packages/ui/src/layouts/shared/content-tab/components/ContentSelectionBar.vue b/packages/ui/src/layouts/shared/content-tab/components/ContentSelectionBar.vue index bd57e1cd0..4b7167716 100644 --- a/packages/ui/src/layouts/shared/content-tab/components/ContentSelectionBar.vue +++ b/packages/ui/src/layouts/shared/content-tab/components/ContentSelectionBar.vue @@ -74,6 +74,7 @@ interface Props { bulkTotal?: number bulkWaiting?: boolean ariaLabel?: string + getItemId?: (item: ContentItem) => string } const props = withDefaults(defineProps(), { @@ -85,6 +86,7 @@ const props = withDefaults(defineProps(), { bulkTotal: 0, bulkWaiting: false, ariaLabel: undefined, + getItemId: undefined, }) const emit = defineEmits<{ @@ -102,6 +104,10 @@ const iconStackWidth = computed(() => { return 32 + (visibleItems.value.length - 1 + (overflowCount.value > 0 ? 1 : 0)) * iconStackOffset }) +function resolveItemId(item: ContentItem) { + return props.getItemId?.(item) ?? item.file_path ?? item.file_name ?? item.id +} + const allDisabled = computed(() => props.selectedItems.every((m) => !m.enabled)) const allEnabled = computed(() => props.selectedItems.every((m) => m.enabled)) @@ -146,7 +152,7 @@ const bulkProgressMessage = computed(() => { >
{ ) { +export function useContentSelection( + items: Ref, + getItemId: (item: ContentItem) => string, +) { const selectedIds = ref([]) const selectedItems = computed(() => - items.value.filter((item) => selectedIds.value.includes(item.id)), + items.value.filter((item) => selectedIds.value.includes(getItemId(item))), ) - watch(items, (newItems) => { - if (selectedIds.value.length === 0) return - const validIds = new Set(newItems.map((item) => item.id)) - const pruned = selectedIds.value.filter((id) => validIds.has(id)) - if (pruned.length !== selectedIds.value.length) { - selectedIds.value = pruned - } - }) + watch( + () => items.value.map(getItemId), + (newIds) => { + if (selectedIds.value.length === 0) return + const validIds = new Set(newIds) + const pruned = selectedIds.value.filter((id) => validIds.has(id)) + if (pruned.length !== selectedIds.value.length) { + selectedIds.value = pruned + } + }, + ) function clearSelection() { selectedIds.value = [] diff --git a/packages/ui/src/layouts/shared/content-tab/layout.vue b/packages/ui/src/layouts/shared/content-tab/layout.vue index bb9574409..ed8bbf9cd 100644 --- a/packages/ui/src/layouts/shared/content-tab/layout.vue +++ b/packages/ui/src/layouts/shared/content-tab/layout.vue @@ -151,6 +151,10 @@ const messages = defineMessages({ const ctx = injectContentManager() +function getItemId(item: ContentItem) { + return ctx.getItemId?.(item) ?? item.file_path ?? item.file_name ?? item.id +} + type SortMode = 'alphabetical-asc' | 'alphabetical-desc' | 'date-added-newest' | 'date-added-oldest' const sortMode = ref('alphabetical-asc') @@ -227,6 +231,7 @@ const { selectedFilters, filterOptions, toggleFilter, applyFilters } = useConten const { selectedIds, selectedItems, clearSelection, removeFromSelection } = useContentSelection( ctx.items, + getItemId, ) const { isBulkOperating, bulkProgress, bulkTotal, bulkOperation, runBulk } = useBulkOperation() @@ -261,13 +266,12 @@ const filteredItems = computed(() => { const tableItems = computed(() => { const items = filteredItems.value.map((item) => { const base = ctx.mapToTableItem(item) + const id = getItemId(item) return { ...base, + id, disabled: - isChanging(base.id) || - ctx.isBusy.value || - isBulkOperating.value || - item.installing === true, + isChanging(id) || ctx.isBusy.value || isBulkOperating.value || item.installing === true, installing: item.installing === true, hasUpdate: item.has_update, isClientOnly: @@ -314,7 +318,7 @@ const pendingDeletionItems = ref([]) const confirmDeletionModal = ref>() function handleDeleteById(id: string, event?: MouseEvent) { - const item = ctx.items.value.find((i) => i.id === id) + const item = ctx.items.value.find((i) => getItemId(i) === id) if (item) { pendingDeletionItems.value = [item] if (event?.shiftKey) { @@ -356,11 +360,14 @@ async function confirmDelete() { if (itemsToDelete.length === 1) { const item = itemsToDelete[0] - const id = item.id + const id = getItemId(item) markChanging(id) - await ctx.deleteItem(item) - removeFromSelection(id) - unmarkChanging(id) + try { + await ctx.deleteItem(item) + removeFromSelection(id) + } finally { + unmarkChanging(id) + } return } @@ -369,14 +376,14 @@ async function confirmDelete() { itemsToDelete, async (item) => { await ctx.deleteItem(item) - removeFromSelection(item.id) + removeFromSelection(getItemId(item)) }, { onComplete: clearSelection }, ) } async function handleToggleEnabledById(id: string, _value: boolean) { - const item = ctx.items.value.find((i) => i.id === id) + const item = ctx.items.value.find((i) => getItemId(i) === id) if (!item) return markChanging(id) try { @@ -431,7 +438,7 @@ function handleUpdateById(id: string) { } function handleSwitchVersionById(id: string) { - const item = ctx.items.value.find((i) => i.id === id) + const item = ctx.items.value.find((i) => getItemId(i) === id) if (item) { ctx.switchVersion?.(item) } @@ -758,6 +765,7 @@ const confirmUnlinkModal = ref>() :bulk-total="bulkTotal" :bulk-waiting="bulkWaiting" :aria-label="formatMessage(commonMessages.selectionActionsLabel)" + :get-item-id="getItemId" @clear="clearSelection" @enable="bulkEnable" @disable="bulkDisable" diff --git a/packages/ui/src/layouts/shared/content-tab/providers/content-manager.ts b/packages/ui/src/layouts/shared/content-tab/providers/content-manager.ts index 01265d1ae..2de32de57 100644 --- a/packages/ui/src/layouts/shared/content-tab/providers/content-manager.ts +++ b/packages/ui/src/layouts/shared/content-tab/providers/content-manager.ts @@ -77,6 +77,9 @@ export interface ContentManagerContext { // Share support (optional — when undefined, share button becomes hidden entirely) shareItems?: (items: ContentItem[], format: 'names' | 'file-names' | 'urls' | 'markdown') => void + // Stable per-row identity. ContentItem.id can be a content hash, so it is not always unique. + getItemId?: (item: ContentItem) => string + // Bulk operation guard — set by layout, checked by providers to suppress refreshes isBulkOperating?: Ref diff --git a/packages/ui/src/layouts/wrapped/hosting/manage/content.vue b/packages/ui/src/layouts/wrapped/hosting/manage/content.vue index 707b33ab1..46f44fc4e 100644 --- a/packages/ui/src/layouts/wrapped/hosting/manage/content.vue +++ b/packages/ui/src/layouts/wrapped/hosting/manage/content.vue @@ -526,6 +526,10 @@ function getContentItemDisplayKey(item: ContentItem) { return item.project?.id ?? item.file_name ?? item.id } +function getContentItemId(item: ContentItem) { + return item.file_name ?? item.id +} + function mergeFragileContentItems(items: ContentItem[]) { const nextItems = new Map(items.map((item) => [getContentItemDisplayKey(item), item])) const mergedItems = displayedContentItems.value.map((item) => { @@ -980,7 +984,7 @@ async function handleBulkUpdate(items: ContentItem[]) { } async function handleUpdateItem(id: string) { - const item = contentItems.value.find((i) => i.id === id) + const item = contentItems.value.find((i) => getContentItemId(i) === id) if (!item?.has_update || !item.project?.id || !item.version?.id) return updatingModpack.value = false @@ -1220,13 +1224,14 @@ provideContentManager({ openSettings: () => openServerSettings({ tabId: 'installation' }), switchVersion: handleSwitchVersion, getOverflowOptions, + getItemId: getContentItemId, mapToTableItem: (item) => { const projectType = item.project_type ?? type.value const addon = addonLookup.value.get(item.file_name) const hasModrinthProject = !!addon?.project_id || (!!item.installing && !!item.project?.id) const projectSlugOrId = item.project.slug ?? item.project.id return { - id: item.id, + id: getContentItemId(item), project: item.project, projectLink: hasModrinthProject ? `/${projectType}/${projectSlugOrId}` : undefined, version: item.version,