From 550a9d04c59d711eb419629e6a5ba4a6c723aaa8 Mon Sep 17 00:00:00 2001 From: bymyself Date: Sun, 9 Feb 2025 18:18:09 -0700 Subject: [PATCH] Fix color and slider settings types to inherit attrs (#2483) --- browser_tests/extensionAPI.spec.ts | 85 +++++++++++++++++++ .../fixtures/components/SettingDialog.ts | 4 + src/components/common/FormColorPicker.vue | 6 +- src/components/common/InputSlider.vue | 5 ++ 4 files changed, 99 insertions(+), 1 deletion(-) diff --git a/browser_tests/extensionAPI.spec.ts b/browser_tests/extensionAPI.spec.ts index 1c1e4184a0..37d6dee20a 100644 --- a/browser_tests/extensionAPI.spec.ts +++ b/browser_tests/extensionAPI.spec.ts @@ -1,5 +1,6 @@ import { expect } from '@playwright/test' +import { SettingParams } from '../src/types/settingTypes' import { comfyPageFixture as test } from './fixtures/ComfyPage' test.describe('Topbar commands', () => { @@ -134,6 +135,90 @@ test.describe('Topbar commands', () => { expect(await comfyPage.getSetting('Comfy.TestSetting')).toBe(true) expect(await comfyPage.page.evaluate(() => window['changeCount'])).toBe(2) }) + + test.describe('Passing through attrs to setting components', () => { + const testCases: Array<{ + config: Partial + selector: string + }> = [ + { + config: { + type: 'boolean', + defaultValue: true + }, + selector: '.p-toggleswitch.p-component' + }, + { + config: { + type: 'number', + defaultValue: 10 + }, + selector: '.p-inputnumber input' + }, + { + config: { + type: 'slider', + defaultValue: 10 + }, + selector: '.p-slider.p-component' + }, + { + config: { + type: 'combo', + defaultValue: 'foo', + options: ['foo', 'bar', 'baz'] + }, + selector: '.p-select.p-component' + }, + { + config: { + type: 'text', + defaultValue: 'Hello' + }, + selector: '.p-inputtext' + }, + { + config: { + type: 'color', + defaultValue: '#000000' + }, + selector: '.p-colorpicker-preview' + } + ] as const + + for (const { config, selector } of testCases) { + test(`${config.type} component should respect disabled attr`, async ({ + comfyPage + }) => { + await comfyPage.page.evaluate((config) => { + window['app'].registerExtension({ + name: 'TestExtension1', + settings: [ + { + id: 'Comfy.TestSetting', + name: 'Test', + // The `disabled` attr is common to all settings components + attrs: { disabled: true }, + ...config + } + ] + }) + }, config) + + await comfyPage.settingDialog.open() + const component = comfyPage.settingDialog.root + .getByText('TestSetting Test') + .locator(selector) + + const isDisabled = await component.evaluate((el) => + el.tagName === 'INPUT' + ? (el as HTMLInputElement).disabled + : el.classList.contains('p-disabled') + ) + expect(isDisabled).toBe(true) + }) + } + }) }) test.describe('About panel', () => { diff --git a/browser_tests/fixtures/components/SettingDialog.ts b/browser_tests/fixtures/components/SettingDialog.ts index 328d89de33..82d47e78e0 100644 --- a/browser_tests/fixtures/components/SettingDialog.ts +++ b/browser_tests/fixtures/components/SettingDialog.ts @@ -3,6 +3,10 @@ import { Page } from '@playwright/test' export class SettingDialog { constructor(public readonly page: Page) {} + get root() { + return this.page.locator('div.settings-container') + } + async open() { const button = this.page.locator('button.comfy-settings-btn:visible') await button.click() diff --git a/src/components/common/FormColorPicker.vue b/src/components/common/FormColorPicker.vue index db7df5ce7b..a5cd319a32 100644 --- a/src/components/common/FormColorPicker.vue +++ b/src/components/common/FormColorPicker.vue @@ -1,6 +1,6 @@ @@ -14,4 +14,8 @@ defineProps<{ defaultValue?: string label?: string }>() + +defineOptions({ + inheritAttrs: false +}) diff --git a/src/components/common/InputSlider.vue b/src/components/common/InputSlider.vue index 1c9bda36e4..4a1284967c 100644 --- a/src/components/common/InputSlider.vue +++ b/src/components/common/InputSlider.vue @@ -8,6 +8,7 @@ :min="min" :max="max" :step="step" + v-bind="$attrs" /> { localValue.value = newValue emit('update:modelValue', newValue) } + +defineOptions({ + inheritAttrs: false +})