[refactor] Extract dialog composables and add showSmallDialog helper #6846
Conversation
viva-jinyi
requested review from
christian-byrne and
ltdrdata
as code owners
dosubot
bot
added
the
size:L
📝 WalkthroughWalkthroughReplaces specific dialog functions with a generic showSmallDialog, adds two dialog composables (useNodeConflictDialog, useMissingNodesDialog), updates components/scripts to call the new composables, and adjusts NodeConflict dialog UI and tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Component as Component (e.g., PackInstallButton)
participant Composable as useNodeConflictDialog / useMissingNodesDialog
participant DialogService as dialogService.showSmallDialog
participant DialogStore as dialogStore
Component->>Composable: call show(...) (aliased showNodeConflictDialog or show)
Note right of Composable: assemble DIALOG_KEY, header/footer/content components and props
Composable->>DialogService: showSmallDialog({ key, component, headerComponent, footerComponent, props, dialogComponentProps })
DialogService->>DialogStore: showDialog({ key, ...mergedOptions })
DialogStore-->>DialogService: return DialogInstance
DialogService-->>Composable: return dialog handle / promise
Composable-->>Component: resolved dialog handle
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 11/27/2025, 01:06:36 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 11/27/2025, 01:14:58 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
viva-jinyi
changed the title
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.18 MB (baseline 3.18 MB) • 🔴 +832 BMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 948 kB (baseline 948 kB) • 🔴 +40 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 6.54 kB (baseline 6.54 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 298 kB (baseline 298 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 139 kB (baseline 139 kB) • ⚪ 0 BReusable component library chunks
Status: 9 added / 9 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 3 added / 3 removed Utilities & Hooks — 2.94 kB (baseline 2.94 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 1 added / 1 removed Vendor & Third-Party — 8.56 MB (baseline 8.56 MB) • 🟢 -3 BExternal libraries and shared vendor chunks
Status: 5 added / 5 removed Other — 3.84 MB (baseline 3.84 MB) • ⚪ 0 BBundles that do not match a named category
Status: 23 added / 23 removed |
![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
🧹 Nitpick comments (1)
src/services/dialogService.ts (1)
419-456: LGTM! Well-designed generic dialog helper.The
showSmallDialogfunction provides a clean, reusable pattern for small dialogs with consistent styling. The generic type parameter ensures type safety for component props, and the merge logic appropriately combines default styling with custom overrides.Optional: Add explicit return type annotation
For improved code clarity and type checking, consider adding an explicit return type:
- function showSmallDialog<T extends Component>(options: { + function showSmallDialog<T extends Component>(options: { key: string component: T headerComponent?: Component footerComponent?: Component props?: ComponentProps<T> footerProps?: Record<string, unknown> dialogComponentProps?: DialogComponentProps - }) { + }): ReturnType<typeof dialogStore.showDialog> {Optional: Consider consistent footer props typing
The
footerPropsparameter usesRecord<string, unknown>whilepropsuses the more specificComponentProps<T>. If the footer component type is known in some contexts, you might consider making it more specific, though the current approach is acceptable if footer components vary widely.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/components/sidebar/SidebarHelpCenterIcon.vue(2 hunks)src/composables/useMissingNodesDialog.ts(1 hunks)src/composables/useNodeConflictDialog.ts(1 hunks)src/scripts/app.ts(2 hunks)src/services/dialogService.ts(2 hunks)src/workbench/extensions/manager/components/manager/NodeConflictDialogContent.vue(6 hunks)src/workbench/extensions/manager/components/manager/NodeConflictHeader.vue(1 hunks)src/workbench/extensions/manager/components/manager/button/PackEnableToggle.vue(2 hunks)src/workbench/extensions/manager/components/manager/button/PackInstallButton.vue(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/scripts/app.ts (1)
src/composables/useMissingNodesDialog.ts (1)
useMissingNodesDialog(10-29)
src/composables/useNodeConflictDialog.ts (2)
src/stores/dialogStore.ts (1)
DialogComponentProps(36-37)src/workbench/extensions/manager/types/conflictDetectionTypes.ts (1)
ConflictDetectionResult(55-61)
src/services/dialogService.ts (1)
src/stores/dialogStore.ts (1)
DialogComponentProps(36-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: collect
- GitHub Check: test
🔇 Additional comments (14)
src/workbench/extensions/manager/components/manager/NodeConflictHeader.vue (1)
2-7: LGTM! Visual refinements align with the new dialog system.The styling updates (padding adjustments, icon library migration to Lucide, and text size reduction) provide a more consistent visual presentation for the conflict dialog header.
src/scripts/app.ts (2)
45-45: LGTM! Clean import of the new composable.
1020-1024: LGTM! Successful migration to the new composable pattern.The refactor from
useDialogService().showLoadWorkflowWarning()touseMissingNodesDialog().show()aligns with the PR's goal of extracting dialog logic into dedicated composables.src/workbench/extensions/manager/components/manager/button/PackInstallButton.vue (2)
29-29: LGTM! Import updated to use the new composable.
60-60: LGTM! Clean migration to the composable pattern.The aliasing of
showtoshowNodeConflictDialogmaintains backward compatibility with the existing codebase while adopting the new dialog composable.src/components/sidebar/SidebarHelpCenterIcon.vue (2)
67-67: LGTM! Import updated to use the new composable.
87-87: LGTM! Consistent migration to the composable pattern.The same refactoring approach (aliasing
showtoshowNodeConflictDialog) is applied consistently across multiple components.src/workbench/extensions/manager/components/manager/NodeConflictDialogContent.vue (3)
16-18: LGTM! Improved conditional rendering and styling consistency.The addition of
v-if="importFailedConflicts.length > 0"prevents rendering empty panels, and the background color standardization tobg-secondary-backgroundimproves visual consistency.
53-53: LGTM! Hover styles migrated to Tailwind classes.The migration from scoped CSS (
.conflict-list-item:hover) to inline Tailwind classes (hover:bg-alpha-azure-600-30) is a good practice that improves maintainability and eliminates the need for a separate style block.Also applies to: 101-101, 148-148
64-66: LGTM! Conditional rendering improves UX.Adding
v-if="allConflictDetails.length > 0"ensures the Conflicts panel only renders when there are actual conflicts to display.src/workbench/extensions/manager/components/manager/button/PackEnableToggle.vue (2)
37-37: LGTM! Import updated to use the new composable.
55-55: LGTM! Consistent migration to the composable pattern.The migration follows the same pattern established in other components, maintaining consistency across the codebase.
src/composables/useMissingNodesDialog.ts (1)
1-29: LGTM! Well-structured composable following Vue best practices.The composable properly encapsulates the missing nodes dialog logic with:
- Clear separation of concerns
- Proper TypeScript typing using
ComponentProps- Consistent API surface (
showandhidemethods)- Alignment with the broader dialog refactoring pattern
src/services/dialogService.ts (1)
519-520: LGTM! Clean public API surface.The export of
showSmallDialogandshowLayoutDialogprovides a focused, composable API. The refactoring successfully replaces specific dialog methods with a generic pattern that can be consumed by dedicated composables.
| import NodeConflictDialogContent from '@/workbench/extensions/manager/components/manager/NodeConflictDialogContent.vue' | ||
| import NodeConflictFooter from '@/workbench/extensions/manager/components/manager/NodeConflictFooter.vue' | ||
| import NodeConflictHeader from '@/workbench/extensions/manager/components/manager/NodeConflictHeader.vue' | ||
| import { useDialogService } from '@/services/dialogService' | ||
| import { useDialogStore } from '@/stores/dialogStore' | ||
| import type { DialogComponentProps } from '@/stores/dialogStore' | ||
| import type { ConflictDetectionResult } from '@/workbench/extensions/manager/types/conflictDetectionTypes' | ||
|
|
||
| const DIALOG_KEY = 'global-node-conflict' | ||
|
|
||
| export const useNodeConflictDialog = () => { | ||
| const dialogService = useDialogService() | ||
| const dialogStore = useDialogStore() | ||
|
|
||
| function hide() { | ||
| dialogStore.closeDialog({ key: DIALOG_KEY }) | ||
| } | ||
|
|
||
| function show( | ||
| options: { | ||
| showAfterWhatsNew?: boolean | ||
| conflictedPackages?: ConflictDetectionResult[] | ||
| dialogComponentProps?: DialogComponentProps | ||
| buttonText?: string | ||
| onButtonClick?: () => void | ||
| } = {} | ||
| ) { | ||
| const { buttonText, onButtonClick, showAfterWhatsNew, conflictedPackages } = | ||
| options | ||
|
|
||
| return dialogService.showSmallDialog({ | ||
| key: DIALOG_KEY, | ||
| headerComponent: NodeConflictHeader, | ||
| footerComponent: NodeConflictFooter, | ||
| component: NodeConflictDialogContent, | ||
| props: { | ||
| showAfterWhatsNew, | ||
| conflictedPackages | ||
| }, | ||
| footerProps: { | ||
| buttonText, | ||
| onButtonClick | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| return { show, hide } | ||
| } |
There was a problem hiding this comment.
LGTM with one concern! Well-structured composable with flexible options.
The composable follows Vue best practices and provides a clean API for displaying node conflict dialogs. However, there's a potential issue:
The dialogComponentProps from the options parameter (line 23) is destructured (line 28) but not passed to showSmallDialog() (lines 31-44).
Apply this diff to pass through dialog component props:
return dialogService.showSmallDialog({
key: DIALOG_KEY,
headerComponent: NodeConflictHeader,
footerComponent: NodeConflictFooter,
component: NodeConflictDialogContent,
props: {
showAfterWhatsNew,
conflictedPackages
},
footerProps: {
buttonText,
onButtonClick
- }
+ },
+ dialogComponentProps
})Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/composables/useNodeConflictDialog.ts around lines 1 to 48, the options
parameter accepts dialogComponentProps but that value is destructured and never
forwarded into dialogService.showSmallDialog; update the call to showSmallDialog
to merge or pass dialogComponentProps into the props object (e.g., spread
dialogComponentProps into props or assign it to a specific prop key) so the
dialog receives those component props, ensuring no other props are overwritten
when merging.
viva-jinyi
force-pushed
the
feature/small-dialog
branch
from
36ed67e to
8011d86
Compare
viva-jinyi
force-pushed
the
feature/small-dialog
branch
from
8011d86 to
7fc434d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/composables/useNodeConflictDialog.ts (1)
23-23:dialogComponentPropsis destructured but not passed toshowSmallDialog.The
dialogComponentPropsoption is accepted in the function signature (line 23) but never forwarded toshowSmallDialog(). This breaks callers likeSidebarHelpCenterIcon.vuethat rely ondialogComponentProps.onClosefor themarkConflictsAsSeen()callback.Apply this diff to pass through dialog component props:
return dialogService.showSmallDialog({ key: DIALOG_KEY, headerComponent: NodeConflictHeader, footerComponent: NodeConflictFooter, component: NodeConflictDialogContent, props: { showAfterWhatsNew, conflictedPackages }, footerProps: { buttonText, onButtonClick - } + }, + dialogComponentProps })Also applies to: 31-44
🧹 Nitpick comments (1)
src/workbench/extensions/manager/components/manager/button/PackEnableToggle.vue (1)
129-139: Consider resettingisLoadingin afinallyblock for robustnessIf
handleEnable()orhandleDisable()throws,isLoadingstaystrueand the toggle effectively “locks up” until reload. A small refactor would make this more robust:-const handleToggle = async (enable: boolean) => { - if (isLoading.value) return - - isLoading.value = true - if (enable) { - await handleEnable() - } else { - await handleDisable() - } - isLoading.value = false -} +const handleToggle = async (enable: boolean) => { + if (isLoading.value) return + + isLoading.value = true + try { + if (enable) { + await handleEnable() + } else { + await handleDisable() + } + } finally { + isLoading.value = false + } +}You can optionally add error reporting inside the
catch/finallypath later to meet the “proper error handling” guideline.
| <div | ||
| v-if="importFailedConflicts.length > 0" | ||
| class="flex min-h-8 w-full flex-col rounded-lg bg-base-background" | ||
| class="flex min-h-8 w-full flex-col rounded-lg bg-secondary-background" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read the NodeConflictDialogContent.vue file to understand its structure
cat -n src/workbench/extensions/manager/components/manager/NodeConflictDialogContent.vueRepository: Comfy-Org/ComfyUI_frontend
Length of output: 9603
Extension List Wrapper should conditionally render when data is empty, for consistency.
The component has three collapsible list sections with identical structure. The "Import Failed List Wrapper" and "Conflict List Wrapper" both use v-if to hide when their respective data arrays are empty (lines 16, 64), but the "Extension List Wrapper" (lines 111-112) renders unconditionally regardless of conflictData.length. This creates visual inconsistency—when conflictData.length === 0, the header displays "0 Extensions At Risk" with an empty expandable list, unlike the other sections which disappear entirely.
For UI consistency, add v-if="conflictData.length > 0" to line 111-112.
🤖 Prompt for AI Agents
In
src/workbench/extensions/manager/components/manager/NodeConflictDialogContent.vue
around lines 111 to 112, the Extension List Wrapper renders even when
conflictData is empty causing an inconsistent UI; update the wrapper element to
include a conditional rendering check (v-if="conflictData.length > 0") so the
entire Extension List section is omitted when there are no conflicts, matching
the other list wrappers that already use v-if.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests-ui/tests/components/dialog/content/manager/NodeConflictDialogContent.test.ts (2)
251-251: Complete the migration to data-testid selectors for conflict items.The class selector
.flex.h-6.flex-shrink-0is fragile and couples tests to implementation details. Consider adding data-testid attributes to conflict/import-failed item elements in the component and updating these tests accordingly.This would make tests more resilient to styling changes:
- const conflictItems = expandedPanels[0].findAll('.flex.h-6.flex-shrink-0') + const conflictItems = expandedPanels[0].findAll('[data-testid="conflict-item"]')Apply similar changes at lines 333 and 350.
Also applies to: 333-333, 350-350
410-412: Prefer data-testid or behavioral testing over class-based selectors.The multi-class selector
[class*="max-h-"][class*="overflow-y-auto"][class*="scrollbar-hide"]is extremely fragile. Consider either:
- Adding a data-testid to scrollable containers in the component
- Testing scroll behavior functionally (e.g., verify content overflow) rather than checking implementation classes
Example approach using data-testid:
- const scrollableContainer = wrapper.find( - '[class*="max-h-"][class*="overflow-y-auto"][class*="scrollbar-hide"]' - ) + const scrollableContainer = wrapper.find('[data-testid="conflict-scrollable-list"]') expect(scrollableContainer.exists()).toBe(true)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests-ui/tests/components/dialog/content/manager/NodeConflictDialogContent.test.ts(6 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
tests-ui/tests/components/dialog/content/manager/NodeConflictDialogContent.test.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
tests-ui/tests/components/dialog/content/manager/NodeConflictDialogContent.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
tests-ui/tests/components/dialog/content/manager/NodeConflictDialogContent.test.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
tests-ui/tests/components/dialog/content/manager/NodeConflictDialogContent.test.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
tests-ui/tests/components/dialog/content/manager/NodeConflictDialogContent.test.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
tests-ui/tests/components/dialog/content/manager/NodeConflictDialogContent.test.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
tests-ui/tests/components/dialog/content/manager/NodeConflictDialogContent.test.ts
tests-ui/**/*.test.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (tests-ui/CLAUDE.md)
tests-ui/**/*.test.{js,ts,jsx,tsx}: Write tests for new features
Follow existing test patterns in the codebase
Use existing test utilities rather than writing custom utilities
Mock external dependencies in tests
Always prefer vitest mock functions over writing verbose manual mocks
Files:
tests-ui/tests/components/dialog/content/manager/NodeConflictDialogContent.test.ts
**/*.{test,spec}.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Unit and component tests should be located in
tests-ui/or co-located with components assrc/components/**/*.{test,spec}.ts; E2E tests should be inbrowser_tests/
Files:
tests-ui/tests/components/dialog/content/manager/NodeConflictDialogContent.test.ts
🧠 Learnings (6)
📚 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} : Write tests for new features
Applied to files:
tests-ui/tests/components/dialog/content/manager/NodeConflictDialogContent.test.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} : Follow existing test patterns in the codebase
Applied to files:
tests-ui/tests/components/dialog/content/manager/NodeConflictDialogContent.test.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} : Prefer specific selectors in browser tests
Applied to files:
tests-ui/tests/components/dialog/content/manager/NodeConflictDialogContent.test.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: Check tests-ui/README.md for test guidelines
Applied to files:
tests-ui/tests/components/dialog/content/manager/NodeConflictDialogContent.test.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} : Use existing test utilities rather than writing custom utilities
Applied to files:
tests-ui/tests/components/dialog/content/manager/NodeConflictDialogContent.test.ts
📚 Learning: 2025-11-24T19:47:56.371Z
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/lib/litegraph/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:56.371Z
Learning: Applies to src/lib/litegraph/**/*.{test,spec}.{ts,tsx} : Use provided test helpers `createTestSubgraph` and `createTestSubgraphNode` from `./fixtures/subgraphHelpers` for consistent subgraph test setup
Applied to files:
tests-ui/tests/components/dialog/content/manager/NodeConflictDialogContent.test.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: setup
- GitHub Check: test
- GitHub Check: lint-and-format
- GitHub Check: collect
🔇 Additional comments (2)
tests-ui/tests/components/dialog/content/manager/NodeConflictDialogContent.test.ts (2)
141-142: LGTM! Good validation of conditional rendering.The assertion correctly validates that the Conflicts section is hidden when there are no conflicts, and the comment helpfully explains the v-if behavior.
189-198: LGTM! Good migration to data-testid selectors.The migration from class-based selectors to data-testid attributes improves test stability and makes the tests more resilient to styling changes.
viva-jinyi
added
the
New Browser Test Expectations
|
Updating Playwright Expectations |
github-actions
bot
removed
the
New Browser Test Expectations
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/stores/dialogStore.ts (1)
206-225: Add explicit return type annotation for type safety.The function now returns a
DialogInstancebut lacks an explicit return type annotation. As per coding guidelines for stores, TypeScript type safety should be enforced throughout state management.Apply this diff to add the return type:
- function showDialog(options: ShowDialogOptions) { + function showDialog(options: ShowDialogOptions): DialogInstance { const dialogKey = options.key || genDialogKey()Based on coding guidelines: "Use TypeScript for type safety in state management stores."
🧹 Nitpick comments (1)
src/stores/dialogStore.ts (1)
218-218: Remove redundant assignment.Setting
dialog.visible = trueis unnecessary here since we're in theelsebranch wheredialog.visibleis alreadytrue(theifcondition checks!dialog.visible).Apply this diff:
} else { - dialog.visible = true riseDialog(dialog) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/stores/dialogStore.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{vue,ts,tsx}: Leverage VueUse functions for performance-enhancing utilities
Use vue-i18n in Composition API for any string literals and place new translation entries in src/locales/en/main.json
Files:
src/stores/dialogStore.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
src/stores/dialogStore.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Use TypeScript for type safety
**/*.{ts,tsx}: Never useanytype - use proper TypeScript types
Never useas anytype assertions - fix the underlying type issue
Files:
src/stores/dialogStore.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (.cursorrules)
Implement proper error handling in components and services
**/*.{ts,tsx,js,vue}: Use 2-space indentation, single quotes, no semicolons, and maintain 80-character line width as configured in.prettierrc
Organize imports by sorting and grouping by plugin, and runpnpm formatbefore committing
Files:
src/stores/dialogStore.ts
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/stores/dialogStore.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/stores/dialogStore.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
src/stores/dialogStore.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,vue}: Useconst settingStore = useSettingStore()andsettingStore.get('Comfy.SomeSetting')to retrieve settings in TypeScript/Vue files
Useawait settingStore.set('Comfy.SomeSetting', newValue)to update settings in TypeScript/Vue files
Check server capabilities usingapi.serverSupportsFeature('feature_name')before using enhanced features
Useapi.getServerFeature('config_name', defaultValue)to retrieve server feature configurationEnforce ESLint rules for Vue + TypeScript including: no floating promises, no unused imports, and i18n raw text restrictions in templates
Files:
src/stores/dialogStore.ts
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Define dynamic setting defaults using runtime context with functions in settings configuration
UsedefaultsByInstallVersionproperty for gradual feature rollout based on version in settings configuration
Files:
src/stores/dialogStore.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/stores/dialogStore.ts
src/**/stores/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
src/**/stores/**/*.{ts,tsx}: Maintain clear public interfaces and restrict extension access in stores
Use TypeScript for type safety in state management stores
Files:
src/stores/dialogStore.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/stores/dialogStore.ts
**/stores/*Store.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name Pinia stores with the
*Store.tssuffix
Files:
src/stores/dialogStore.ts
🧬 Code graph analysis (1)
src/stores/dialogStore.ts (1)
src/lib/litegraph/src/LGraphCanvas.ts (1)
createDialog(7509-7614)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test
- GitHub Check: lint-and-format
- GitHub Check: setup
- GitHub Check: collect
Summary
showSmallDialoghelper function todialogServicefor reusable small dialog stylingshowLoadWorkflowWarningandshowNodeConflictDialoginto independent composablesuseMissingNodesDialoguseNodeConflictDialogChanges
showSmallDialoghelper, removed migrated functions🤖 Generated with Claude Code