[QPOv2] Add stories for list view and general job card #7743
Conversation
📝 WalkthroughWalkthroughThis PR configures Storybook module aliases, introduces mock implementations for useJobActions and useJobList composables, adds a new story file for AssetsSidebarListView, and refactors AssetsListCard.stories.ts to use named Story exports with multiple asset state variants. Changes
✨ Finishing touches
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 |
dosubot
bot
added
the
size:L
🎭 Playwright Test Results⏰ Completed at: 12/23/2025, 10:19:01 PM 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: 12/23/2025, 10:10:20 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
There was a problem hiding this comment.
Pull request overview
This PR adds Storybook stories for the media assets sidebar tab to support prototyping for QPOv2. The changes include mock implementations of composables used by the stories and configuration updates to enable the mocks in Storybook.
- Adds mock implementations for
useJobListanduseJobActionscomposables - Creates comprehensive stories for
AssetsListCardcomponent covering different job states and asset types - Creates stories for
AssetsSidebarListViewcomponent demonstrating running jobs and generated assets - Updates Storybook and knip configuration to support the mocks
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/storybook/mocks/useJobList.ts |
Mock implementation of useJobList composable for storybook stories |
src/storybook/mocks/useJobActions.ts |
Mock implementation of useJobActions composable for storybook stories |
src/platform/assets/components/AssetsListCard.stories.ts |
Comprehensive stories for AssetsListCard component covering job states and asset types |
src/components/sidebar/tabs/AssetsSidebarListView.stories.ts |
Stories for AssetsSidebarListView component with mock data |
knip.config.ts |
Adds knip ignore tag for storybook-specific exports |
.storybook/main.ts |
Updates alias configuration to use mocks for storybook environment |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
.storybook/main.tsknip.config.tssrc/components/sidebar/tabs/AssetsSidebarListView.stories.tssrc/platform/assets/components/AssetsListCard.stories.tssrc/storybook/mocks/useJobActions.tssrc/storybook/mocks/useJobList.ts
🧰 Additional context used
📓 Path-based instructions (11)
src/**/*.{vue,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{vue,ts}: Leverage VueUse functions for performance-enhancing styles
Implement proper error handling
Use vue-i18n in composition API for any string literals. Place new translation entries in src/locales/en/main.json
Files:
src/storybook/mocks/useJobList.tssrc/platform/assets/components/AssetsListCard.stories.tssrc/storybook/mocks/useJobActions.tssrc/components/sidebar/tabs/AssetsSidebarListView.stories.ts
src/**/*.ts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.ts: Use es-toolkit for utility functions
Use TypeScript for type safety
Files:
src/storybook/mocks/useJobList.tssrc/platform/assets/components/AssetsListCard.stories.tssrc/storybook/mocks/useJobActions.tssrc/components/sidebar/tabs/AssetsSidebarListView.stories.ts
src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/*.{ts,tsx,vue}: Sanitize HTML with DOMPurify to prevent XSS attacks
Avoid using @ts-expect-error; use proper TypeScript types instead
Use es-toolkit for utility functions instead of other utility libraries
Implement proper TypeScript types throughout the codebase
Files:
src/storybook/mocks/useJobList.tssrc/platform/assets/components/AssetsListCard.stories.tssrc/storybook/mocks/useJobActions.tssrc/components/sidebar/tabs/AssetsSidebarListView.stories.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/storybook/mocks/useJobList.tssrc/platform/assets/components/AssetsListCard.stories.tssrc/storybook/mocks/useJobActions.tssrc/components/sidebar/tabs/AssetsSidebarListView.stories.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,vue}: Use TypeScript exclusively; do not write new JavaScript code
Use sorted and grouped imports organized by plugin/source
Enforce ESLint rules including Vue + TypeScript rules, disallow floating promises, disallow unused imports, and restrict i18n raw text in templates
Do not useanytype oras anytype assertions; fix the underlying type issue instead
Write code that is expressive and self-documenting; avoid redundant comments and clean as you go
Keep functions short and functional; minimize nesting and follow the arrow anti-pattern
Avoid mutable state; prefer immutability and assignment at point of declaration
Use function declarations instead of function expressions when possible
Use es-toolkit for utility functions
Implement proper error handling in code
Files:
src/storybook/mocks/useJobList.tssrc/platform/assets/components/AssetsListCard.stories.tssrc/storybook/mocks/useJobActions.tsknip.config.tssrc/components/sidebar/tabs/AssetsSidebarListView.stories.ts
**/**/use[A-Z]*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name composables using the pattern
useXyz.ts
Files:
src/storybook/mocks/useJobList.tssrc/storybook/mocks/useJobActions.ts
**/*.{ts,tsx,vue,js,jsx,json,css}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Prettier formatting with 2-space indentation, single quotes, no trailing semicolons, and 80-character line width
Files:
src/storybook/mocks/useJobList.tssrc/platform/assets/components/AssetsListCard.stories.tssrc/storybook/mocks/useJobActions.tsknip.config.tssrc/components/sidebar/tabs/AssetsSidebarListView.stories.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Minimize the surface area (exported values) of each module and composable
Files:
src/storybook/mocks/useJobList.tssrc/platform/assets/components/AssetsListCard.stories.tssrc/storybook/mocks/useJobActions.tsknip.config.tssrc/components/sidebar/tabs/AssetsSidebarListView.stories.ts
src/**/{composables,components}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Clean up subscriptions in state management to prevent memory leaks
Files:
src/platform/assets/components/AssetsListCard.stories.tssrc/components/sidebar/tabs/AssetsSidebarListView.stories.ts
src/**/{components,composables}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Use vue-i18n for ALL user-facing strings by adding them to
src/locales/en/main.json
Files:
src/platform/assets/components/AssetsListCard.stories.tssrc/components/sidebar/tabs/AssetsSidebarListView.stories.ts
src/components/**/*.{vue,ts,js}
📄 CodeRabbit inference engine (src/components/CLAUDE.md)
src/components/**/*.{vue,ts,js}: Use existing VueUse composables (such as useElementHover) instead of manually managing event listeners
Use useIntersectionObserver for visibility detection instead of custom scroll handlers
Use vue-i18n for ALL UI strings
Files:
src/components/sidebar/tabs/AssetsSidebarListView.stories.ts
🧠 Learnings (14)
📓 Common learnings
Learnt from: Myestery
Repo: Comfy-Org/ComfyUI_frontend PR: 7422
File: .github/workflows/pr-update-playwright-expectations.yaml:131-135
Timestamp: 2025-12-12T23:02:37.473Z
Learning: In the `.github/workflows/pr-update-playwright-expectations.yaml` workflow in the Comfy-Org/ComfyUI_frontend repository, the snapshot update process is intentionally scoped to only add and update snapshot images. Deletions of snapshot files are handled explicitly outside this workflow and should not be suggested as part of this automation.
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Mock external dependencies in tests
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Mock external dependencies in tests
Applied to files:
src/storybook/mocks/useJobList.tssrc/storybook/mocks/useJobActions.ts.storybook/main.ts
📚 Learning: 2025-11-24T19:48:09.318Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: .cursor/rules/unit-test.mdc:0-0
Timestamp: 2025-11-24T19:48:09.318Z
Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Mocks should be cleanly written and easy to understand, with reusable mocks where possible
Applied to files:
src/storybook/mocks/useJobList.ts
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to **/*.test.ts : For mocking in tests, leverage Vitest utilities; keep module mocks contained and avoid global mutable state within test files
Applied to files:
src/storybook/mocks/useJobList.tssrc/storybook/mocks/useJobActions.ts.storybook/main.ts
📚 Learning: 2025-12-09T03:39:54.501Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7169
File: src/platform/remote/comfyui/jobs/jobTypes.ts:1-107
Timestamp: 2025-12-09T03:39:54.501Z
Learning: In the ComfyUI_frontend project, Zod is on v3.x. Do not suggest Zod v4 standalone validators (z.uuid, z.ulid, z.cuid2, z.nanoid) until an upgrade to Zod 4 is performed. When reviewing TypeScript files (e.g., src/platform/remote/comfyui/jobs/jobTypes.ts) validate against Zod 3 capabilities and avoid introducing v4-specific features; flag any proposal to upgrade or incorporate v4-only validators and propose staying with compatible 3.x patterns.
Applied to files:
src/storybook/mocks/useJobList.tssrc/platform/assets/components/AssetsListCard.stories.tssrc/storybook/mocks/useJobActions.tsknip.config.tssrc/components/sidebar/tabs/AssetsSidebarListView.stories.ts
📚 Learning: 2025-12-13T11:03:11.264Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7416
File: src/stores/imagePreviewStore.ts:5-7
Timestamp: 2025-12-13T11:03:11.264Z
Learning: In the ComfyUI_frontend repository, lint rules require keeping 'import type' statements separate from non-type imports, even if importing from the same module. Do not suggest consolidating them into a single import statement. Ensure type imports remain on their own line (import type { ... } from 'module') and regular imports stay on separate lines.
Applied to files:
src/storybook/mocks/useJobList.tssrc/platform/assets/components/AssetsListCard.stories.tssrc/storybook/mocks/useJobActions.tsknip.config.tssrc/components/sidebar/tabs/AssetsSidebarListView.stories.ts
📚 Learning: 2025-12-17T00:40:09.635Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7537
File: src/components/ui/button/Button.stories.ts:45-55
Timestamp: 2025-12-17T00:40:09.635Z
Learning: Prefer pure function declarations over function expressions (e.g., use function foo() { ... } instead of const foo = () => { ... }) for pure functions in the repository. Function declarations are more functional-leaning, offer better hoisting clarity, and can improve readability and tooling consistency. Apply this guideline across TypeScript files in Comfy-Org/ComfyUI_frontend, including story and UI component code, except where a function expression is semantically required (e.g., callbacks, higher-order functions with closures).
Applied to files:
src/storybook/mocks/useJobList.tssrc/platform/assets/components/AssetsListCard.stories.tssrc/storybook/mocks/useJobActions.tsknip.config.tssrc/components/sidebar/tabs/AssetsSidebarListView.stories.ts
📚 Learning: 2025-12-30T22:22:33.836Z
Learnt from: kaili-yang
Repo: Comfy-Org/ComfyUI_frontend PR: 7805
File: src/composables/useCoreCommands.ts:439-439
Timestamp: 2025-12-30T22:22:33.836Z
Learning: When accessing reactive properties from Pinia stores in TypeScript files, avoid using .value on direct property access (e.g., useStore().isOverlayExpanded). Pinia auto-wraps refs when accessed directly, returning the primitive value. The .value accessor is only needed when destructuring store properties or when using storeToRefs().
Applied to files:
src/storybook/mocks/useJobList.tssrc/platform/assets/components/AssetsListCard.stories.tssrc/storybook/mocks/useJobActions.tsknip.config.tssrc/components/sidebar/tabs/AssetsSidebarListView.stories.ts
📚 Learning: 2025-12-11T12:25:15.470Z
Learnt from: christian-byrne
Repo: Comfy-Org/ComfyUI_frontend PR: 7358
File: src/components/dialog/content/signin/SignUpForm.vue:45-54
Timestamp: 2025-12-11T12:25:15.470Z
Learning: This repository uses CI automation to format code (pnpm format). Do not include manual formatting suggestions in code reviews for Comfy-Org/ComfyUI_frontend. If formatting issues are detected, rely on the CI formatter or re-run pnpm format. Focus reviews on correctness, readability, performance, accessibility, and maintainability rather than style formatting.
Applied to files:
src/storybook/mocks/useJobList.tssrc/platform/assets/components/AssetsListCard.stories.tssrc/storybook/mocks/useJobActions.tsknip.config.tssrc/components/sidebar/tabs/AssetsSidebarListView.stories.ts
📚 Learning: 2025-12-16T17:30:29.719Z
Learnt from: DrJKL
Repo: Comfy-Org/ComfyUI_frontend PR: 7559
File: .storybook/preview.ts:61-61
Timestamp: 2025-12-16T17:30:29.719Z
Learning: In .storybook/preview.ts for the Comfy-Org/ComfyUI_frontend repository, using `document.body.classList.add('[&_*]:!font-inter')` is the correct approach for applying the Inter font to all Storybook story elements. The simpler `font-inter` class alone does not work in this context. This runtime arbitrary variant pattern is valid and should not be flagged as an issue.
Applied to files:
src/platform/assets/components/AssetsListCard.stories.ts
📚 Learning: 2025-12-21T06:04:12.562Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-21T06:04:12.562Z
Learning: Applies to browser_tests/**/*.spec.ts : Use tags like `mobile` and `2x` in Playwright tests for relevant test variations; tags are respected by config
Applied to files:
knip.config.ts
📚 Learning: 2025-12-16T17:41:58.464Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T17:41:58.464Z
Learning: Prerequisites for repository setup: Node.js >= 24, Git repository, available ports for dev server and storybook
Applied to files:
knip.config.ts
📚 Learning: 2025-11-24T19:48:03.270Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: tests-ui/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:48:03.270Z
Learning: Applies to tests-ui/**/*.test.{js,ts,jsx,tsx} : Always prefer vitest mock functions over writing verbose manual mocks
Applied to files:
.storybook/main.ts
📚 Learning: 2025-11-24T19:47:22.909Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: browser_tests/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:22.909Z
Learning: Applies to browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx} : Check assets/ directory for test data when writing tests
Applied to files:
src/components/sidebar/tabs/AssetsSidebarListView.stories.ts
🧬 Code graph analysis (4)
src/storybook/mocks/useJobList.ts (2)
src/composables/queue/useJobList.ts (3)
JobGroup(49-53)JobTab(25-25)JobSortMode(28-28)src/stores/queueStore.ts (1)
TaskItemImpl(216-475)
src/platform/assets/components/AssetsListCard.stories.ts (2)
src/components/input/SingleSelect.stories.ts (1)
Story(40-40)src/utils/queueDisplay.ts (1)
iconForJobState(27-42)
src/storybook/mocks/useJobActions.ts (1)
src/composables/queue/useJobActions.ts (1)
JobAction(11-16)
src/components/sidebar/tabs/AssetsSidebarListView.stories.ts (5)
src/platform/assets/schemas/assetSchema.ts (1)
AssetItem(74-74)src/composables/queue/useJobActions.ts (1)
JobAction(11-16)src/utils/queueDisplay.ts (1)
iconForJobState(27-42)src/storybook/mocks/useJobList.ts (1)
setMockJobItems(42-44)src/storybook/mocks/useJobActions.ts (1)
setMockJobActions(8-10)
🔇 Additional comments (11)
knip.config.ts (1)
67-71: LGTM!The new
-knipIgnoreUnusedButUsedByStorybooktag follows the established naming convention and correctly uses the minus prefix for exclusion. This will properly suppress knip warnings for Storybook-only mock exports..storybook/main.ts (1)
72-97: LGTM!The alias array is correctly ordered with more specific paths before the generic
@alias, ensuring proper module resolution. The mock aliases will intercept imports touseJobListanduseJobActionscomposables in Storybook, redirecting them to mock implementations.src/components/sidebar/tabs/AssetsSidebarListView.stories.ts (3)
1-39: LGTM!Imports are properly organized with type imports separated from value imports. The
StoryArgstype provides good type safety for story arguments, and the meta configuration follows Storybook conventions. Function declarations are used appropriately per repository guidelines.
43-121: LGTM!Sample data is comprehensive, covering multiple job states (pending, initialization, running) and asset types (image, video, audio, 3D). Using
iconForJobStateutility ensures consistency with the actual application. ThecancelActiondefinition follows theJobActiontype structure correctly.
136-155: Consider cleaning up mock state between stories.The mock state is set in
setup()but never reset. If multiple stories run sequentially or a user navigates between stories, stale mock data could persist. Consider adding cleanup in abeforeEachor using Storybook'splayfunction lifecycle, or document that stories should set all required mock state explicitly.Since
setMockJobItemsandsetMockJobActionsreplace the entire state (not append), this may be acceptable if every story explicitly sets its required mocks. The current implementation does this correctly.⛔ Skipped due to learnings
Learnt from: CR Repo: Comfy-Org/ComfyUI_frontend PR: 0 File: .cursor/rules/unit-test.mdc:0-0 Timestamp: 2025-11-24T19:48:09.318Z Learning: Applies to test/**/*.{test,spec}.{js,ts,jsx,tsx} : Mocks should be cleanly written and easy to understand, with reusable mocks where possiblesrc/storybook/mocks/useJobList.ts (2)
3-9: Relative imports are intentionally used here.Using
../../composables/...and../../stores/...instead of@/aliases is correct. The@/composables/queue/useJobListalias in Storybook points to this mock file, so using the alias here would create a circular reference.
46-59: Mock API already matches the realuseJobListcomposable perfectly.The mock returns all properties that the real composable exports:
selectedJobTab,selectedWorkflowFilter,selectedSortMode,hasFailedJobs,allTasksSorted,filteredTasks,jobItems,groupedJobItems, andcurrentNodeName. No discrepancies exist between the two implementations.Likely an incorrect or invalid review comment.
src/storybook/mocks/useJobActions.ts (1)
1-10: LGTM!Clean implementation with proper type imports and relative paths to avoid alias circular references. The
setMockJobActionsfunction provides a straightforward way to inject mock actions per job ID.src/platform/assets/components/AssetsListCard.stories.ts (3)
4-16: LGTM!Good refactoring to use
iconForJobStateutility for consistent icon names across stories. ThecenteredDecoratorfunction follows the repository's preference for function declarations over function expressions.
33-113: Comprehensive story coverage.Excellent coverage of different component states:
- Job states: pending, initialization, running, failed
- Generated asset types: image, video, audio, 3D
Each story clearly demonstrates a specific use case with appropriate props. Using constants for preview URLs (
IMAGE_PREVIEW,VIDEO_PREVIEW) improves maintainability.
115-133: LGTM!The
renderRunningJobWithActionshelper demonstrates the#actionsslot usage with a cancel button. Using a dedicated render function keeps the story definition clean while showing the slot API.
| /** @knipIgnoreUnusedButUsedByStorybook */ | ||
| export function useJobActions() { | ||
| function getJobActions(job: JobListItem) { | ||
| return actionsByJobId.value[job.id] ?? [] | ||
| } | ||
|
|
||
| async function runJobAction() { | ||
| return undefined | ||
| } | ||
|
|
||
| return { | ||
| getJobActions, | ||
| runJobAction | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the real useJobActions composable and extract its return statement
find . -path "*/composables/queue/useJobActions.ts" -o -path "*/composables/*useJobActions.ts" | head -5Repository: Comfy-Org/ComfyUI_frontend
Length of output: 110
🏁 Script executed:
#!/bin/bash
# Extract the return statement and function signatures from real useJobActions
cat -n src/composables/queue/useJobActions.ts | grep -A 100 "export function useJobActions"Repository: Comfy-Org/ComfyUI_frontend
Length of output: 1377
🏁 Script executed:
#!/bin/bash
# Read the complete mock file to understand full context
cat -n src/storybook/mocks/useJobActions.tsRepository: Comfy-Org/ComfyUI_frontend
Length of output: 877
Fix runJobAction signature to match the real composable interface.
The mock's runJobAction does not match the real composable. The real function accepts (action: JobAction, job: JobListItem) parameters and performs actual job cancellation logic, but the mock accepts no parameters and returns undefined. Additionally, getJobActions uses a different lookup strategy (job.id) instead of the real implementation's logic based on job.state and job.showClear. The mock should either properly emulate the real behavior or document that it's a simplified stub unsuitable for integration testing.
| '-knipIgnoreUnusedButUsedByCustomNodes', | ||
| '-knipIgnoreUnusedButUsedByVueNodesBranch' | ||
| '-knipIgnoreUnusedButUsedByVueNodesBranch', | ||
| '-knipIgnoreUnusedButUsedByStorybook' |
There was a problem hiding this comment.
nit: https://knip.dev/reference/plugins/storybook
The knip mocks should be accessed via the entry point for storybook, should we explore why that is not happening automatically?
Add stories for the media assets sidebar tab for easier prototyping.
Includes mocks for storybook.
Because some functions in the mocks are only used in the storybook main.ts resolve, knip flags them as unused because it doesn't check that path. So knipIgnoreUnusedButUsedByStorybook was added.
Part of the QPO v2 iteration, figma design can be found here. This will be implemented in a series of stacked PRs that can be reviewed and merged individually.
main <-- #7729, #7731, #7737, #7743, #7745
┆Issue is synchronized with this Notion page by Unito