From cf64e5baa8099446fa2310d48b5d2d791b12f2e2 Mon Sep 17 00:00:00 2001 From: Will Miao Date: Fri, 4 Sep 2026 19:02:16 +0800 Subject: [PATCH] fix(ui): remove per-node active-filters chip from loras widgets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The indicator chip added for /activefilters discoverability was broken by design of its import path: AutocompleteTextWidget.vue imported web/comfyui/settings.js into the vue-widgets bundle, and settings.js's "../../scripts/app.js" import resolved at build time to the repo-root test shim (scripts/app.js, an in-memory settings store). The chip therefore read and wrote an orphaned in-memory Map: clicking it flipped only its own visual state and never touched the real ComfyUI setting that autocomplete.js consults (use_active_filters query param). Beyond the defect, a persistent per-node control for a global persisted setting misleads users and needs cross-instance sync machinery, which the footer hint, slash commands, right-click menu entry and settings dialog already cover. - AutocompleteTextWidget.vue: remove the chip button, its state/handlers, the settings.js import (the shim-inlining pathway) and all chip styles - AutocompleteTextWidget.test.ts: drop the chip indicator describe block and the settings.js module mock; beforeEach import no longer needed - settings.js: drop the lora-manager:setting-toggled window broadcast and its export — the chip was its only consumer, so every setLoraManagerSettingValue write no longer dispatches a dead event - autocomplete.activeFilters.test.js: drop the broadcast assertion test - loraLoader.activeFiltersMenu.test.js: drop SETTING_TOGGLED_EVENT_NAME from the settings.js mock Discoverability of /activefilters // /noactivefilters is unchanged: command-list footer, first-run hint, node context menu, settings dialog. --- .../autocomplete.activeFilters.test.js | 33 ----- .../loraLoader.activeFiltersMenu.test.js | 1 - .../src/components/AutocompleteTextWidget.vue | 122 +----------------- .../components/AutocompleteTextWidget.test.ts | 119 +---------------- web/comfyui/settings.js | 20 --- 5 files changed, 11 insertions(+), 284 deletions(-) diff --git a/tests/frontend/components/autocomplete.activeFilters.test.js b/tests/frontend/components/autocomplete.activeFilters.test.js index 594c7071..90d10816 100644 --- a/tests/frontend/components/autocomplete.activeFilters.test.js +++ b/tests/frontend/components/autocomplete.activeFilters.test.js @@ -253,37 +253,4 @@ describe('AutoComplete active-filters flag', () => { expect(autoComplete.dropdown.querySelector('.lm-autocomplete-first-run-hint')).toBeNull(); }); - - it('broadcasts a setting-toggled window event when /activefilters is accepted', async () => { - const events = []; - const listener = (event) => events.push(event.detail); - window.addEventListener('lora-manager:setting-toggled', listener); - try { - const input = document.createElement('textarea'); - input.value = '/activefilters'; - input.selectionStart = input.value.length; - input.focus = vi.fn(); - input.setSelectionRange = vi.fn(); - document.body.append(input); - - caretHelperInstance.getBeforeCursor.mockReturnValue('/activefilters'); - - const { AutoComplete } = await import(AUTOCOMPLETE_MODULE); -const autoComplete = new AutoComplete(input, 'loras', { showPreview: false, minChars: 1 }); - input.dispatchEvent(new Event('input', { bubbles: true })); - - // The command token is cleared after acceptance; simulate the caret - // helper seeing the cleared input so the synthetic input event does - // not re-trigger command parsing (same pattern as behavior tests). - caretHelperInstance.getBeforeCursor.mockReturnValue(''); - await Promise.resolve(); - await Promise.resolve(); - expect(events).toContainEqual({ - settingId: 'loramanager.lora_active_filters_autocomplete', - value: true, - }); - } finally { - window.removeEventListener('lora-manager:setting-toggled', listener); - } - }); }); diff --git a/tests/frontend/components/loraLoader.activeFiltersMenu.test.js b/tests/frontend/components/loraLoader.activeFiltersMenu.test.js index ffe422a1..a78805bf 100644 --- a/tests/frontend/components/loraLoader.activeFiltersMenu.test.js +++ b/tests/frontend/components/loraLoader.activeFiltersMenu.test.js @@ -54,7 +54,6 @@ const setSettingValueMock = vi.fn(); vi.mock(SETTINGS_MODULE, () => ({ LORA_ACTIVE_FILTERS_AUTOCOMPLETE_SETTING_ID: "loramanager.lora_active_filters_autocomplete", - SETTING_TOGGLED_EVENT_NAME: "lora-manager:setting-toggled", getLoraActiveFiltersAutocompletePreference: getActiveFiltersPreferenceMock, setLoraManagerSettingValue: setSettingValueMock, })); diff --git a/vue-widgets/src/components/AutocompleteTextWidget.vue b/vue-widgets/src/components/AutocompleteTextWidget.vue index 13546cc2..a9010188 100644 --- a/vue-widgets/src/components/AutocompleteTextWidget.vue +++ b/vue-widgets/src/components/AutocompleteTextWidget.vue @@ -27,18 +27,6 @@ - @@ -46,8 +34,6 @@ @@ -484,58 +422,6 @@ onUnmounted(() => { height: 12px; } -/* Active-filters search indicator (loras nodes only) */ -.active-filters-toggle { - position: absolute; - top: 3px; - right: calc(3px + var(--lm-vscrollbar-width, 0px)); - width: 16px; - height: 16px; - padding: 2px; - margin: 0; - border: none; - border-radius: 4px; - background: rgba(128, 128, 128, 0.25); - color: rgba(255, 255, 255, 0.5); - cursor: pointer; - display: flex; - align-items: center; - justify-content: center; - opacity: 0.7; - transition: opacity 0.2s ease, background-color 0.2s ease, color 0.2s ease; - z-index: 10; -} - -.active-filters-toggle:hover { - opacity: 1; - background: rgba(128, 128, 128, 0.45); - color: rgba(255, 255, 255, 0.85); -} - -.active-filters-toggle.is-active { - background: rgba(59, 130, 246, 0.35); - color: #7db8ff; - opacity: 1; -} - -.active-filters-toggle svg { - width: 11px; - height: 11px; -} - -/* Vue DOM mode adjustments for the indicator */ -.text-input.vue-dom-mode ~ .active-filters-toggle { - top: 8px; - right: calc(8px + var(--lm-vscrollbar-width, 0px)); - width: 20px; - height: 20px; -} - -.text-input.vue-dom-mode ~ .active-filters-toggle svg { - width: 13px; - height: 13px; -} - /* Vue DOM mode adjustments for clear button */ .text-input.vue-dom-mode ~ .clear-button { right: calc(8px + var(--lm-vscrollbar-width, 0px)); diff --git a/vue-widgets/tests/components/AutocompleteTextWidget.test.ts b/vue-widgets/tests/components/AutocompleteTextWidget.test.ts index d89402c8..941cda0a 100644 --- a/vue-widgets/tests/components/AutocompleteTextWidget.test.ts +++ b/vue-widgets/tests/components/AutocompleteTextWidget.test.ts @@ -10,7 +10,7 @@ import { nextTick } from 'vue' import { shallowMount } from '@vue/test-utils' -import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest' +import { describe, expect, it, vi, afterEach } from 'vitest' import AutocompleteTextWidget from '@/components/AutocompleteTextWidget.vue' function createMockWidget() { @@ -135,121 +135,15 @@ describe('AutocompleteTextWidget clear button', () => { }) }) -/** - * Tests for the active-filters search indicator (loras mode only). - * - * The small filter chip in the textarea corner mirrors the - * loramanager.lora_active_filters_autocomplete setting: it reflects the - * current state, can toggle it, and stays in sync with slash-command / - * context-menu toggles via the lora-manager:setting-toggled window event. - */ - -const settingsMocks = vi.hoisted(() => ({ - getPreference: vi.fn(), - setValue: vi.fn(), -})) - -vi.mock('../../../web/comfyui/settings.js', () => ({ - LORA_ACTIVE_FILTERS_AUTOCOMPLETE_SETTING_ID: - 'loramanager.lora_active_filters_autocomplete', - SETTING_TOGGLED_EVENT_NAME: 'lora-manager:setting-toggled', - getLoraActiveFiltersAutocompletePreference: settingsMocks.getPreference, - setLoraManagerSettingValue: settingsMocks.setValue, -})) - -const getActiveFiltersPreferenceMock = settingsMocks.getPreference -const setSettingValueMock = settingsMocks.setValue - -function mountLorasWidget() { - const widget = createMockWidget() - const node = { id: 1 } - const wrapper = shallowMount(AutocompleteTextWidget, { - props: { widget, node, modelType: 'loras' }, - attachTo: document.body, - }) - return { wrapper, widget } -} - -describe('AutocompleteTextWidget active-filters indicator', () => { - beforeEach(() => { - getActiveFiltersPreferenceMock.mockReset() - getActiveFiltersPreferenceMock.mockReturnValue(false) - setSettingValueMock.mockReset() - setSettingValueMock.mockResolvedValue(true) - }) - - it('renders only in loras mode', () => { - const loras = mountLorasWidget() - expect(loras.wrapper.find('.active-filters-toggle').exists()).toBe(true) - - const widget = createMockWidget() - const prompt = shallowMount(AutocompleteTextWidget, { - props: { widget, node: { id: 2 }, modelType: 'prompt' }, - attachTo: document.body, - }) - expect(prompt.find('.active-filters-toggle').exists()).toBe(false) - }) - - it('reflects the current setting state', async () => { - const { wrapper } = mountLorasWidget() - await nextTick() - expect(wrapper.find('.active-filters-toggle').classes()).not.toContain('is-active') - - getActiveFiltersPreferenceMock.mockReturnValue(true) - const wrapper2 = mountLorasWidget().wrapper - await nextTick() - expect(wrapper2.find('.active-filters-toggle').classes()).toContain('is-active') - }) - - it('toggles the setting when clicked', async () => { - const { wrapper } = mountLorasWidget() - await nextTick() - - await wrapper.find('.active-filters-toggle').trigger('click') - expect(setSettingValueMock).toHaveBeenCalledWith( - 'loramanager.lora_active_filters_autocomplete', - true - ) - await nextTick() - expect(wrapper.find('.active-filters-toggle').classes()).toContain('is-active') - }) - - it('stays in sync with setting-toggled window events', async () => { - const { wrapper } = mountLorasWidget() - await nextTick() - expect(wrapper.find('.active-filters-toggle').classes()).not.toContain('is-active') - - window.dispatchEvent( - new CustomEvent('lora-manager:setting-toggled', { - detail: { - settingId: 'loramanager.lora_active_filters_autocomplete', - value: true, - }, - }) - ) - await nextTick() - expect(wrapper.find('.active-filters-toggle').classes()).toContain('is-active') - - window.dispatchEvent( - new CustomEvent('lora-manager:setting-toggled', { - detail: { settingId: 'loramanager.some_other_setting', value: true }, - }) - ) - await nextTick() - // Unrelated settings must not flip the indicator - expect(wrapper.find('.active-filters-toggle').classes()).toContain('is-active') - }) -}) - /** * Tests for the vertical-scrollbar inset. * * When the textarea content overflows and a classic (non-overlay) scrollbar - * is shown, the absolutely-positioned corner buttons (clear x, active-filters - * filter chip) would sit on top of the scrollbar. The component measures the - * scrollbar gutter and exposes it as the --lm-vscrollbar-width CSS var on - * .input-wrapper so the buttons shift left of the scrollbar. jsdom does no - * layout, so overflow is simulated by overriding the scroll/dimension props. + * is shown, the absolutely-positioned corner clear (x) button would sit on + * top of the scrollbar. The component measures the scrollbar gutter and + * exposes it as the --lm-vscrollbar-width CSS var on .input-wrapper so the + * button shifts left of the scrollbar. jsdom does no layout, so overflow is + * simulated by overriding the scroll/dimension props. */ describe('AutocompleteTextWidget vertical scrollbar inset', () => { function overrideTextareaMetrics( @@ -322,4 +216,5 @@ describe('AutocompleteTextWidget vertical scrollbar inset', () => { await nextTick() expect(wrapperEl.style.getPropertyValue('--lm-vscrollbar-width')).toBe('0px') }) + }) diff --git a/web/comfyui/settings.js b/web/comfyui/settings.js index 66c382b2..ecc9c792 100644 --- a/web/comfyui/settings.js +++ b/web/comfyui/settings.js @@ -175,42 +175,23 @@ const getPromptTagAutocompletePreference = (() => { /** * Persist a LoRA Manager setting through ComfyUI's setting API. * Returns true when the setting was written successfully. - * - * Every successful write broadcasts a "lora-manager:setting-toggled" window - * event (see SETTING_TOGGLED_EVENT_NAME) so widgets mirroring the setting - * (e.g. the active-filters indicator in the autocomplete text widget) stay in - * sync with slash-command / context-menu toggles. */ -const SETTING_TOGGLED_EVENT_NAME = "lora-manager:setting-toggled"; - const setLoraManagerSettingValue = async (settingId, value) => { const settingManager = app?.extensionManager?.setting; if (settingManager && typeof settingManager.set === "function") { await settingManager.set(settingId, value); - _notifySettingToggled(settingId, value); return true; } const setting = app?.ui?.settings?.settingsById?.[settingId]; if (setting) { app.ui.settings.setSettingValue(settingId, value); - _notifySettingToggled(settingId, value); return true; } return false; }; -const _notifySettingToggled = (settingId, value) => { - try { - window.dispatchEvent(new CustomEvent(SETTING_TOGGLED_EVENT_NAME, { - detail: { settingId, value }, - })); - } catch (error) { - // Best-effort notification; ignore non-browser environments - } -}; - const getAutocompleteAppendCommaPreference = (() => { let settingsUnavailableLogged = false; @@ -617,7 +598,6 @@ app.registerExtension({ export { PROMPT_TAG_AUTOCOMPLETE_SETTING_ID, LORA_ACTIVE_FILTERS_AUTOCOMPLETE_SETTING_ID, - SETTING_TOGGLED_EVENT_NAME, getWheelSensitivity, getAutoPathCorrectionPreference, getAutocompleteAppendCommaPreference,