Skip to content

Commit 38b07da

Browse files
claude[bot]claudegithub-actionschristian-byrneactions-user
authored
fix: keep locked widget hover color consistent with node color (Comfy-Org#14387)
<!-- ccr-slack-attribution --> _Requested by **Christian Byrne** · [Slack thread](https://comfy-organization.slack.com/archives/C0A4XMHANP3/p1784139879572499?thread_ts=1784139879.572499&cid=C0A4XMHANP3)_ ## Summary Hovering over a locked (connected-input) text or textarea widget on a custom-colored node showed a generic gray hover background instead of one consistent with the node's color. ## Changes - **What**: `WidgetInputText.vue` and `WidgetTextarea.vue` unconditionally applied `hover:bg-component-node-widget-background-hovered`, even when the field was read-only/disabled. That opaque hover color overrode the translucent `background-disabled` overlay locked fields use at rest — the overlay is what lets a node's custom color show through. The hover class is now gated behind `!isReadOnly`, matching the existing `not-disabled:hover` pattern already used by sibling widgets (`WidgetInputNumberSlider`, `WidgetSelectDefault`, `FormDropdownInput`). ## Review Focus - The fix removes the hover background entirely for locked fields rather than deriving a colored hover variant — this matches the existing convention for disabled/locked controls elsewhere in the same widget folder (no hover affordance on non-interactive fields), rather than introducing a new color-derivation mechanism. ## Testing - Added a Vitest unit test to `WidgetInputText.test.ts` and `WidgetTextarea.test.ts` asserting the hover class is present when editable and omitted when read-only/disabled (confirmed the new tests fail against the pre-fix component). - Added a Playwright e2e test (`browser_tests/tests/vueNodes/widgets/text/lockedWidgetHoverColor.spec.ts`) with a new workflow fixture (`browser_tests/assets/vueNodes/linked-string-widget-color.json`) that loads a custom-colored node with a widget locked via a connected input, and asserts its background color is unchanged on hover (unlike a normal editable widget's). ## Screenshots (if applicable) N/A — see the added e2e test for the behavior this fixes. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: github-actions <github-actions@github.com> Co-authored-by: bymyself <cbyrne@comfy.org> Co-authored-by: GitHub Action <action@github.com>
1 parent 6cde22b commit 38b07da

8 files changed

Lines changed: 203 additions & 2 deletions

File tree

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
{
2+
"id": "3d3c9d1a-5a0f-4b1e-9d9b-2e8d1c2f6a11",
3+
"revision": 0,
4+
"last_node_id": 2,
5+
"last_link_id": 1,
6+
"nodes": [
7+
{
8+
"id": 1,
9+
"type": "DevToolsNodeWithOnlyOptionalInput",
10+
"pos": [400, 100],
11+
"size": [270, 160],
12+
"flags": {},
13+
"order": 1,
14+
"mode": 0,
15+
"title": "Locked Widget Target",
16+
"inputs": [
17+
{
18+
"name": "text",
19+
"type": "STRING",
20+
"widget": { "name": "text" },
21+
"link": 1
22+
},
23+
{ "name": "clip", "type": "CLIP", "link": null, "shape": 7 }
24+
],
25+
"outputs": [],
26+
"properties": {
27+
"Node name for S&R": "DevToolsNodeWithOnlyOptionalInput"
28+
},
29+
"widgets_values": ["Locked multiline text"],
30+
"color": "#232",
31+
"bgcolor": "#353"
32+
},
33+
{
34+
"id": 2,
35+
"type": "PrimitiveStringMultiline",
36+
"pos": [24, 100],
37+
"size": [270, 140],
38+
"flags": {},
39+
"order": 0,
40+
"mode": 0,
41+
"title": "String Source",
42+
"inputs": [],
43+
"outputs": [{ "name": "STRING", "type": "STRING", "links": [1] }],
44+
"properties": {
45+
"Node name for S&R": "PrimitiveStringMultiline"
46+
},
47+
"widgets_values": ["locked_prefix"]
48+
}
49+
],
50+
"links": [[1, 2, 0, 1, 1, "STRING"]],
51+
"groups": [],
52+
"config": {},
53+
"extra": {
54+
"ds": {
55+
"scale": 1,
56+
"offset": [0, 0]
57+
},
58+
"frontendVersion": "1.45.4"
59+
},
60+
"version": 0.4
61+
}
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import {
2+
comfyExpect as expect,
3+
comfyPageFixture as test
4+
} from '@e2e/fixtures/ComfyPage'
5+
6+
test.describe(
7+
'Vue Locked Widget Hover Styling',
8+
{ tag: ['@vue-nodes', '@widget', '@screenshot'] },
9+
() => {
10+
test.beforeEach(async ({ comfyPage }) => {
11+
await comfyPage.workflow.loadWorkflow(
12+
'vueNodes/linked-string-widget-color'
13+
)
14+
})
15+
16+
test.afterEach(async ({ comfyPage }) => {
17+
await comfyPage.canvasOps.resetView()
18+
})
19+
20+
test('keeps a locked field on a custom-colored node the same on hover instead of turning generic gray', async ({
21+
comfyPage
22+
}) => {
23+
const targetNode = comfyPage.vueNodes.getNodeByTitle(
24+
'Locked Widget Target'
25+
)
26+
const lockedInput = comfyPage.vueNodes.getWidgetByName(
27+
'Locked Widget Target',
28+
'text'
29+
)
30+
// proxy check: covers both link-locked and hardcoded read_only cases
31+
await expect(lockedInput).toHaveAttribute('readonly', '')
32+
33+
// Hovering the locked field must not replace its (node-color-tinted)
34+
// background with the app's opaque generic hover gray, so the node's
35+
// custom color should still show through in the screenshot.
36+
//
37+
// The native `disabled` attribute on the locked textarea makes it
38+
// `pointer-events: none` (see Textarea.vue's `disabled:pointer-events-none`
39+
// class), so the browser never delivers pointer events to it directly —
40+
// hovering must instead target its parent, which is the actual element
41+
// carrying the hover background class this test is verifying.
42+
await lockedInput.locator('..').hover()
43+
await comfyPage.expectScreenshot(
44+
targetNode,
45+
'locked-widget-hover-color.png'
46+
)
47+
})
48+
49+
test('editable field on the same node still shows the generic hover background', async ({
50+
comfyPage
51+
}) => {
52+
const sourceNode = comfyPage.vueNodes.getNodeByTitle('String Source')
53+
const editableInput = comfyPage.vueNodes.getWidgetByName(
54+
'String Source',
55+
'value'
56+
)
57+
58+
await expect(editableInput).toBeVisible()
59+
await editableInput.hover()
60+
await comfyPage.expectScreenshot(
61+
sourceNode,
62+
'editable-widget-hover-color.png'
63+
)
64+
})
65+
}
66+
)
5.98 KB
Loading
5.82 KB
Loading

src/renderer/extensions/vueNodes/widgets/components/WidgetInputText.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,40 @@ describe('WidgetInputText Value Binding', () => {
170170
})
171171
})
172172

173+
describe('Locked Field Hover Styling', () => {
174+
// Tests assert on the Tailwind class name directly because that's the
175+
// mechanism being guarded — a rename would need a baseline update anyway.
176+
const HOVER_CLASS = 'hover:bg-component-node-widget-background-hovered'
177+
178+
it('omits the generic hover background class when the field is locked (read-only)', () => {
179+
const widget = createInputTextWidget('locked value', {
180+
read_only: true
181+
})
182+
renderComponent(widget, 'locked value')
183+
184+
const input = screen.getByRole('textbox')
185+
expect(input.className).not.toContain(HOVER_CLASS)
186+
})
187+
188+
it('omits the generic hover background class when the field is disabled by a link', () => {
189+
const widget = createInputTextWidget('linked value', {
190+
disabled: true
191+
})
192+
renderComponent(widget, 'linked value')
193+
194+
const input = screen.getByRole('textbox')
195+
expect(input.className).not.toContain(HOVER_CLASS)
196+
})
197+
198+
it('applies the generic hover background class when the field is editable', () => {
199+
const widget = createInputTextWidget('editable value')
200+
renderComponent(widget, 'editable value')
201+
202+
const input = screen.getByRole('textbox')
203+
expect(input.className).toContain(HOVER_CLASS)
204+
})
205+
})
206+
173207
describe('Edge Cases', () => {
174208
it('handles very long strings', async () => {
175209
const widget = createInputTextWidget('short')

src/renderer/extensions/vueNodes/widgets/components/WidgetInputText.vue

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212
:class="
1313
cn(
1414
WidgetInputBaseClass,
15-
'w-full px-4 hover:bg-component-node-widget-background-hovered',
15+
'w-full px-4',
16+
!isReadOnly && 'hover:bg-component-node-widget-background-hovered',
1617
size === 'large' ? 'py-3 text-sm' : 'py-2 text-xs',
1718
loading && 'pl-9'
1819
)

src/renderer/extensions/vueNodes/widgets/components/WidgetTextarea.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,44 @@ describe('WidgetTextarea Value Binding', () => {
220220
})
221221
})
222222

223+
describe('Locked Field Hover Styling', () => {
224+
// Tests assert on the Tailwind class name directly because that's the
225+
// mechanism being guarded — a rename would need a baseline update anyway.
226+
const HOVER_CLASS = 'hover:bg-component-node-widget-background-hovered'
227+
228+
it('omits the generic hover background class from the wrapper when read-only', () => {
229+
const widget = createTextareaWidget('locked value', {
230+
read_only: true
231+
})
232+
const { container } = renderComponent(widget, 'locked value')
233+
234+
// hover class lives on wrapper <div>, not the <textarea>
235+
// eslint-disable-next-line testing-library/no-node-access
236+
const wrapper = container.firstElementChild
237+
expect(wrapper?.className).not.toContain(HOVER_CLASS)
238+
})
239+
240+
it('omits the generic hover background class from the wrapper when disabled', () => {
241+
const widget = createTextareaWidget('linked value', { disabled: true })
242+
const { container } = renderComponent(widget, 'linked value')
243+
244+
// hover class lives on wrapper <div>, not the <textarea>
245+
// eslint-disable-next-line testing-library/no-node-access
246+
const wrapper = container.firstElementChild
247+
expect(wrapper?.className).not.toContain(HOVER_CLASS)
248+
})
249+
250+
it('applies the generic hover background class to the wrapper when editable', () => {
251+
const widget = createTextareaWidget('editable value')
252+
const { container } = renderComponent(widget, 'editable value')
253+
254+
// hover class lives on wrapper <div>, not the <textarea>
255+
// eslint-disable-next-line testing-library/no-node-access
256+
const wrapper = container.firstElementChild
257+
expect(wrapper?.className).toContain(HOVER_CLASS)
258+
})
259+
})
260+
223261
describe('Edge Cases', () => {
224262
it('handles very long text', async () => {
225263
const widget = createTextareaWidget('short')

src/renderer/extensions/vueNodes/widgets/components/WidgetTextarea.vue

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
<div
33
:class="
44
cn(
5-
'group relative rounded-lg transition-all focus-within:ring focus-within:ring-component-node-widget-background-highlighted hover:bg-component-node-widget-background-hovered',
5+
'group relative rounded-lg transition-all focus-within:ring focus-within:ring-component-node-widget-background-highlighted',
6+
!isReadOnly && 'hover:bg-component-node-widget-background-hovered',
67
widget.borderStyle
78
)
89
"

0 commit comments

Comments
 (0)