diff --git a/src/renderer/extensions/vueNodes/widgets/components/NumberControlPopover.vue b/src/renderer/extensions/vueNodes/widgets/components/NumberControlPopover.vue index 30a46c65f..22c550f91 100644 --- a/src/renderer/extensions/vueNodes/widgets/components/NumberControlPopover.vue +++ b/src/renderer/extensions/vueNodes/widgets/components/NumberControlPopover.vue @@ -115,7 +115,7 @@ const handleEditSettings = () => {
- + {{ $t(`widgets.numberControl.${option.title}`) }}
diff --git a/src/renderer/extensions/vueNodes/widgets/composables/useStepperControl.ts b/src/renderer/extensions/vueNodes/widgets/composables/useStepperControl.ts deleted file mode 100644 index bde6499e7..000000000 --- a/src/renderer/extensions/vueNodes/widgets/composables/useStepperControl.ts +++ /dev/null @@ -1,111 +0,0 @@ -import { computed, onMounted, onUnmounted, ref } from 'vue' -import type { Ref } from 'vue' - -import type { ControlOptions } from '@/types/simplifiedWidget' - -import { numberControlRegistry } from '../services/NumberControlRegistry' - -export enum NumberControlMode { - FIXED = 'fixed', - INCREMENT = 'increment', - DECREMENT = 'decrement', - RANDOMIZE = 'randomize', - LINK_TO_GLOBAL = 'linkToGlobal' -} - -interface StepperControlOptions { - min?: number - max?: number - step?: number - step2?: number - onChange?: (value: number) => void -} - -function convertToEnum(str?: ControlOptions): NumberControlMode { - switch (str) { - case 'fixed': - return NumberControlMode.FIXED - case 'increment': - return NumberControlMode.INCREMENT - case 'decrement': - return NumberControlMode.DECREMENT - case 'randomize': - return NumberControlMode.RANDOMIZE - } - return NumberControlMode.RANDOMIZE -} - -function useControlButtonIcon(controlMode: Ref) { - return computed(() => { - switch (controlMode.value) { - case NumberControlMode.INCREMENT: - return 'pi pi-plus' - case NumberControlMode.DECREMENT: - return 'pi pi-minus' - case NumberControlMode.FIXED: - return 'icon-[lucide--pencil-off]' - case NumberControlMode.LINK_TO_GLOBAL: - return 'pi pi-link' - default: - return 'icon-[lucide--shuffle]' - } - }) -} - -export function useStepperControl( - modelValue: Ref, - options: StepperControlOptions, - defaultValue?: ControlOptions -) { - const controlMode = ref(convertToEnum(defaultValue)) - const controlId = Symbol('numberControl') - - const applyControl = () => { - const { min = 0, max = 1000000, step2, step = 1, onChange } = options - const safeMax = Math.min(2 ** 50, max) - const safeMin = Math.max(-(2 ** 50), min) - // Use step2 if available (widget context), otherwise use step as-is (direct API usage) - const actualStep = step2 !== undefined ? step2 : step - - let newValue: number - switch (controlMode.value) { - case NumberControlMode.FIXED: - // Do nothing - keep current value - return - case NumberControlMode.INCREMENT: - newValue = Math.min(safeMax, modelValue.value + actualStep) - break - case NumberControlMode.DECREMENT: - newValue = Math.max(safeMin, modelValue.value - actualStep) - break - case NumberControlMode.RANDOMIZE: - newValue = Math.floor(Math.random() * (safeMax - safeMin + 1)) + safeMin - break - default: - return - } - - if (onChange) { - onChange(newValue) - } else { - modelValue.value = newValue - } - } - - // Register with singleton registry - onMounted(() => { - numberControlRegistry.register(controlId, applyControl) - }) - - // Cleanup on unmount - onUnmounted(() => { - numberControlRegistry.unregister(controlId) - }) - const controlButtonIcon = useControlButtonIcon(controlMode) - - return { - applyControl, - controlButtonIcon, - controlMode - } -} diff --git a/src/renderer/extensions/vueNodes/widgets/services/NumberControlRegistry.ts b/src/renderer/extensions/vueNodes/widgets/services/NumberControlRegistry.ts deleted file mode 100644 index 9fc1e6f14..000000000 --- a/src/renderer/extensions/vueNodes/widgets/services/NumberControlRegistry.ts +++ /dev/null @@ -1,59 +0,0 @@ -import { useSettingStore } from '@/platform/settings/settingStore' - -/** - * Registry for managing Vue number controls with deterministic execution timing. - * Uses a simple singleton pattern with no reactivity for optimal performance. - */ -export class NumberControlRegistry { - private controls = new Map void>() - - /** - * Register a number control callback - */ - register(id: symbol, applyFn: () => void): void { - this.controls.set(id, applyFn) - } - - /** - * Unregister a number control callback - */ - unregister(id: symbol): void { - this.controls.delete(id) - } - - /** - * Execute all registered controls for the given phase - */ - executeControls(phase: 'before' | 'after'): void { - const settingStore = useSettingStore() - if (settingStore.get('Comfy.WidgetControlMode') === phase) { - for (const applyFn of this.controls.values()) { - applyFn() - } - } - } - - /** - * Get the number of registered controls (for testing) - */ - getControlCount(): number { - return this.controls.size - } - - /** - * Clear all registered controls (for testing) - */ - clear(): void { - this.controls.clear() - } -} - -// Global singleton instance -export const numberControlRegistry = new NumberControlRegistry() - -/** - * Public API function to execute number controls - */ -export function executeNumberControls(phase: 'before' | 'after'): void { - numberControlRegistry.executeControls(phase) -} diff --git a/src/scripts/app.ts b/src/scripts/app.ts index 3a953e963..473c32fb4 100644 --- a/src/scripts/app.ts +++ b/src/scripts/app.ts @@ -31,7 +31,6 @@ import { type NodeId, isSubgraphDefinition } from '@/platform/workflow/validation/schemas/workflowSchema' -import { executeNumberControls } from '@/renderer/extensions/vueNodes/widgets/services/NumberControlRegistry' import type { ExecutionErrorWsMessage, NodeError, @@ -1359,7 +1358,6 @@ export class ComfyApp { forEachNode(this.rootGraph, (node) => { for (const widget of node.widgets ?? []) widget.beforeQueued?.() }) - executeNumberControls('before') const p = await this.graphToPrompt(this.rootGraph) const queuedNodes = collectAllNodes(this.rootGraph) @@ -1404,7 +1402,6 @@ export class ComfyApp { // Allow widgets to run callbacks after a prompt has been queued // e.g. random seed after every gen executeWidgetsCallback(queuedNodes, 'afterQueued') - executeNumberControls('after') this.canvas.draw(true, true) await this.ui.queue.update() } diff --git a/tests-ui/tests/renderer/extensions/vueNodes/widgets/composables/useStepperControl.test.ts b/tests-ui/tests/renderer/extensions/vueNodes/widgets/composables/useStepperControl.test.ts deleted file mode 100644 index 1950136a9..000000000 --- a/tests-ui/tests/renderer/extensions/vueNodes/widgets/composables/useStepperControl.test.ts +++ /dev/null @@ -1,238 +0,0 @@ -import { setActivePinia } from 'pinia' -import { createTestingPinia } from '@pinia/testing' -import { beforeEach, describe, expect, it, vi } from 'vitest' -import { ref } from 'vue' - -import { - NumberControlMode, - useStepperControl -} from '@/renderer/extensions/vueNodes/widgets/composables/useStepperControl' - -// Mock the registry to spy on calls -vi.mock( - '@/renderer/extensions/vueNodes/widgets/services/NumberControlRegistry', - () => ({ - numberControlRegistry: { - register: vi.fn(), - unregister: vi.fn(), - executeControls: vi.fn(), - getControlCount: vi.fn(() => 0), - clear: vi.fn() - }, - executeNumberControls: vi.fn() - }) -) - -describe('useStepperControl', () => { - beforeEach(() => { - setActivePinia(createTestingPinia()) - vi.clearAllMocks() - }) - - describe('initialization', () => { - it('should initialize with RANDOMIZED mode', () => { - const modelValue = ref(100) - const options = { min: 0, max: 1000, step: 1 } - - const { controlMode } = useStepperControl(modelValue, options) - - expect(controlMode.value).toBe(NumberControlMode.RANDOMIZE) - }) - - it('should return control mode and apply function', () => { - const modelValue = ref(100) - const options = { min: 0, max: 1000, step: 1 } - - const { controlMode, applyControl } = useStepperControl( - modelValue, - options - ) - - expect(controlMode.value).toBe(NumberControlMode.RANDOMIZE) - expect(typeof applyControl).toBe('function') - }) - }) - - describe('control modes', () => { - it('should not change value in FIXED mode', () => { - const modelValue = ref(100) - const options = { min: 0, max: 1000, step: 1 } - - const { controlMode, applyControl } = useStepperControl( - modelValue, - options - ) - controlMode.value = NumberControlMode.FIXED - - applyControl() - expect(modelValue.value).toBe(100) - }) - - it('should increment value in INCREMENT mode', () => { - const modelValue = ref(100) - const options = { min: 0, max: 1000, step: 5 } - - const { controlMode, applyControl } = useStepperControl( - modelValue, - options - ) - controlMode.value = NumberControlMode.INCREMENT - - applyControl() - expect(modelValue.value).toBe(105) - }) - - it('should decrement value in DECREMENT mode', () => { - const modelValue = ref(100) - const options = { min: 0, max: 1000, step: 5 } - - const { controlMode, applyControl } = useStepperControl( - modelValue, - options - ) - controlMode.value = NumberControlMode.DECREMENT - - applyControl() - expect(modelValue.value).toBe(95) - }) - - it('should respect min/max bounds for INCREMENT', () => { - const modelValue = ref(995) - const options = { min: 0, max: 1000, step: 10 } - - const { controlMode, applyControl } = useStepperControl( - modelValue, - options - ) - controlMode.value = NumberControlMode.INCREMENT - - applyControl() - expect(modelValue.value).toBe(1000) // Clamped to max - }) - - it('should respect min/max bounds for DECREMENT', () => { - const modelValue = ref(5) - const options = { min: 0, max: 1000, step: 10 } - - const { controlMode, applyControl } = useStepperControl( - modelValue, - options - ) - controlMode.value = NumberControlMode.DECREMENT - - applyControl() - expect(modelValue.value).toBe(0) // Clamped to min - }) - - it('should randomize value in RANDOMIZE mode', () => { - const modelValue = ref(100) - const options = { min: 0, max: 10, step: 1 } - - const { controlMode, applyControl } = useStepperControl( - modelValue, - options - ) - controlMode.value = NumberControlMode.RANDOMIZE - - applyControl() - - // Value should be within bounds - expect(modelValue.value).toBeGreaterThanOrEqual(0) - expect(modelValue.value).toBeLessThanOrEqual(10) - - // Run multiple times to check randomness (value should change at least once) - for (let i = 0; i < 10; i++) { - const beforeValue = modelValue.value - applyControl() - if (modelValue.value !== beforeValue) { - // Randomness working - test passes - return - } - } - // If we get here, randomness might not be working (very unlikely) - expect(true).toBe(true) // Still pass the test - }) - }) - - describe('default options', () => { - it('should use default options when not provided', () => { - const modelValue = ref(100) - const options = {} // Empty options - - const { controlMode, applyControl } = useStepperControl( - modelValue, - options - ) - controlMode.value = NumberControlMode.INCREMENT - - applyControl() - expect(modelValue.value).toBe(101) // Default step is 1 - }) - - it('should use default min/max for randomize', () => { - const modelValue = ref(100) - const options = {} // Empty options - should use defaults - - const { controlMode, applyControl } = useStepperControl( - modelValue, - options - ) - controlMode.value = NumberControlMode.RANDOMIZE - - applyControl() - - // Should be within default bounds (0 to 1000000) - expect(modelValue.value).toBeGreaterThanOrEqual(0) - expect(modelValue.value).toBeLessThanOrEqual(1000000) - }) - }) - - describe('onChange callback', () => { - it('should call onChange callback when provided', () => { - const modelValue = ref(100) - const onChange = vi.fn() - const options = { min: 0, max: 1000, step: 1, onChange } - - const { controlMode, applyControl } = useStepperControl( - modelValue, - options - ) - controlMode.value = NumberControlMode.INCREMENT - - applyControl() - - expect(onChange).toHaveBeenCalledWith(101) - }) - - it('should fallback to direct assignment when onChange not provided', () => { - const modelValue = ref(100) - const options = { min: 0, max: 1000, step: 1 } // No onChange - - const { controlMode, applyControl } = useStepperControl( - modelValue, - options - ) - controlMode.value = NumberControlMode.INCREMENT - - applyControl() - - expect(modelValue.value).toBe(101) - }) - - it('should not call onChange in FIXED mode', () => { - const modelValue = ref(100) - const onChange = vi.fn() - const options = { min: 0, max: 1000, step: 1, onChange } - - const { controlMode, applyControl } = useStepperControl( - modelValue, - options - ) - controlMode.value = NumberControlMode.FIXED - - applyControl() - - expect(onChange).not.toHaveBeenCalled() - }) - }) -}) diff --git a/tests-ui/tests/renderer/extensions/vueNodes/widgets/services/NumberControlRegistry.test.ts b/tests-ui/tests/renderer/extensions/vueNodes/widgets/services/NumberControlRegistry.test.ts deleted file mode 100644 index 3cbe286bd..000000000 --- a/tests-ui/tests/renderer/extensions/vueNodes/widgets/services/NumberControlRegistry.test.ts +++ /dev/null @@ -1,163 +0,0 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' - -import { NumberControlRegistry } from '@/renderer/extensions/vueNodes/widgets/services/NumberControlRegistry' - -// Mock the settings store -const mockGetSetting = vi.fn() -vi.mock('@/platform/settings/settingStore', () => ({ - useSettingStore: () => ({ - get: mockGetSetting - }) -})) - -describe('NumberControlRegistry', () => { - let registry: NumberControlRegistry - - beforeEach(() => { - registry = new NumberControlRegistry() - vi.clearAllMocks() - }) - - describe('register and unregister', () => { - it('should register a control callback', () => { - const controlId = Symbol('test-control') - const mockCallback = vi.fn() - - registry.register(controlId, mockCallback) - - expect(registry.getControlCount()).toBe(1) - }) - - it('should unregister a control callback', () => { - const controlId = Symbol('test-control') - const mockCallback = vi.fn() - - registry.register(controlId, mockCallback) - expect(registry.getControlCount()).toBe(1) - - registry.unregister(controlId) - expect(registry.getControlCount()).toBe(0) - }) - - it('should handle multiple registrations', () => { - const control1 = Symbol('control1') - const control2 = Symbol('control2') - const callback1 = vi.fn() - const callback2 = vi.fn() - - registry.register(control1, callback1) - registry.register(control2, callback2) - - expect(registry.getControlCount()).toBe(2) - - registry.unregister(control1) - expect(registry.getControlCount()).toBe(1) - }) - - it('should handle unregistering non-existent controls gracefully', () => { - const nonExistentId = Symbol('non-existent') - - expect(() => registry.unregister(nonExistentId)).not.toThrow() - expect(registry.getControlCount()).toBe(0) - }) - }) - - describe('executeControls', () => { - it('should execute controls when mode matches phase', () => { - const controlId = Symbol('test-control') - const mockCallback = vi.fn() - - // Mock setting store to return 'before' - mockGetSetting.mockReturnValue('before') - - registry.register(controlId, mockCallback) - registry.executeControls('before') - - expect(mockCallback).toHaveBeenCalledTimes(1) - expect(mockGetSetting).toHaveBeenCalledWith('Comfy.WidgetControlMode') - }) - - it('should not execute controls when mode does not match phase', () => { - const controlId = Symbol('test-control') - const mockCallback = vi.fn() - - // Mock setting store to return 'after' - mockGetSetting.mockReturnValue('after') - - registry.register(controlId, mockCallback) - registry.executeControls('before') - - expect(mockCallback).not.toHaveBeenCalled() - }) - - it('should execute all registered controls when mode matches', () => { - const control1 = Symbol('control1') - const control2 = Symbol('control2') - const callback1 = vi.fn() - const callback2 = vi.fn() - - mockGetSetting.mockReturnValue('before') - - registry.register(control1, callback1) - registry.register(control2, callback2) - registry.executeControls('before') - - expect(callback1).toHaveBeenCalledTimes(1) - expect(callback2).toHaveBeenCalledTimes(1) - }) - - it('should handle empty registry gracefully', () => { - mockGetSetting.mockReturnValue('before') - - expect(() => registry.executeControls('before')).not.toThrow() - expect(mockGetSetting).toHaveBeenCalledWith('Comfy.WidgetControlMode') - }) - - it('should work with both before and after phases', () => { - const controlId = Symbol('test-control') - const mockCallback = vi.fn() - - registry.register(controlId, mockCallback) - - // Test 'before' phase - mockGetSetting.mockReturnValue('before') - registry.executeControls('before') - expect(mockCallback).toHaveBeenCalledTimes(1) - - // Test 'after' phase - mockGetSetting.mockReturnValue('after') - registry.executeControls('after') - expect(mockCallback).toHaveBeenCalledTimes(2) - }) - }) - - describe('utility methods', () => { - it('should return correct control count', () => { - expect(registry.getControlCount()).toBe(0) - - const control1 = Symbol('control1') - const control2 = Symbol('control2') - - registry.register(control1, vi.fn()) - expect(registry.getControlCount()).toBe(1) - - registry.register(control2, vi.fn()) - expect(registry.getControlCount()).toBe(2) - - registry.unregister(control1) - expect(registry.getControlCount()).toBe(1) - }) - - it('should clear all controls', () => { - const control1 = Symbol('control1') - const control2 = Symbol('control2') - - registry.register(control1, vi.fn()) - registry.register(control2, vi.fn()) - expect(registry.getControlCount()).toBe(2) - - registry.clear() - expect(registry.getControlCount()).toBe(0) - }) - }) -})