Media Assets Management Sidebar Tab Implementation #6112
+1,802
−200
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/24/2025, 05:58:39 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/24/2025, 05:43:24 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
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.3 MB (baseline 3.29 MB) • 🔴 +5.69 kB_Main entry bundles and manifests_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | ---------------------------------------- | ------- | ------- | ----------------------- | ---------------------- | ----------------------- | | **assets/index-CMkEzUg0.js** _(new)_ | — | 2.68 MB | 🔴 +2.68 MB | 🔴 +557 kB | 🔴 +422 kB | | ~~assets/index-Bn4mcbF8.js~~ _(removed)_ | 2.68 MB | — | 🟢 -2.68 MB | 🟢 -556 kB | 🟢 -421 kB | | ~~assets/index-DqZ3XyGx.js~~ _(removed)_ | 614 kB | — | 🟢 -614 kB | 🟢 -114 kB | 🟢 -90.2 kB | | **assets/index-hrrLJHxV.js** _(new)_ | — | 614 kB | 🔴 +614 kB | 🔴 +114 kB | 🔴 +90.1 kB |Status: 2 added / 2 removed Graph Workspace — 713 kB (baseline 712 kB) • 🔴 +335 B_Graph editor runtime, canvas, workflow orchestration_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | -------------------------------------------- | ------ | ------ | ---------------------- | ---------------------- | ---------------------- | | **assets/GraphView-bMJLL44c.js** _(new)_ | — | 713 kB | 🔴 +713 kB | 🔴 +140 kB | 🔴 +108 kB | | ~~assets/GraphView-zxChLZ3d.js~~ _(removed)_ | 712 kB | — | 🟢 -712 kB | 🟢 -140 kB | 🟢 -108 kB |Status: 1 added / 1 removed Views & Navigation — 8.15 kB (baseline 8.15 kB) • ⚪ 0 B_Top-level views, pages, and routed surfaces_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | ------------------------------------------------- | ------- | ------- | ----------------------- | ----------------------- | ----------------------- | | **assets/UserSelectView-CFaTo5_Q.js** _(new)_ | — | 8.15 kB | 🔴 +8.15 kB | 🔴 +2.46 kB | 🔴 +2.16 kB | | ~~assets/UserSelectView-DIulM0up.js~~ _(removed)_ | 8.15 kB | — | 🟢 -8.15 kB | 🟢 -2.47 kB | 🟢 -2.15 kB |Status: 1 added / 1 removed Panels & Settings — 294 kB (baseline 294 kB) • ⚪ 0 B_Configuration panels, inspectors, and settings screens_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | ---------------------------------------------------- | ------- | ------- | ----------------------- | ----------------------- | ----------------------- | | **assets/CreditsPanel-Bo18EkkI.js** _(new)_ | — | 22.1 kB | 🔴 +22.1 kB | 🔴 +5.28 kB | 🔴 +4.6 kB | | ~~assets/CreditsPanel-IqMyc7Du.js~~ _(removed)_ | 22.1 kB | — | 🟢 -22.1 kB | 🟢 -5.28 kB | 🟢 -4.6 kB | | **assets/KeybindingPanel-bD0vtzzz.js** _(new)_ | — | 15.2 kB | 🔴 +15.2 kB | 🔴 +3.76 kB | 🔴 +3.31 kB | | ~~assets/KeybindingPanel-iNFLHeIf.js~~ _(removed)_ | 15.2 kB | — | 🟢 -15.2 kB | 🟢 -3.76 kB | 🟢 -3.31 kB | | **assets/ExtensionPanel-DbxrYX85.js** _(new)_ | — | 12.1 kB | 🔴 +12.1 kB | 🔴 +2.83 kB | 🔴 +2.47 kB | | ~~assets/ExtensionPanel-OrjbJS0F.js~~ _(removed)_ | 12.1 kB | — | 🟢 -12.1 kB | 🟢 -2.83 kB | 🟢 -2.47 kB | | **assets/AboutPanel-CkwC-JI6.js** _(new)_ | — | 10.3 kB | 🔴 +10.3 kB | 🔴 +2.66 kB | 🔴 +2.33 kB | | ~~assets/AboutPanel-DZeSgxLV.js~~ _(removed)_ | 10.3 kB | — | 🟢 -10.3 kB | 🟢 -2.66 kB | 🟢 -2.35 kB | | ~~assets/ServerConfigPanel-BMgotDNq.js~~ _(removed)_ | 8.2 kB | — | 🟢 -8.2 kB | 🟢 -2.16 kB | 🟢 -1.89 kB | | **assets/ServerConfigPanel-C7BWke8u.js** _(new)_ | — | 8.2 kB | 🔴 +8.2 kB | 🔴 +2.16 kB | 🔴 +1.9 kB | | ~~assets/UserPanel-hw7WxpaD.js~~ _(removed)_ | 7.91 kB | — | 🟢 -7.91 kB | 🟢 -2.06 kB | 🟢 -1.79 kB | | **assets/UserPanel-jKu_xRdT.js** _(new)_ | — | 7.91 kB | 🔴 +7.91 kB | 🔴 +2.06 kB | 🔴 +1.8 kB | | assets/settings-B-df0dZe.js | 20.7 kB | 20.7 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-CI6OKvJn.js | 22.9 kB | 22.9 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-CXGVj_nD.js | 24.5 kB | 24.5 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-DfQ6dSJj.js | 31.6 kB | 31.6 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-DJ2QgDzm.js | 25.2 kB | 25.2 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-DRNLPMG6.js | 23.7 kB | 23.7 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-DVVycxDc.js | 19.9 kB | 19.9 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-G6Dybj1b.js | 24.1 kB | 24.1 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/settings-M6_GZccG.js | 26 kB | 26 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B |Status: 6 added / 6 removed UI Components — 12.3 kB (baseline 12.3 kB) • ⚪ 0 B_Reusable component library chunks_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | ----------------------------------------------------------------- | ------- | ------- | ----------------------- | ----------------------- | ----------------------- | | **assets/ComfyQueueButton-Cr1FF2Xt.js** _(new)_ | — | 11.1 kB | 🔴 +11.1 kB | 🔴 +2.76 kB | 🔴 +2.44 kB | | ~~assets/ComfyQueueButton-MMCxJeIn.js~~ _(removed)_ | 11.1 kB | — | 🟢 -11.1 kB | 🟢 -2.76 kB | 🟢 -2.44 kB | | assets/UserAvatar.vue_vue_type_script_setup_true_lang-C9bSkTC5.js | 1.12 kB | 1.12 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B |Status: 1 added / 1 removed Data & Services — 10 kB (baseline 10 kB) • ⚪ 0 B_Stores, services, APIs, and repositories_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | ---------------------------------------------------- | ------- | ------- | ----------------------- | ----------------------- | ----------------------- | | ~~assets/keybindingService-BuTi_yvU.js~~ _(removed)_ | 7.21 kB | — | 🟢 -7.21 kB | 🟢 -1.75 kB | 🟢 -1.51 kB | | **assets/keybindingService-CFOCX-xD.js** _(new)_ | — | 7.21 kB | 🔴 +7.21 kB | 🔴 +1.75 kB | 🔴 +1.5 kB | | ~~assets/serverConfigStore-CvX_HMTJ.js~~ _(removed)_ | 2.79 kB | — | 🟢 -2.79 kB | 🟢 -891 B | 🟢 -777 B | | **assets/serverConfigStore-DGtDMYK2.js** _(new)_ | — | 2.79 kB | 🔴 +2.79 kB | 🔴 +889 B | 🔴 +775 B |Status: 2 added / 2 removed Utilities & Hooks — 1.07 kB (baseline 1.07 kB) • ⚪ 0 B_Helpers, composables, and utility bundles_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | --------------------------- | ------- | ------- | ------------------ | ------------------ | ------------------ | | assets/mathUtil-CTARWQ-l.js | 1.07 kB | 1.07 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B |Vendor & Third-Party — 5.36 MB (baseline 5.36 MB) • ⚪ 0 B_External libraries and shared vendor chunks_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | ------------------------------------------------ | ------- | ------- | ----------------------- | ----------------------- | ----------------------- | | **assets/vendor-other-Cf3diXgO.js** _(new)_ | — | 3.22 MB | 🔴 +3.22 MB | 🔴 +685 kB | 🔴 +549 kB | | ~~assets/vendor-other-EuL9bHKm.js~~ _(removed)_ | 3.22 MB | — | 🟢 -3.22 MB | 🟢 -685 kB | 🟢 -549 kB | | **assets/vendor-tiptap-CfWvY-eC.js** _(new)_ | — | 232 kB | 🔴 +232 kB | 🔴 +45.7 kB | 🔴 +37.7 kB | | ~~assets/vendor-tiptap-DY0_3CMM.js~~ _(removed)_ | 232 kB | — | 🟢 -232 kB | 🟢 -45.7 kB | 🟢 -37.7 kB | | **assets/vendor-vue-D_foLepl.js** _(new)_ | — | 92.4 kB | 🔴 +92.4 kB | 🔴 +23.9 kB | 🔴 +20.8 kB | | ~~assets/vendor-vue-Di9L6lvm.js~~ _(removed)_ | 92.4 kB | — | 🟢 -92.4 kB | 🟢 -23.9 kB | 🟢 -20.8 kB | | assets/vendor-primevue-PESgPnbc.js | 517 B | 517 B | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/vendor-visualization-BEfdbjRw.js | 1.82 MB | 1.82 MB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B |Status: 3 added / 3 removed Other — 2.58 MB (baseline 2.58 MB) • ⚪ 0 B_Bundles that do not match a named category_ | File | Before | After | Δ Raw | Δ Gzip | Δ Brotli | | --------------------------- | ------- | ------- | ------------------ | ------------------ | ------------------ | | assets/commands-B2KZRBmX.js | 15.1 kB | 15.1 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-Bw-ckyga.js | 13.9 kB | 13.9 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-C_NmM85I.js | 13.8 kB | 13.8 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-CuozCW4W.js | 14 kB | 14 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-DGfVUJCR.js | 16.2 kB | 16.2 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-dOJNDogK.js | 14.5 kB | 14.5 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-DwiE551e.js | 14.7 kB | 14.7 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-Fw7mvqSy.js | 13.1 kB | 13.1 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/commands-FXnO1W4Q.js | 13.2 kB | 13.2 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-Bgu6_Hvd.js | 59.5 kB | 59.5 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-Bv0L0qvp.js | 93 kB | 93 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-C3Doz3n_.js | 67.6 kB | 67.6 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-C7eBl607.js | 70.7 kB | 70.7 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-CHiV9ds2.js | 76.4 kB | 76.4 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-CIc79Nts.js | 68.5 kB | 68.5 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-DK5LmuBm.js | 58.8 kB | 58.8 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-J1nit7cj.js | 66.3 kB | 66.3 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/main-W97XgvAQ.js | 80.4 kB | 80.4 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-BePSqkA4.js | 195 kB | 195 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-BfT7dJcF.js | 204 kB | 204 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-BiAtoiXc.js | 194 kB | 194 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-CDfbduPY.js | 219 kB | 219 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-CDurg_KW.js | 197 kB | 197 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-CE-vG3RG.js | 182 kB | 182 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-DAwVV156.js | 200 kB | 200 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-DexhCMEi.js | 233 kB | 233 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | | assets/nodeDefs-kTrYLFPK.js | 184 kB | 184 kB | ⚪ 0 B | ⚪ 0 B | ⚪ 0 B | |
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
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
…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
viva-jinyi
force-pushed
the
feature/media-asset-sidebar-tab
branch
from
21df448 to
4eac455
Compare
DrJKL
reviewed
Oct 24, 2025
Comment on lines
+10
to
+12
| const props = defineProps<{ | ||
| modelValue: string | ||
| }>() |
DrJKL
reviewed
Oct 24, 2025
There was a problem hiding this comment.
I don't usually like composables that aren't using refs or other composables.
Could this be a constant?
There was a problem hiding this comment.
I don't usually like composables that aren't using refs or other composables.
Can you explain reasoning to help others learn?
DrJKL
reviewed
Oct 24, 2025
| } | ||
| const handleOutputCountClick = () => { | ||
| emit('output-count-click') |
There was a problem hiding this comment.
Is this not being consumed yet?
DrJKL
reviewed
Oct 24, 2025
| :title="asset.name" | ||
| > | ||
| {{ fileName }} | ||
| {{ truncateFilename(fileName) }} |
There was a problem hiding this comment.
It feels like there's a lot of duplication across these, could they be joined into a single component?
DrJKL
reviewed
Oct 24, 2025
| }>() | ||
| const emit = defineEmits<{ | ||
| 'image-loaded': [width: number, height: number] |
There was a problem hiding this comment.
If it weren't in the litegraph section, I'd say you should use the Size type 🫤
DrJKL
reviewed
Oct 24, 2025
DrJKL
reviewed
Oct 24, 2025
Comment on lines
+127
to
+140
| return { | ||
| // States | ||
| inputAssets, | ||
| historyAssets, | ||
| inputLoading, | ||
| historyLoading, | ||
| inputError, | ||
| historyError, | ||
| isLoading, | ||
|
|
||
| // Actions | ||
| updateInputs, | ||
| updateHistory, | ||
| update |
There was a problem hiding this comment.
I like this store, just to check, all of these are used within this PR?
DrJKL
reviewed
Oct 24, 2025
Comment on lines
+28
to
+35
| /** | ||
| * Check if a string is a valid UUID (any version) | ||
| * @param str - The string to check | ||
| * @returns true if the string is a valid UUID | ||
| */ | ||
| export function isValidUuid(str: string): boolean { | ||
| return uuidValidate(str) | ||
| } |
There was a problem hiding this comment.
Could we just use the uuid method in place wherever this is being called?
DrJKL
reviewed
Oct 24, 2025
| * @param str - The string to extract UUID from | ||
| * @returns The extracted UUID or null if not found | ||
| */ | ||
| export function extractUuidFromString(str: string): string | null { |
There was a problem hiding this comment.
What strings do we have that start with a UUID but contain other text?
DrJKL
reviewed
Oct 24, 2025
|
|
||
| describe('getMediaTypeFromFilename', () => { | ||
| describe('image files', () => { | ||
| it('should identify image extensions correctly', () => { |
There was a problem hiding this comment.
Did I already recommend it.for?
DrJKL
approved these changes
Oct 24, 2025
1 task
1 task
I can reproduce it on this branch. Steps:
|
|
This looks nice, but it would be more useful if it had a search field. You could add ( to each image ), other information useful for the search:
This last parameter is useful because complex workflows create lots of images ( e.g. Control Nets, multiple samplers, upscalers ), but in the gallery, we may only want to look at the final image. I already asked for something similar for Queue sidebar, a while algo: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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