Compare commits

...

8 Commits

Author SHA1 Message Date
Glary-Bot
da92d0ccd3 fix: suppress UA focus-visible outline on selected Vue nodes
Adds focus:outline-none focus-visible:outline-none scoped to the
selected-state class on the LGraphNode root, so a selected node never
gains a second, browser-default focus ring on top of the existing
selection overlay (the bug captured by the regression test in #11814).

The suppression is intentionally scoped to isSelected only — unselected
but Tab-focused nodes still get the UA focus ring for keyboard a11y.

Removes the test.fail annotation from the regression test; with this
fix in place the post-shift screenshot matches the no-shift baseline
byte-for-byte and the test passes naturally as a positive guard.
2026-05-04 23:25:20 +00:00
Glary-Bot
2b30f3f355 test: clip outline snapshot tightly around the node
Per @christian-byrne: at canvas-wide capture (1280x720) the 1px UA
focus ring was visually invisible in the snapshot even though the
diff still detected it. Use the node's boundingBox + 16px padding as
a clip region so the regression is unambiguously visible in the PNG
diff (now 464x266) — the focus ring traces the node perimeter and is
clearly distinguishable from the no-shift baseline.
2026-05-04 23:09:23 +00:00
Glary-Bot
38c4811a0d test: use minimal single-Note workflow for outline regression
Per @christian-byrne review on #11814: the default workflow's
Load Checkpoint node currently shows error-border noise on the
unfixed HEAD because no checkpoint is available, which contaminates
the baseline screenshot.

Adds a new asset browser_tests/assets/nodes/single_note.json with one
plain Note node (no loaders, no model dependencies, no error
rendering) and switches outline.spec.ts to load it. Baseline
regenerated against this clean workflow; with test.fail removed the
diff is now a tight 3125 px (0.3%) bbox around the Note's focus ring,
unambiguously isolating the Chromium :focus-visible regression.
2026-05-04 21:41:26 +00:00
Glary-Bot
29e82748a5 test: simplify outline regression — snapshot-only, rename file
Per @christian-byrne review on #11814:

- Rename file to outline.spec.ts to match sibling naming (bypass,
  colors, mute, pin, error, etc.)
- Drop the behavioral test that queried getComputedStyle(); rely on
  snapshot diff as the assertion (user-visible contract).
- Drop the try/finally wrapping keyboard.up('Shift'); Playwright
  handles cleanup between tests, matching the convention in
  zoom.spec.ts and linkInteraction.spec.ts.
- Move regression context out of the file docstring into a
  test.info().annotations.push({ type: 'regression', ... }) entry,
  matching the pattern in workflowPersistence.spec.ts.
- Drop 'Chrome-only' from the test title; the browserName-gated
  test.fail and the regression annotation describe the scope.
- Capture the baseline as the *desired* (no-shift) state so the test
  body fails today (Shift adds a focus ring that diffs from the
  baseline) and passes naturally once the bug is fixed; test.fail
  flips today's failure to expected-pass for green CI.
2026-05-04 20:08:22 +00:00
Glary-Bot
4ebfecb42e test: trim regression docstring
Per CodeRabbit nitpick on #11814 and AGENTS.md guidance to avoid
non-essential comments. Keep only the minimum needed to explain why
test.fail() is used and the upgrade path when the bug is fixed; drop
PR/bug references and backstory recoverable from git blame.
2026-05-02 02:27:44 +00:00
Glary-Bot
b0347f9c21 test: capture full canvas and assert only user-visible outline
Per Oracle review feedback:

- Screenshot the canvas, not the node locator: the regression is a
  focus ring drawn outside the node element box, which Playwright was
  clipping when capturing the locator. Switching to comfyPage.canvas
  matches the convention in bypass/colors/mute specs and captures the
  ring on visible sides.
- Drop the :focus-visible pseudo-class assertion: assert only
  outlineStyle === 'none' so the test guards the user-visible
  contract (no extra outline) and a future fix that removes the
  outline by other means still passes.

Snapshot baseline regenerated with the wider capture region.
2026-05-02 02:22:44 +00:00
Glary-Bot
f7590bc2c9 test: gate :focus-visible test.fail to chromium only
Per CodeRabbit review on #11814: the bug is Chrome-only, but the
test.fail() declaration was unconditional. Future addition of a WebKit
or Firefox project to the Playwright matrix would have caused unexpected
passes there. Move the annotation inside the test body and gate it on
browserName === 'chromium' using Playwright's documented runtime form.
2026-05-02 02:13:29 +00:00
Glary-Bot
d4fd92f8db test: add Playwright regression test for Shift focus-visible outline on Vue nodes
Captures the Chrome-only bug where pressing Shift on a selected Vue
node shows a second, browser-default focus outline on top of the
selection overlay. Triggered by tabindex="0" on the LGraphNode root
combined with Chromium's :focus-visible heuristic, which Safari/WebKit
doesn't share.

Behavior was introduced in #9360 (replaced CSS outline with layered
border overlay).

Two complementary tests:
- Screenshot test runs unconditionally and snapshots the trigger point
  to catch unrelated visual regressions.
- Behavioral test asserts the intended post-fix state (no
  :focus-visible, no UA outline) and is annotated test.fail(); it is
  expected to fail until the bug is fixed, at which point Playwright
  will surface the unexpected pass and prompt removal of the
  annotation alongside snapshot regeneration.
2026-05-01 03:41:34 +00:00
4 changed files with 68 additions and 1 deletions

View File

@@ -0,0 +1,24 @@
{
"last_node_id": 1,
"last_link_id": 0,
"nodes": [
{
"id": 1,
"type": "Note",
"pos": [200, 200],
"size": [240, 100],
"flags": {},
"order": 0,
"mode": 0,
"inputs": [],
"outputs": [],
"properties": {},
"widgets_values": ["Selected"]
}
],
"links": [],
"groups": [],
"config": {},
"extra": {},
"version": 0.4
}

View File

@@ -0,0 +1,42 @@
import {
comfyExpect as expect,
comfyPageFixture as test
} from '@e2e/fixtures/ComfyPage'
test.describe(
'Vue Node selection outline',
{ tag: ['@vue-nodes', '@screenshot'] },
() => {
test('does not gain an extra focus outline when Shift is held on a selected node', async ({
comfyPage
}) => {
test.info().annotations.push({
type: 'regression',
description:
'Chromium paints a UA :focus-visible outline on a focused element after a bare Shift keypress; the LGraphNode root has tabindex="0", so a selected Vue node would otherwise gain a second outline. The selected-state class adds focus-visible:outline-none on the node root to suppress the UA outline, while leaving the focus ring intact for unselected (Tab-focused) nodes for keyboard a11y.'
})
await comfyPage.workflow.loadWorkflow('nodes/single_note')
const note = comfyPage.vueNodes.getNodeByTitle('Note').first()
await note.locator('.lg-node-header').click()
await expect(note).toHaveClass(/outline-node-component-outline/)
const box = await note.boundingBox()
if (!box) throw new Error('Node bounding box not available')
const PAD = 16
const clip = {
x: Math.max(0, Math.floor(box.x - PAD)),
y: Math.max(0, Math.floor(box.y - PAD)),
width: Math.ceil(box.width + PAD * 2),
height: Math.ceil(box.height + PAD * 2)
}
await comfyPage.page.keyboard.down('Shift')
await expect(comfyPage.page).toHaveScreenshot(
'vue-node-shift-focus-outline.png',
{ clip }
)
})
}
)

Binary file not shown.

After

Width:  |  Height:  |  Size: 9.8 KiB

View File

@@ -17,7 +17,8 @@
? 'h-(--node-height)'
: 'min-h-(--node-height) min-w-(--min-node-width)',
cursorClass,
isSelected && 'outline-node-component-outline',
isSelected &&
'outline-node-component-outline focus:outline-none focus-visible:outline-none',
executing && 'outline-node-stroke-executing',
shouldHandleNodePointerEvents &&
!nodeData.flags?.ghost &&