Skip to content

Render actual images as thumbnails using blob URLs #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zudsniper wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Render actual images as thumbnails using blob URLs #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions packages/dashboard/src/appUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -521,3 +521,20 @@ export const getThumbnailUrl = (file, bucket) => {
disposition: "inline",
});
};

export const getThumbnailBlobUrl = async (file, bucket) => {
try {
const response = await api.get(`/buckets/${bucket}/${encode(file.key)}`, {
responseType: "arraybuffer",
});

const mimeType =
file.httpMetadata?.contentType ||
inferMimeTypeFromFilename(file.name);
const blob = new Blob([response.data], { type: mimeType });
return URL.createObjectURL(blob);
} catch (error) {
console.error("Failed to load thumbnail:", error);
return null;
}
};
Comment on lines +525 to +540
Copy link

@coderabbitai coderabbitai bot Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Critical: Missing cache-busting parameters creates stale thumbnail issue.

The new getThumbnailBlobUrl function doesn't include the disposition or cache version parameters that getThumbnailUrl uses (lines 515-523). This means:

  • Thumbnails may be served from browser cache even after file replacement
  • The cache version in file.customMetadata["cache-version"] is completely ignored
  • Users won't see updated thumbnails after replacing files

This is especially problematic since the codebase has a whole "Refresh Cache" feature (see handleRefreshCache, handleBulkRefreshCache in FileBrowser.vue) that updates cache versions—but those updates won't affect blob URL thumbnails at all!

Apply this diff to include cache-busting parameters:

 export const getThumbnailBlobUrl = async (file, bucket) => {
 	try {
+		const cacheVersion = file.customMetadata?.["cache-version"] || Date.now();
+		const params = new URLSearchParams({
+			v: cacheVersion.toString(),
+			disposition: "inline"
+		});
+		
-		const response = await api.get(`/buckets/${bucket}/${encode(file.key)}`, {
+		const response = await api.get(`/buckets/${bucket}/${encode(file.key)}?${params.toString()}`, {
 			responseType: "arraybuffer",
 		});

 		const mimeType =
 			file.httpMetadata?.contentType ||
 			inferMimeTypeFromFilename(file.name);
 		const blob = new Blob([response.data], { type: mimeType });
 		return URL.createObjectURL(blob);
 	} catch (error) {
 		console.error("Failed to load thumbnail:", error);
 		return null;
 	}
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const getThumbnailBlobUrl = async (file, bucket) => {
try {
const response = await api.get(`/buckets/${bucket}/${encode(file.key)}`, {
responseType: "arraybuffer",
});
const mimeType =
file.httpMetadata?.contentType ||
inferMimeTypeFromFilename(file.name);
const blob = new Blob([response.data], { type: mimeType });
return URL.createObjectURL(blob);
} catch (error) {
console.error("Failed to load thumbnail:", error);
return null;
}
};
export const getThumbnailBlobUrl = async (file, bucket) => {
try {
const cacheVersion = file.customMetadata?.["cache-version"] || Date.now();
const params = new URLSearchParams({
v: cacheVersion.toString(),
disposition: "inline"
});
const response = await api.get(`/buckets/${bucket}/${encode(file.key)}?${params.toString()}`, {
responseType: "arraybuffer",
});
const mimeType =
file.httpMetadata?.contentType ||
inferMimeTypeFromFilename(file.name);
const blob = new Blob([response.data], { type: mimeType });
return URL.createObjectURL(blob);
} catch (error) {
console.error("Failed to load thumbnail:", error);
return null;
}
};
🤖 Prompt for AI Agents 
In packages/dashboard/src/appUtils.js around lines 525 to 540,
getThumbnailBlobUrl is missing the cache-busting query params used by
getThumbnailUrl so thumbnails can be served from cached responses; modify the
api.get call to include the same query parameters (e.g. disposition=inline and
cache-version equal to file.customMetadata["cache-version"] or a sensible
default) when building the request URL/params (ensuring you still URL-encode
file.key), so the returned arraybuffer reflects the current cache-version and
avoids stale browser cache; keep the existing mime type inference and Blob
creation logic and return the object URL as before.

46 changes: 42 additions & 4 deletions packages/dashboard/src/components/files/FileBrowser.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,14 @@
<!-- Thumbnail or icon -->
<div class="file-card-preview">
<img
v-if="item.type === 'file' && isMediaFile(item.name)"
:src="getThumbnailUrl(item, bucket)"
v-if="item.type === 'file' && isMediaFile(item.name) && getThumbnailUrlForItem(item)"
:src="getThumbnailUrlForItem(item)"
:alt="item.name"
class="file-thumbnail-img"
loading="lazy"
@error="handleImageError($event, item)"
@load="handleImageLoad($event, item)"
@mouseenter="loadThumbnailBlobUrl(item)"
/>
<q-icon
v-else
Expand Down Expand Up @@ -249,13 +250,14 @@
<div class="file-list-cell file-list-name">
<div class="file-list-name-content">
<img
v-if="item.type === 'file' && isMediaFile(item.name) && showThumbnailsInList"
:src="getThumbnailUrl(item, bucket)"
v-if="item.type === 'file' && isMediaFile(item.name) && showThumbnailsInList && getThumbnailUrlForItem(item)"
:src="getThumbnailUrlForItem(item)"
:alt="item.name"
class="file-list-thumbnail"
loading="lazy"
@error="handleImageError($event, item)"
@load="handleImageLoad($event, item)"
@mouseenter="loadThumbnailBlobUrl(item)"
/>
<q-icon
v-else
Expand Down Expand Up @@ -556,6 +558,7 @@ import {
decode,
encode,
getThumbnailUrl,
getThumbnailBlobUrl,
Copy link

@coderabbitai coderabbitai bot Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Pipeline failure: Import sorting required.

The linter is complaining that imports aren't sorted. You added getThumbnailBlobUrl to line 561, but the imports need to be organized.

Run the safe fix suggested by the linter to organize imports, or manually ensure alphabetical ordering.

🤖 Prompt for AI Agents 
In packages/dashboard/src/components/files/FileBrowser.vue around line 561, the
import list is not alphabetically sorted after adding getThumbnailBlobUrl; run
the project's import-sort/linter safe-fix (e.g., eslint --fix or the
import-sorting tool configured) or manually reorder the import entries so they
follow the project's alphabetical ordering rules (place getThumbnailBlobUrl in
the correct position among other imports) and save to clear the import-sorting
lint error.

isMediaFile,
ROOT_FOLDER,
} from "../../appUtils";
Expand Down Expand Up @@ -618,6 +621,7 @@ export default defineComponent({
const touchTimer = ref(null);
const touchStartTime = ref(0);
const selectAll = ref(false);
const thumbnailBlobUrls = ref({}); // Map of file key to blob URL

// Computed
const selectedFolder = computed(() => {
Expand Down Expand Up @@ -684,12 +688,22 @@ export default defineComponent({
// Methods
const fetchFiles = async () => {
loading.value = true;
// Clear old thumbnail URLs when changing folders
thumbnailBlobUrls.value = {};
try {
items.value = await apiHandler.fetchFile(
props.bucket,
selectedFolder.value,
"/",
);
// Start loading thumbnails for media files in background
const mediaFiles = items.value.filter(
(item) => item.type === "file" && isMediaFile(item.name),
);
// Load first 5 thumbnails immediately, rest on demand
for (let i = 0; i < Math.min(5, mediaFiles.length); i++) {
loadThumbnailBlobUrl(mediaFiles[i]);
}
} catch (error) {
console.error("Failed to fetch files:", error);
$q.notify({
Expand Down Expand Up @@ -964,6 +978,21 @@ export default defineComponent({
return "Folder";
};

const getThumbnailUrlForItem = (item) => {
// Return blob URL if available, otherwise return empty string to show fallback
return thumbnailBlobUrls.value[item.key] || "";
};

const loadThumbnailBlobUrl = async (item) => {
if (thumbnailBlobUrls.value[item.key]) {
return; // Already loaded
}
const blobUrl = await getThumbnailBlobUrl(item, props.bucket);
if (blobUrl) {
thumbnailBlobUrls.value[item.key] = blobUrl;
}
};

const showContextMenu = (_event, item) => {
currentItem.value = item;
if (!isSelected(item)) {
Expand Down Expand Up @@ -1466,6 +1495,12 @@ export default defineComponent({
onBeforeUnmount(() => {
document.removeEventListener("keydown", handleKeyDown);
bus?.off("fetchFiles", fetchFiles);
// Clean up blob URLs to prevent memory leaks
Object.values(thumbnailBlobUrls.value).forEach((blobUrl) => {
if (blobUrl && blobUrl.startsWith("blob:")) {
URL.revokeObjectURL(blobUrl);
}
});
});
Comment on lines +1498 to 1504
Copy link

@coderabbitai coderabbitai bot Nov 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Cleanup logic is correct but incomplete.

You're properly revoking blob URLs on component unmount (lines 1498-1504), which is essential for memory management as per the Web Blob API docs. HOWEVER, you're not revoking the old blob URLs when clearing the map at line 692 during folder navigation. This means:

  1. User opens folder A → creates 5 blob URLs
  2. User navigates to folder B → line 692 clears the map BUT doesn't revoke the URLs
  3. Those 5 blob URLs from folder A are now orphaned in memory
  4. Repeat 10 times across folders → 50+ blob URLs leaked

Based on learnings about blob URL lifecycle management, this will accumulate memory until the page is closed or refreshed.

See the earlier comment on lines 689-716 for the fix—you need to revoke before clearing the map.

🧰 Tools
🪛 GitHub Actions: Build

[error] 1500-1502: lint/complexity/useOptionalChain: Change to an optional chain.

🤖 Prompt for AI Agents 
In packages/dashboard/src/components/files/FileBrowser.vue around lines 692 and
1498-1504, the cleanup on unmount revokes blob URLs but the map-clear at ~line
692 does not — causing orphaned blob URLs when navigating folders. Before
clearing or replacing thumbnailBlobUrls at line 692, iterate its values and call
URL.revokeObjectURL on any non-empty value that startsWith("blob:"), then clear
the map; keep the existing unmount cleanup as-is. Ensure this
revoke-before-clear behavior is applied wherever the map is reset during
navigation.

⚠️ Potential issue | 🟡 Minor

Pipeline failure: Use optional chain instead of manual checks.

The linter is complaining about lines 1500-1502. You're manually checking blobUrl && blobUrl.startsWith("blob:") when you could use optional chaining.

Apply this diff:

 			// Clean up blob URLs to prevent memory leaks
 			Object.values(thumbnailBlobUrls.value).forEach((blobUrl) => {
-				if (blobUrl && blobUrl.startsWith("blob:")) {
+				if (blobUrl?.startsWith("blob:")) {
 					URL.revokeObjectURL(blobUrl);
 				}
 			});
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Clean up blob URLs to prevent memory leaks
Object.values(thumbnailBlobUrls.value).forEach((blobUrl) => {
if (blobUrl && blobUrl.startsWith("blob:")) {
URL.revokeObjectURL(blobUrl);
}
});
});
// Clean up blob URLs to prevent memory leaks
Object.values(thumbnailBlobUrls.value).forEach((blobUrl) => {
if (blobUrl?.startsWith("blob:")) {
URL.revokeObjectURL(blobUrl);
}
});
🧰 Tools
🪛 GitHub Actions: Build

[error] 1500-1502: lint/complexity/useOptionalChain: Change to an optional chain.

🤖 Prompt for AI Agents 
In packages/dashboard/src/components/files/FileBrowser.vue around lines 1498 to
1504, the cleanup loop manually checks blobUrl with "blobUrl &&
blobUrl.startsWith('blob:')"; replace that manual null/undefined check with
optional chaining so the condition becomes blobUrl?.startsWith('blob:'), keeping
the URL.revokeObjectURL(blobUrl) call unchanged and preserving the same
behavior.


// Watch for folder changes
Expand Down Expand Up @@ -1499,6 +1534,7 @@ export default defineComponent({
currentItem,
showThumbnailsInList,
selectAll,
thumbnailBlobUrls,

// Computed
selectedFolder,
Expand Down Expand Up @@ -1530,6 +1566,8 @@ export default defineComponent({
handleImageError,
handleImageLoad,
getItemCount,
getThumbnailUrlForItem,
loadThumbnailBlobUrl,
showContextMenu,
handleDragStart,
handleDragOver,
Expand Down
Loading