Media Assets Management Sidebar Tab Implementation #6112
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🎭 Playwright Test Results⏰ Completed at: 10/29/2025, 03:41:36 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 10/29/2025, 03:27:59 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
Contributor
There was a problem hiding this comment.
Can we put the composables in the platform/assets folder and then re-arrange so it's like:
useMediaAssets/
useAssetsApi.ts
useInternalFilesApi.ts
index.ts
then inside the index.ts file you can put:
import { isCloud } from '@/platform/distribution/types'
if (isCloud) { export { useAssetsApi as useMediaAssets } from './useAssetsApi' }
else { export { useInternalFilesApi as useMediaAssets } from './useInternalFilesApi' }Then we should also define an interface or abstract class
interface IAssetsProvider {
loading: ref<boolean>
error: ref<null | string>
fetchMediaList: () => UninifedType
}This way, no other parts of the codebase need to understand the difference in implementations for assets - everything is abstracted behind the UnifiedType and AssetsProvider concepts. This makes refactoring, reasoning about the code, and especially dealing with types considerably easier and more maintainable.
The additional benefit is that for OSS build, the entire useAssetsApi.ts file actually gets tree-shaken.
Merged
viva-jinyi
force-pushed
the
feature/media-asset-sidebar-tab
branch
from
172b3c3 to
be16171
Compare
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.31 MB (baseline 3.3 MB) • 🔴 +6 kBMain entry bundles and manifests
Status: 2 added / 2 removed Graph Workspace — 716 kB (baseline 715 kB) • 🔴 +335 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 8.15 kB (baseline 8.15 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 294 kB (baseline 294 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 12.3 kB (baseline 12.3 kB) • ⚪ 0 BReusable component library chunks
Status: 1 added / 1 removed Data & Services — 10 kB (baseline 10 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 2 added / 2 removed Utilities & Hooks — 1.07 kB (baseline 1.07 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 5.36 MB (baseline 5.36 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Status: 3 added / 3 removed Other — 2.55 MB (baseline 2.55 MB) • ⚪ 0 BBundles that do not match a named category
|
viva-jinyi
added a commit
that referenced
this pull request
Oct 20, 2025
- Move composables to platform/assets directory structure - Extract interface-based abstraction (IAssetsProvider) for cloud/internal implementations - Move constants to module scope to avoid re-initialization - Extract helper functions (truncateFilename, assetMappers) for reusability - Rename getMediaTypeFromFilename to return singular form (image/video/audio) - Add deprecated plural version for backward compatibility - Add comprehensive test coverage for new utility functions 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
viva-jinyi
added a commit
that referenced
this pull request
Oct 20, 2025
- Move composables to platform/assets directory structure - Extract interface-based abstraction (IAssetsProvider) for cloud/internal implementations - Move constants to module scope to avoid re-initialization - Extract helper functions (truncateFilename, assetMappers) for reusability - Rename getMediaTypeFromFilename to return singular form (image/video/audio) - Add deprecated plural version for backward compatibility - Add comprehensive test coverage for new utility functions 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
viva-jinyi
force-pushed
the
feature/media-asset-sidebar-tab
branch
from
f6b8dbb to
308f864
Compare
viva-jinyi
requested review from
a team,
KarryCharon,
Yorha4D,
arjansingh and
shinshin86
as code owners
dosubot
bot
added
the
size:XXL
viva-jinyi
added a commit
that referenced
this pull request
Oct 22, 2025
- Move composables to platform/assets directory structure - Extract interface-based abstraction (IAssetsProvider) for cloud/internal implementations - Move constants to module scope to avoid re-initialization - Extract helper functions (truncateFilename, assetMappers) for reusability - Rename getMediaTypeFromFilename to return singular form (image/video/audio) - Add deprecated plural version for backward compatibility - Add comprehensive test coverage for new utility functions 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
viva-jinyi
force-pushed
the
feature/media-asset-sidebar-tab
branch
from
bee62d3 to
61eed48
Compare
viva-jinyi
added a commit
that referenced
this pull request
Oct 22, 2025
- Move composables to platform/assets directory structure - Extract interface-based abstraction (IAssetsProvider) for cloud/internal implementations - Move constants to module scope to avoid re-initialization - Extract helper functions (truncateFilename, assetMappers) for reusability - Rename getMediaTypeFromFilename to return singular form (image/video/audio) - Add deprecated plural version for backward compatibility - Add comprehensive test coverage for new utility functions 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
viva-jinyi
force-pushed
the
feature/media-asset-sidebar-tab
branch
from
ac427e5 to
bd23ccc
Compare
viva-jinyi
added a commit
that referenced
this pull request
Oct 22, 2025
- Move composables to platform/assets directory structure - Extract interface-based abstraction (IAssetsProvider) for cloud/internal implementations - Move constants to module scope to avoid re-initialization - Extract helper functions (truncateFilename, assetMappers) for reusability - Rename getMediaTypeFromFilename to return singular form (image/video/audio) - Add deprecated plural version for backward compatibility - Add comprehensive test coverage for new utility functions 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
viva-jinyi
force-pushed
the
feature/media-asset-sidebar-tab
branch
from
97d6a6f to
6769687
Compare
viva-jinyi
added a commit
that referenced
this pull request
Oct 22, 2025
- Move composables to platform/assets directory structure - Extract interface-based abstraction (IAssetsProvider) for cloud/internal implementations - Move constants to module scope to avoid re-initialization - Extract helper functions (truncateFilename, assetMappers) for reusability - Rename getMediaTypeFromFilename to return singular form (image/video/audio) - Add deprecated plural version for backward compatibility - Add comprehensive test coverage for new utility functions 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
DrJKL
reviewed
Oct 22, 2025
Contributor
DrJKL
left a comment
DrJKL
left a comment
There was a problem hiding this comment.
Reviewed from the bottom up 🙃
src/platform/assets/composables/useMediaAssets/useInternalFilesApi.ts
Outdated
Show resolved
Hide resolved
src/platform/assets/composables/useMediaAssets/useInternalFilesApi.ts
Outdated
Show resolved
Hide resolved
src/platform/assets/composables/useMediaAssets/useInternalFilesApi.ts
Outdated
Show resolved
Hide resolved
christian-byrne
added
the
claude-review
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| }) | ||
| // Override the url getter to use asset.preview_url | ||
| Object.defineProperty(resultItem, 'url', { |
There was a problem hiding this comment.
[security] high Priority
Issue: Direct DOM manipulation vulnerability - Using object.defineProperty to dynamically override url getter creates potential XSS vector
Context: Overriding object getters at runtime can lead to unexpected behavior and security issues if the asset data is not sanitized
Suggestion: Use a computed property or safer mapping approach instead of defineProperty to avoid runtime property manipulation
arjansingh
reviewed
Oct 29, 2025
Contributor
arjansingh
left a comment
arjansingh
left a comment
There was a problem hiding this comment.
Generally, I think if you reduced the number of components you could use inject/emit a little less and simplify the code.
Only follow up I have is the assetStore, we should definitely bring that into the platform directory for assets.
| @@ -0,0 +1,133 @@ | |||
| import { useAsyncState } from '@vueuse/core' | |||
Contributor
There was a problem hiding this comment.
follow-up: This should go in /platform/assets
viva-jinyi
added a commit
that referenced
this pull request
Oct 29, 2025
- Move composables to platform/assets directory structure - Extract interface-based abstraction (IAssetsProvider) for cloud/internal implementations - Move constants to module scope to avoid re-initialization - Extract helper functions (truncateFilename, assetMappers) for reusability - Rename getMediaTypeFromFilename to return singular form (image/video/audio) - Add deprecated plural version for backward compatibility - Add comprehensive test coverage for new utility functions 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
viva-jinyi
added a commit
that referenced
this pull request
Oct 29, 2025
- Integrate AssetsStore with useNodeImageUpload - Automatically update input assets when uploading to input folder - Support drag & drop, paste, and file selection Solution 2 complete: Real-time assets sync with upload triggers Related: #6112 code review feedback applied
viva-jinyi
force-pushed
the
feature/media-asset-sidebar-tab
branch
from
208e522 to
5780728
Compare
arjansingh
approved these changes
Oct 29, 2025
- Implement new sidebar tab for managing imported/generated files - Add separate composables for internal and cloud environments - Display execution time from history API on generated outputs - Support gallery view with keyboard navigation - Auto-truncate long filenames in cloud environment - Add utility functions for media type detection - Enable feature only in development mode 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Move composables to platform/assets directory structure - Extract interface-based abstraction (IAssetsProvider) for cloud/internal implementations - Move constants to module scope to avoid re-initialization - Extract helper functions (truncateFilename, assetMappers) for reusability - Rename getMediaTypeFromFilename to return singular form (image/video/audio) - Add deprecated plural version for backward compatibility - Add comprehensive test coverage for new utility functions 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
…truncation - Add file format tags (PNG, JPG, etc.) for input directory assets - Truncate long filenames in input assets with originalFilename preservation - Show file format chip independently from duration chip - Fix conditional display logic for chips in MediaAssetCard - Apply consistent filename truncation (20 chars) across cloud assets 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add optional includePublic parameter (defaults to true) to getAssetsByTag - Exclude public assets for media assets in sidebar by passing false - Use URLSearchParams for cleaner query string construction 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Implement factory pattern with useAsyncState for composables - Add internationalization support for aria-labels - Remove deprecated functions and improve type safety - Simplify component logic with VueUse patterns - Add comprehensive edge case tests
## Summary - Extract sidebar template into reusable AssetSidebarTemplate component - Replace PrimeVue Tabs with TextButton for better visual consistency - Improve job detail view header layout with better spacing ## Changes - Created `AssetSidebarTemplate.vue` as a reusable template component - Replaced PrimeVue Tabs with TextButton components for tab navigation - Added i18n translation key for "Back to all assets" button - Improved spacing and layout in job detail view header - Maintained all existing functionality while cleaning up template structure ## Test Plan - [ ] Verify tab switching between Imported and Generated tabs works correctly - [ ] Test job detail view displays properly with Job ID and execution time - [ ] Confirm "Back to all assets" button returns to main view - [ ] Check that all existing media asset features remain functional - [ ] Verify UI consistency with other sidebar tabs [screen-capture.webm](https://github.com/user-attachments/assets/4ed192e1-a9f7-4fc1-a41e-f732741dd55d)
- Create AssetsStore following QueueStore pattern for history-based assets - Use useAsyncState for async state management (loading/error handling) - Support both cloud and local environments (via isCloud flag) - Auto-update history assets on status events in GraphView - Refactor useMediaAssets composables to use AssetsStore
- Remove deprecated getMediaKindFromFilename function (no usages found) - Define MediaType using const assertion pattern - Apply as const to extension arrays with type guards - Use type assertions for type-safe includes checks
- Integrate AssetsStore with useNodeImageUpload - Automatically update input assets when uploading to input folder - Support drag & drop, paste, and file selection Solution 2 complete: Real-time assets sync with upload triggers Related: #6112 code review feedback applied
- Integrate AssetsStore with WidgetSelectDropdown component - Auto-refresh input assets after file upload via dropdown widget - Ensures Assets sidebar stays in sync with uploaded files Part of reactive assets update implementation for better UX
- refactor(components): Use defineModel in TabList.vue for cleaner v-model implementation - refactor(components): Extract MediaTitle component to eliminate duplication across media bottom components - Consolidate title rendering logic from MediaImageBottom, MediaVideoBottom, MediaAudioBottom, Media3DBottom - Improve code reusability and maintainability - refactor(composables): Restructure useMediaAssets directory structure - Rename directory from 'useMediaAssets' to 'media' - Rename index.ts to useMediaAssets.ts to avoid index pattern - Update all import paths accordingly - refactor(utils): Simplify UUID utility exports - Remove isValidUuid wrapper function - Export validate directly from uuid library - Update tests to use validate instead of isValidUuid - test(formatUtil): Apply it.for pattern for better test readability - Convert repetitive test cases to data-driven tests using it.for
- Fix issue where viewing image changes when new outputs are added to the gallery - Add currentGalleryAssetId ref to track the opened asset ID - Update index to maintain same image when displayAssets changes - Clear stored ID when gallery closes
…ss promptId directly from metadata instead of encoding/parsing it in IDs
## Summary - Implement asset deletion for media assets - Add delete confirmation dialog - Support both cloud and internal asset deletion - Refresh assets list after successful deletion ## Changes - Add delete button to MediaAssetActions component - Implement deleteAsset method in useMediaAssetActions - Add confirmation dialog before deletion - Handle asset-deleted event to refresh the list - Refactor to use QueueStore.tasks for proper grouping ## Test plan - [x] Delete output assets in internal mode - [x] Delete input/output assets in cloud mode - [x] Verify confirmation dialog appears - [x] Check assets list refreshes after deletion - [x] Test folder view functionality [screen-capture (3).webm](https://github.com/user-attachments/assets/3306262e-627a-4db0-90c1-fd59ba3abf7c)
viva-jinyi
force-pushed
the
feature/media-asset-sidebar-tab
branch
from
5780728 to
b778cde
Compare
christian-byrne
added a commit
that referenced
this pull request
Oct 30, 2025
Add missing English translation keys that were added in recent releases (PR #6256, #6112, #6187) but not backported to rh-test: - Media asset management (delete, selection, job ID toast) - Asset browser aria labels - Sidebar labels (console, menu, assets, imported, generated) - File management strings (no files found messages) - Resize handle tooltips - Basic UI strings (edit/delete image, chart, file, etc.) These will be translated in the next commit.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📋 Overview
Implemented a new Media Assets sidebar tab in ComfyUI for managing user-uploaded input files and generated output files. This feature supports both local and cloud environments and is currently enabled only in development mode.
🎯 Key Features
1. Media Assets Sidebar Tab
2. Environment-Specific Implementation
useInternalMediaAssets: For local environment/filesAPI/historyAPIuseCloudMediaAssets: For cloud environmentvery_long_filename_here.png→very_long_...here.png)3. Execution Time Display
execution_startandexecution_successmessages4. Gallery Feature
5. Development Mode Only
import.meta.env.DEVcondition🛠️ Technical Changes
New Files Added
src/components/sidebar/tabs/AssetsSidebarTab.vue- Main sidebar tab componentsrc/composables/sidebarTabs/useAssetsSidebarTab.ts- Sidebar tab definitionsrc/composables/useInternalMediaAssets.ts- Local environment implementationsrc/composables/useCloudMediaAssets.ts- Cloud environment implementationpackages/design-system/src/icons/image-ai-edit.svg- Icon additionModified Files
src/stores/workspace/sidebarTabStore.ts- Added dev mode only tab display logicsrc/platform/assets/components/MediaAssetCard.vue- Added execution time display, zoom eventsrc/platform/assets/components/MediaImageTop.vue- Added image dimension detectionpackages/shared-frontend-utils/src/formatUtil.ts- Added media type determination utility functionssrc/locales/en/main.json- Added translation keysmedia_asset_OSS_cloud.webm