Support "control after generate" in vue #6985
Conversation
The @PointerUp.stop was breaking reka ui NumberFields. IIRC, this was added to allow selecting text without dragging nodes. Current testing suggests this isn't required for pointerup This reduces the margins some on number inputs. It's trivial to add a px-2.5, but helps with information density
AustinMroz
requested review from
a team,
KarryCharon,
Yorha4D and
shinshin86
as code owners
dosubot
bot
added
the
size:XXL
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (17)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the 📝 WalkthroughWalkthroughAdds a number-control system: new control widget types and types, UI components and composables to set/apply control modes, a registry to run controls before/after generation, a global seed store, test coverage, and hooks into the app queue flow. Changes
Sequence DiagramsequenceDiagram
participant User
participant WidgetInputNumberWithControl
participant NumberControlPopover
participant useStepperControl
participant NumberControlRegistry
participant GlobalSeedStore
participant app
User->>WidgetInputNumberWithControl: click control button
WidgetInputNumberWithControl->>NumberControlPopover: togglePopover()
User->>NumberControlPopover: select control mode
NumberControlPopover-->>WidgetInputNumberWithControl: emit update:control-mode
WidgetInputNumberWithControl->>useStepperControl: setControlMode(newMode)
Note over User,app: During prompt queuing
User->>app: queuePrompt()
app->>NumberControlRegistry: executeNumberControls('before')
NumberControlRegistry->>useStepperControl: invoke registered applyControl callbacks
alt INCREMENT/DECREMENT
useStepperControl->>WidgetInputNumberWithControl: apply step change
else RANDOMIZE
useStepperControl->>GlobalSeedStore: read seed
useStepperControl->>WidgetInputNumberWithControl: apply random value
else LINK_TO_GLOBAL
useStepperControl->>GlobalSeedStore: read & clamp seed
useStepperControl->>WidgetInputNumberWithControl: apply clamped seed
else FIXED
Note right of useStepperControl: no-op
end
app->>NumberControlRegistry: executeNumberControls('after')
Possibly related PRs
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 12/05/2025, 12:42:00 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results⏰ Completed at: 12/05/2025, 12:50:53 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.2 MB (baseline 3.2 MB) • 🔴 +2.77 kBMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 975 kB (baseline 974 kB) • 🔴 +729 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 — 180 kB (baseline 173 kB) • 🔴 +7 kBReusable component library chunks
Status: 7 added / 7 removed Data & Services — 12.5 kB (baseline 12.5 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 2 added / 2 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) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 3.82 MB (baseline 3.81 MB) • 🔴 +7.51 kBBundles that do not match a named category
Status: 19 added / 18 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: 3
🧹 Nitpick comments (8)
src/renderer/extensions/vueNodes/widgets/components/WidgetSelectDropdown.vue (2)
148-165: UnifieddropdownItemslogic looks correctThe refactor to have asset mode always use
allItemsand non-asset “all” explicitly concatenateinputItemsandoutputItemsis consistent with the surrounding data flow; no functional issues stand out. If you want to reduce duplication, the'all'branch could just returnallItems.value, but that’s purely optional.
70-81: Localize user-facing strings and tighten upload error messagingSeveral strings are user-visible but not run through vue-i18n, which conflicts with the repo guidelines:
- Filter option labels:
'All','Inputs','Outputs'(Lines 76–79).- Toast messages:
File upload failed,Upload failed: ${error}(Lines 296, 320).Consider:
- Replacing these literals with
t(...)calls and adding appropriate entries insrc/locales/en/main.json.- For the upload error toast, deriving a readable message (e.g.
error instanceof Error ? error.message : String(error)) rather than stringifying the raw error object.This keeps UX consistent and ready for localization without changing behavior.
Also applies to: 263-264, 295-321
tests-ui/tests/stores/globalSeedStore.test.ts (1)
21-28: Consider replacing the probabilistic assertion.This test relies on random seed generation to be different across store instances, which introduces non-determinism. While the 1-in-1,000,000 chance is low, flaky tests can cause false failures in CI/CD pipelines.
Consider one of these alternatives:
- Mock
Math.random()to return predictable values- Test that the seed is within the valid range instead of comparing uniqueness
- Accept the low flakiness risk and document it clearly
Example with mocking:
it('should create different seeds for different store instances', () => { + const mockRandom = vi.spyOn(Math, 'random') + mockRandom.mockReturnValueOnce(0.5) const store1 = useGlobalSeedStore() setActivePinia(createPinia()) // Reset pinia + mockRandom.mockReturnValueOnce(0.7) const store2 = useGlobalSeedStore() - // Very unlikely to be the same (1 in 1,000,000 chance) - expect(store1.globalSeed).not.toBe(store2.globalSeed) + expect(store1.globalSeed).toBe(500000) + expect(store2.globalSeed).toBe(700000) + mockRandom.mockRestore() })src/types/simplifiedWidget.ts (1)
18-26: Consider adding 'link-to-global' to ControlWidgetOptions type.The
NumberControlPopover.vuecomponent references aLINK_TO_GLOBALmode (currently disabled via feature flag), but this option is missing from theControlWidgetOptionstype union. While the feature is disabled now, including it in the type definition would ensure type safety when the feature is enabled.export type ControlWidgetOptions = | 'fixed' | 'increment' | 'decrement' | 'randomize' + | 'link-to-global'src/renderer/extensions/vueNodes/widgets/components/NumberControlPopover.vue (1)
29-29: Document the LINK_TO_GLOBAL feature flag.The
ENABLE_LINK_TO_GLOBALconstant is hardcoded tofalse, but there's no comment explaining why this feature is disabled or what would be required to enable it. Consider adding a comment to guide future development.+// TODO: Enable LINK_TO_GLOBAL once global seed synchronization is fully implemented const ENABLE_LINK_TO_GLOBAL = falsesrc/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberWithControl.vue (1)
57-64: Consider usingcn()utility for class merging.Per coding guidelines, prefer using the
cn()utility from@/utils/tailwindUtilfor class merging instead of template string interpolation.- <i :class="`${controlButtonIcon} text-blue-100 text-xs`" /> + <i :class="cn(controlButtonIcon, 'text-blue-100 text-xs')" />You'll need to import
cnfrom@/utils/tailwindUtil.src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue (1)
123-128: Remove potentially dead PrimeVue CSS.This scoped CSS targets
.p-inputnumber-input, a PrimeVue class. Since the component now uses Reka UI'sNumberFieldRoot/NumberFieldInput, this CSS may no longer apply and could be removed.src/renderer/extensions/vueNodes/widgets/services/NumberControlRegistry.ts (1)
27-34: Consider wrapping callback execution in try-catch for resilience.If one registered
applyFnthrows an error, it will prevent subsequent controls from being applied. Consider wrapping each callback in a try-catch to ensure all controls are executed.executeControls(phase: 'before' | 'after'): void { const settingStore = useSettingStore() if (settingStore.get('Comfy.WidgetControlMode') === phase) { for (const applyFn of this.controls.values()) { - applyFn() + try { + applyFn() + } catch (error) { + console.error('Error executing number control:', error) + } } } }
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue
Outdated
Show resolved
Hide resolved
| if (typeof window !== 'undefined') { | ||
| import('@/base/common/downloadUtil') | ||
| .then((module) => { | ||
| const fn = ( | ||
| module as { | ||
| downloadBlob?: typeof import('@/base/common/downloadUtil').downloadBlob | ||
| } | ||
| ).downloadBlob | ||
| if (typeof fn === 'function') { | ||
| ;(window as any).downloadBlob = fn | ||
| } | ||
| }) | ||
| .catch(() => {}) | ||
| } |
There was a problem hiding this comment.
Fix TypeScript violations: avoid as any and improve error handling.
This code violates two explicit coding guidelines:
- Line 63 uses
as anytype assertion - Line 66 silently swallows errors
As per coding guidelines, never use as any type assertions and implement proper error handling.
Apply this diff to fix both issues:
+declare global {
+ interface Window {
+ downloadBlob?: typeof import('@/base/common/downloadUtil').downloadBlob
+ }
+}
+
if (typeof window !== 'undefined') {
import('@/base/common/downloadUtil')
.then((module) => {
const fn = (
module as {
downloadBlob?: typeof import('@/base/common/downloadUtil').downloadBlob
}
).downloadBlob
if (typeof fn === 'function') {
- ;(window as any).downloadBlob = fn
+ window.downloadBlob = fn
}
})
- .catch(() => {})
+ .catch((error) => {
+ console.error('Failed to load downloadBlob utility:', error)
+ })
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (typeof window !== 'undefined') { | |
| import('@/base/common/downloadUtil') | |
| .then((module) => { | |
| const fn = ( | |
| module as { | |
| downloadBlob?: typeof import('@/base/common/downloadUtil').downloadBlob | |
| } | |
| ).downloadBlob | |
| if (typeof fn === 'function') { | |
| ;(window as any).downloadBlob = fn | |
| } | |
| }) | |
| .catch(() => {}) | |
| } | |
| declare global { | |
| interface Window { | |
| downloadBlob?: typeof import('@/base/common/downloadUtil').downloadBlob | |
| } | |
| } | |
| if (typeof window !== 'undefined') { | |
| import('@/base/common/downloadUtil') | |
| .then((module) => { | |
| const fn = ( | |
| module as { | |
| downloadBlob?: typeof import('@/base/common/downloadUtil').downloadBlob | |
| } | |
| ).downloadBlob | |
| if (typeof fn === 'function') { | |
| window.downloadBlob = fn | |
| } | |
| }) | |
| .catch((error) => { | |
| console.error('Failed to load downloadBlob utility:', error) | |
| }) | |
| } |
DrJKL
added
the
New Browser Test Expectations
|
Updating Playwright Expectations |
github-actions
bot
removed
the
New Browser Test Expectations
dosubot
bot
added
size:L
dosubot
bot
added
size:XXL
AustinMroz
added
the
New Browser Test Expectations
github-actions
bot
removed
the
New Browser Test Expectations
dosubot
bot
added
size:XL
Playwright locators break because there's now an additional button Swapping to reka ui introduces a bug where clicking and dragging the mouse off a button causes the number to change continually. This is hard to change and I found a workaround to get truncation to work with primevue instead.
dosubot
bot
added
size:XXL
AustinMroz
added
the
New Browser Test Expectations
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
browser_tests/fixtures/VueNodeHelpers.ts (1)
159-164: Avoid relying on button index; prefer a more specific decrement selectorSwitching from
last()tonth(1)will correctly skip a trailing control/config button, but this is still fragile: tests now depend on the exact DOM order and button count inside the widget. If the template gains another button or the order changes,nth(1)may stop pointing to the decrement control.Where possible, prefer a more explicit locator for the decrement button (e.g. a
data-testid, role+name, or_vueselector targeting the specific component/prop) rather than its position among allbuttonelements:// Example shape, assuming markup supports it decrementButton: widget.getByTestId('number-decrement') // or decrementButton: widget.getByRole('button', { name: 'Decrement' })This will make the tests more stable and aligns with the guidance to use specific selectors in browser tests. Based on learnings, ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
browser_tests/fixtures/VueNodeHelpers.ts(1 hunks)browser_tests/tests/vueNodes/widgets/int/integerWidget.spec.ts(1 hunks)src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue(4 hunks)
🧰 Additional context used
📓 Path-based instructions (18)
**/*.{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:
browser_tests/fixtures/VueNodeHelpers.tsbrowser_tests/tests/vueNodes/widgets/int/integerWidget.spec.tssrc/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
browser_tests/fixtures/VueNodeHelpers.tsbrowser_tests/tests/vueNodes/widgets/int/integerWidget.spec.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:
browser_tests/fixtures/VueNodeHelpers.tsbrowser_tests/tests/vueNodes/widgets/int/integerWidget.spec.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:
browser_tests/fixtures/VueNodeHelpers.tsbrowser_tests/tests/vueNodes/widgets/int/integerWidget.spec.tssrc/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
browser_tests/fixtures/VueNodeHelpers.tsbrowser_tests/tests/vueNodes/widgets/int/integerWidget.spec.tssrc/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue
**/*.{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:
browser_tests/fixtures/VueNodeHelpers.tsbrowser_tests/tests/vueNodes/widgets/int/integerWidget.spec.tssrc/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue
**/*.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:
browser_tests/fixtures/VueNodeHelpers.tsbrowser_tests/tests/vueNodes/widgets/int/integerWidget.spec.ts
browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (browser_tests/CLAUDE.md)
browser_tests/**/*.{e2e,spec}.{ts,tsx,js,jsx}: Test user workflows in browser tests
Use Playwright fixtures for browser tests
Follow naming conventions for browser tests
Check assets/ directory for test data when writing tests
Prefer specific selectors in browser tests
Test across multiple viewports
Files:
browser_tests/tests/vueNodes/widgets/int/integerWidget.spec.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:
browser_tests/tests/vueNodes/widgets/int/integerWidget.spec.ts
browser_tests/**/*.{test,spec}.ts
📄 CodeRabbit inference engine (AGENTS.md)
Playwright E2E tests can use optional tags like
@mobileand@2xwhich are respected by the Playwright configuration
Files:
browser_tests/tests/vueNodes/widgets/int/integerWidget.spec.ts
**/*.vue
📄 CodeRabbit inference engine (.cursorrules)
**/*.vue: Use setup() function for component logic in Vue 3 Composition API
Utilize ref and reactive for reactive state in Vue 3
Implement computed properties with computed() function
Use watch and watchEffect for side effects in Vue 3
Implement lifecycle hooks with onMounted, onUpdated, etc.
Utilize provide/inject for dependency injection in Vue 3
Use Vue 3.5 style of default prop declaration with defineProps()
Organize Vue components in <script> <style> order
Use Tailwind CSS for styling Vue components
Implement responsive design with Tailwind CSS
Do not use deprecated PrimeVue components (Dropdown, OverlayPanel, Calendar, InputSwitch, Sidebar, Chips, TabMenu, Steps, InlineMessage). Use replacements: Select, Popover, DatePicker, ToggleSwitch, Drawer, AutoComplete, Tabs, Stepper, Message respectively
Implement proper props and emits definitions in Vue components
Utilize Vue 3's Teleport component when needed
Use Suspense for async components in Vue 3
Follow Vue 3 style guide and naming conventions
Never use deprecated PrimeVue components (Dropdown, OverlayPanel, Calendar, InputSwitch, Sidebar, Chips, TabMenu, Steps, InlineMessage)Never use
:class="[]"to merge class names - always useimport { cn } from '@/utils/tailwindUtil'for class merging in Vue templates
**/*.vue: Use TypeScript with Vue 3 Single File Components (.vuefiles)
Name Vue components in PascalCase (e.g.,MenuHamburger.vue)Files:
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vuesrc/**/*.vue
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.vue: Use the Vue 3 Composition API instead of the Options API when writing Vue components (exception: when overriding or extending PrimeVue components for compatibility)
Use setup() function for component logic
Utilize ref and reactive for reactive state
Implement computed properties with computed()
Use watch and watchEffect for side effects
Implement lifecycle hooks with onMounted, onUpdated, etc.
Utilize provide/inject for dependency injection
Use vue 3.5 style of default prop declaration
Use Tailwind CSS for styling
Implement proper props and emits definitions
Utilize Vue 3's Teleport component when needed
Use Suspense for async components
Follow Vue 3 style guide and naming conventionsFiles:
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vuesrc/**/*.{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.jsonFiles:
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue**/*.{vue,html}
📄 CodeRabbit inference engine (CLAUDE.md)
Never use
dark:ordark-theme:Tailwind variants - instead use semantic values fromstyle.csstheme, e.g.bg-node-component-surfaceFiles:
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vuesrc/**/*.{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 codebaseFiles:
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vuesrc/**/{composables,components}/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Clean up subscriptions in state management to prevent memory leaks
Files:
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vuesrc/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vuesrc/**/{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.jsonFiles:
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue🧠 Learnings (14)
📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR Repo: Comfy-Org/ComfyUI_frontend PR: 0 File: src/components/CLAUDE.md:0-0 Timestamp: 2025-11-24T19:47:45.616Z Learning: Applies to src/components/**/*.vue : Replace PrimeVue InputSwitch component with ToggleSwitchApplied to files:
browser_tests/fixtures/VueNodeHelpers.tssrc/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue📚 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 featuresApplied to files:
browser_tests/fixtures/VueNodeHelpers.tsbrowser_tests/tests/vueNodes/widgets/int/integerWidget.spec.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 testsApplied to files:
browser_tests/fixtures/VueNodeHelpers.tsbrowser_tests/tests/vueNodes/widgets/int/integerWidget.spec.ts📚 Learning: 2025-11-24T19:46:52.279Z
Learnt from: CR Repo: Comfy-Org/ComfyUI_frontend PR: 0 File: .cursorrules:0-0 Timestamp: 2025-11-24T19:46:52.279Z Learning: Applies to **/*.vue : Do not use deprecated PrimeVue components (Dropdown, OverlayPanel, Calendar, InputSwitch, Sidebar, Chips, TabMenu, Steps, InlineMessage). Use replacements: Select, Popover, DatePicker, ToggleSwitch, Drawer, AutoComplete, Tabs, Stepper, Message respectivelyApplied to files:
browser_tests/fixtures/VueNodeHelpers.tssrc/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue📚 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 codebaseApplied to files:
browser_tests/tests/vueNodes/widgets/int/integerWidget.spec.ts📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR Repo: Comfy-Org/ComfyUI_frontend PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-11-24T19:47:02.860Z Learning: Applies to src/**/*.{vue,ts} : Leverage VueUse functions for performance-enhancing stylesApplied to files:
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue📚 Learning: 2025-11-24T19:46:52.279Z
Learnt from: CR Repo: Comfy-Org/ComfyUI_frontend PR: 0 File: .cursorrules:0-0 Timestamp: 2025-11-24T19:46:52.279Z Learning: Applies to **/*.vue : Never use deprecated PrimeVue components (Dropdown, OverlayPanel, Calendar, InputSwitch, Sidebar, Chips, TabMenu, Steps, InlineMessage)Applied to files:
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR Repo: Comfy-Org/ComfyUI_frontend PR: 0 File: src/components/CLAUDE.md:0-0 Timestamp: 2025-11-24T19:47:45.616Z Learning: Applies to src/components/**/*.vue : Replace PrimeVue Steps component with Stepper without panelsApplied to files:
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR Repo: Comfy-Org/ComfyUI_frontend PR: 0 File: src/components/CLAUDE.md:0-0 Timestamp: 2025-11-24T19:47:45.616Z Learning: Applies to src/components/**/*.vue : Use setup() function in Vue 3 Composition APIApplied to files:
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue📚 Learning: 2025-11-24T19:47:45.616Z
Learnt from: CR Repo: Comfy-Org/ComfyUI_frontend PR: 0 File: src/components/CLAUDE.md:0-0 Timestamp: 2025-11-24T19:47:45.616Z Learning: Applies to src/components/**/*.vue : Implement computed() for derived state in Vue 3 Composition APIApplied to files:
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue📚 Learning: 2025-11-24T19:46:52.279Z
Learnt from: CR Repo: Comfy-Org/ComfyUI_frontend PR: 0 File: .cursorrules:0-0 Timestamp: 2025-11-24T19:46:52.279Z Learning: Applies to **/*.vue : Implement computed properties with computed() functionApplied to files:
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue📚 Learning: 2025-11-24T19:47:02.860Z
Learnt from: CR Repo: Comfy-Org/ComfyUI_frontend PR: 0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-11-24T19:47:02.860Z Learning: Applies to src/**/*.vue : Implement computed properties with computed()Applied to files:
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue📚 Learning: 2025-11-24T19:46:52.279Z
Learnt from: CR Repo: Comfy-Org/ComfyUI_frontend PR: 0 File: .cursorrules:0-0 Timestamp: 2025-11-24T19:46:52.279Z Learning: Applies to **/*.{vue,ts,tsx} : Leverage VueUse functions for performance-enhancing utilitiesApplied to files:
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue📚 Learning: 2025-11-24T19:46:52.279Z
Learnt from: CR Repo: Comfy-Org/ComfyUI_frontend PR: 0 File: .cursorrules:0-0 Timestamp: 2025-11-24T19:46:52.279Z Learning: Applies to **/*.vue : Use setup() function for component logic in Vue 3 Composition APIApplied to files:
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue🪛 ESLint
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue
[error] 3-3: Unable to resolve path to module 'vue'.
(import-x/no-unresolved)
⏰ 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). (6)
- GitHub Check: deploy-and-comment
- GitHub Check: setup
- GitHub Check: collect
- GitHub Check: test
- GitHub Check: setup
- GitHub Check: lint-and-format
🔇 Additional comments (2)
browser_tests/tests/vueNodes/widgets/int/integerWidget.spec.ts (1)
18-20: The.first()selector is appropriate and improves code explicitness.The workflow contains only one KSampler node, making the explicit
.first()call safe and deterministic. This aligns with Playwright best practices for clear widget selection in tests.src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue (1)
3-3: ESLintimport-x/no-unresolvedon'vue'is likely configuration-related
import { computed, useSlots } from 'vue'is the standard Vue 3 import. If ESLint is flagging this as unresolved, it’s almost certainly animport/resolveror dependency setup issue rather than a code problem. Please verify thatvueis installed and that your ESLint resolver is configured to work with your bundler/TypeScript setup.
| const slots = useSlots() | ||
| const inputClass = computed(() => | ||
| cn( | ||
| '[&>input]:bg-transparent [&>input]:border-0', | ||
| '[&>input]:truncate [&>input]:min-w-[4ch]', | ||
| slots.default && '[&>input]:pr-7' | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Avoid always-rendered overlay div that can block input interaction
The useSlots + inputClass approach to add right padding when a default slot exists is solid, but the absolutely positioned wrapper:
<div class="absolute top-5 right-8 h-4 w-7 -translate-y-4/5">
<slot />
</div>renders unconditionally. When no default slot is provided, this creates an invisible box over the input’s top‑right area, which can intercept clicks/focus without showing any control.
You can align behavior with inputClass by only rendering the wrapper when a default slot is present:
- <div class="absolute top-5 right-8 h-4 w-7 -translate-y-4/5">
- <slot />
- </div>
+ <div
+ v-if="slots.default"
+ class="absolute top-5 right-8 h-4 w-7 -translate-y-4/5"
+ >
+ <slot />
+ </div>This keeps the overlay and extra padding in sync and avoids an invisible hitbox when no control is slotted in.
Also applies to: 100-103, 119-121
🤖 Prompt for AI Agents
In
src/renderer/extensions/vueNodes/widgets/components/WidgetInputNumberInput.vue
around lines 72-80 (and similarly at 100-103 and 119-121), the absolutely
positioned wrapper divs are rendered unconditionally and can create an invisible
hitbox when no default slot is provided; make those wrapper divs render only
when a default slot exists by guarding them with a conditional (e.g.
v-if="slots.default" or v-if="$slots.default" depending on script setup/context)
so the overlay appears only when the slot is present and stays in sync with the
inputClass padding logic.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (11)
browser_tests/tests/vueNodes/groups/groups.spec.ts-snapshots/vue-groups-fit-to-contents-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-dragging-link-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-input-drag-ctrl-alt-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-input-drag-reuses-origin-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-reroute-input-drag-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-reroute-output-shift-drag-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-shift-output-multi-link-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-snap-to-node-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/interactions/links/linkInteraction.spec.ts-snapshots/vue-node-snap-to-slot-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/nodeStates/colors.spec.ts-snapshots/vue-node-custom-colors-dark-all-colors-chromium-linux.pngis excluded by!**/*.pngbrowser_tests/tests/vueNodes/nodeStates/colors.spec.ts-snapshots/vue-node-custom-colors-light-all-colors-chromium-linux.pngis excluded by!**/*.png
📒 Files selected for processing (3)
src/locales/en/main.json(1 hunks)src/renderer/extensions/vueNodes/widgets/components/layout/WidgetLayoutField.vue(0 hunks)src/scripts/app.ts(3 hunks)
💤 Files with no reviewable changes (1)
- src/renderer/extensions/vueNodes/widgets/components/layout/WidgetLayoutField.vue
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{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/scripts/app.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursorrules)
Use es-toolkit for utility functions
Files:
src/scripts/app.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/scripts/app.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/scripts/app.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/scripts/app.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/scripts/app.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use camelCase for variable and setting names in TypeScript/Vue files
Files:
src/scripts/app.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/scripts/app.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/scripts/app.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/scripts/app.ts
src/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (src/CLAUDE.md)
Follow Vue 3 composition API style guide
Files:
src/scripts/app.ts
🧬 Code graph analysis (1)
src/scripts/app.ts (1)
src/renderer/extensions/vueNodes/widgets/services/NumberControlRegistry.ts (1)
executeNumberControls(57-59)
⏰ 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). (6)
- GitHub Check: deploy-and-comment
- GitHub Check: setup
- GitHub Check: setup
- GitHub Check: lint-and-format
- GitHub Check: collect
- GitHub Check: test
🔇 Additional comments (3)
src/scripts/app.ts (3)
34-34: Import placement and dependency layering look goodThe new
executeNumberControlsimport follows the existing pattern of pulling renderer/vueNodes utilities intoapp.ts(we already importensureCorrectLayoutScalefrom the same area), so this dependency direction is consistent and there’s no unused import issue here.
1345-1358: Confirm intended scope and timing ofexecuteNumberControls('before')
executeNumberControls('before')now runs once per batch iteration, after allbeforeQueuedwidget callbacks (main graph + subgraphs) and immediately beforegraphToPrompt. That means:
- Controls see the post‑widget state of the graph and affect the prompt that is just about to be queued.
- The hook runs regardless of whether any number controls are actually registered, assuming the registry is a safe no‑op in that case.
- For partial executions (
queueNodeIds), the hook still runs globally and doesn’t filter by the nodes being executed.Please double‑check that this ordering and global scope match the intended design for number controls (especially for control‑before‑generate vs control‑after‑generate modes and partial execution flows). If you want controls to be able to key off which nodes will run, they may need access to
porqueueNodeIdshere.
1396-1408: ClarifyexecuteNumberControls('after')behavior on errors and partial failures
executeNumberControls('after')is invoked only whenapi.queuePromptresolves successfully; it is skipped when an exception is thrown because thebreakinside thecatchexits the loop before reaching this call. It still runs whenres.node_errorsis non‑empty.This seems reasonable (don’t run “after” controls when the prompt wasn’t queued at all), but it does mean:
- Any “after‑generate” behavior will not run on
PromptExecutionErroreven if prior hooks (e.g.before) have already adjusted state.- “After” hooks will still run when the backend reports node‑level errors in
res.node_errors.Please confirm that this matches the intended UX for control‑after‑generate, especially around failed/partial executions; if you expect symmetrical cleanup or state updates on certain error paths, we may want to call
executeNumberControls('after')in a narrowerfinallyaroundqueuePromptor gate it explicitly onres.prompt_id/lastNodeErrors.
| "numberControl": { | ||
| "controlHeaderBefore": "Automatically update the value", | ||
| "controlHeaderAfter": "AFTER", | ||
| "controlHeaderBefore2": "BEFORE", | ||
| "controlHeaderEnd": "running the workflow:", | ||
| "linkToGlobal": "Link to", | ||
| "linkToGlobalSeed": "Global Value", | ||
| "linkToGlobalDesc": "Unique value linked to the Global Value's control setting", | ||
| "randomize": "Randomize Value", | ||
| "randomizeDesc": "Shuffles the value randomly after each generation", | ||
| "increment": "Increment Value", | ||
| "incrementDesc": "Adds 1 to the value number", | ||
| "decrement": "Decrement Value", | ||
| "decrementDesc": "Subtracts 1 from the value number", | ||
| "editSettings": "Edit control settings" | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify inconsistent capitalization and key naming in number control strings.
Lines 1915-1916 show inconsistent capitalization: controlHeaderBefore uses mixed case ("Automatically update the value"), while controlHeaderAfter and controlHeaderBefore2 use ALL CAPS ("AFTER", "BEFORE"). This inconsistency suggests either an error or unclear intent about how these strings are displayed.
Additionally, using both controlHeaderBefore and controlHeaderBefore2 as key names is confusing—why does one have a "2" suffix? If these are meant to be alternative headers or parts of the same phrase (e.g., a sentence fragment), use more descriptive names (e.g., controlModePrefix, controlModeAfter, controlModeBefore, controlModeSuffix) to clarify their purpose.
Please verify:
- Why
controlHeaderBeforeandcontrolHeaderBefore2have different naming; clarify if one should be renamed. - How these strings are composed in the Vue component—do they concatenate into a grammatically correct sentence?
- Ensure capitalization is intentional (ALL CAPS for mode labels vs. mixed case for descriptions).
🤖 Prompt for AI Agents
In src/locales/en/main.json around lines 1914 to 1929, the keys
controlHeaderBefore and controlHeaderBefore2 are confusing and capitalization is
inconsistent (mixed case vs ALL CAPS); rename keys to expressive names (e.g.,
controlModeBefore, controlModeAfter, controlModePrefix, controlModeSuffix) to
reflect their roles, standardize string capitalization (use ALL CAPS only for
mode labels or mixed case for descriptive text consistently), update the Vue
component to read the new keys and concatenate them into a single grammatically
correct sentence (ensure spacing/punctuation when joining fragments), and remove
the numeric suffix key name; confirm these changes across all locale files and
component usages.
Continuation of #6034 with
Several issues from original PR have not (yet) been addressed, but are likely better moved to future PR
┆Issue is synchronized with this Notion page by Unito