Skip to content

Commit 75e29c1

Browse files
alicodingclaude
andcommitted
fix: plain-click selection ring invisible -- Primer's [role="button"] focus reset beat the inner-element box-shadow
Live-reproduced, screenshot-confirmed: a plain click on a single card selected it with zero visible change. Primer's global focus reset, [role="button"]:focus:not(:focus-visible):not(.focus-visible) { box-shadow: none } (specificity 0,4,0), beats every ring rule of the shape :global(.react-flow__node.selected) .card (0,3,0) whenever the clicked node's inner div is :focus without :focus-visible -- exactly the state every mouse click leaves it in. The ring only ever survived on non-focused members of a multi-selection and under keyboard :focus-visible, which is why the prior computed-style e2e assertions passed while the live gesture was broken. role="button"/tabIndex pre-existed on main, so the collision itself is pre-existing, but the new click model makes plain-click-select the PRIMARY gesture, so it cannot ship this way. Moves the ring from the inner element to the React Flow node WRAPPER for all four node types, keyed off the type-scoped class React Flow already stamps per nodeTypes key (.react-flow__node-<type>.selected) -- the wrapper carries no role, so Primer's reset structurally cannot match it, rather than out-specificity-ing a reset that could always be bumped again. The matching border-radius moves with it so the ring follows each shape. The sticky's border-color flip stays on the inner element -- Primer's reset only touches box-shadow, so that half already survived. New e2e coverage pins the exact broken state: a plain click on a single card, then the wrapper's own computed box-shadow while that card still holds DOM focus (confirmed failing against the pre-fix selector, passing after). Existing multi-select ring assertions (atlas-select-group.spec.ts) now measure the wrapper instead of the inner card, for the same reason. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FW5GkkAG8du7tNdYLk2zSd
1 parent aebb343 commit 75e29c1

6 files changed

Lines changed: 88 additions & 24 deletions

frontend/e2e/atlas-gestures.spec.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { test, expect } from './fixtures/server'
22
import { groupCard, noteCard } from './fixtures/atlasCards'
33
import { clickCorner, zoomAllTheWayOut } from './fixtures/atlasBoard'
4+
import { contextMenu } from './fixtures/contextMenu'
45

56
// The click model (goal 0102's gesture table) + surface-scoped
67
// shortcuts (goal 0071 slice): plain click selects/replaces, a second
@@ -42,6 +43,47 @@ test('plain click selects (replacing any prior selection); a second click on the
4243
await expect(overlay).not.toBeVisible()
4344
})
4445

46+
test('a plain click leaves the selection ring visibly showing on the clicked card while it still holds DOM focus, for a note card and a sticky', async ({ page }) => {
47+
await page.goto('/')
48+
await page.getByRole('link', { name: 'Atlas' }).click()
49+
await expect(page.getByTestId('atlas-board')).toBeVisible()
50+
51+
// Regression: Primer's own [role="button"]:focus:not(:focus-visible)
52+
// reset zeroes any box-shadow scoped to that inner role="button"
53+
// element -- exactly the state a plain mouse click leaves the
54+
// clicked card in (focused, but never :focus-visible from a pointer
55+
// gesture), which made a single-card selection invisible. The ring
56+
// must show on the wrapper, immune to that reset, while the card is
57+
// still focused -- not just once focus moves elsewhere.
58+
const getting = noteCard(page, 'Getting started')
59+
await getting.click()
60+
expect(await page.evaluate(() => document.activeElement?.getAttribute('data-testid'))).toBe('atlas-note-card')
61+
const cardWrapper = selectedWrapper(page, getting)
62+
await expect(cardWrapper).toHaveCount(1)
63+
await expect.poll(() => cardWrapper.evaluate((el) => getComputedStyle(el).boxShadow)).not.toBe('none')
64+
65+
// A sticky note's own, separately-declared (heavier) ring rule.
66+
const board = page.getByTestId('atlas-board')
67+
await zoomAllTheWayOut(page)
68+
await page.keyboard.press('n')
69+
await clickCorner(board, 'top-right')
70+
const noteTA = page.getByTestId('atlas-sticky-textarea')
71+
await noteTA.fill('ZzE2eStickyRing')
72+
await noteTA.blur()
73+
const sticky = page.locator('[data-testid="atlas-sticky-note"]')
74+
await sticky.click()
75+
expect(await page.evaluate(() => document.activeElement?.getAttribute('data-testid'))).toBe('atlas-sticky-note')
76+
const stickyWrapper = page.locator('.react-flow__node.selected').filter({ has: sticky })
77+
await expect(stickyWrapper).toHaveCount(1)
78+
await expect.poll(() => stickyWrapper.evaluate((el) => getComputedStyle(el).boxShadow)).not.toBe('none')
79+
80+
// Cleanup (testing.md's within-file discipline).
81+
const menu = contextMenu(page)
82+
await sticky.click({ button: 'right' })
83+
await menu.getByText('Delete note', { exact: true }).click()
84+
await expect(sticky).toHaveCount(0)
85+
})
86+
4587
test('a real double-click reproduces the same select-then-commit outcome as two plain clicks, for both a leaf and a frame body', async ({ page }) => {
4688
await page.goto('/')
4789
await page.getByRole('link', { name: 'Atlas' }).click()

frontend/e2e/atlas-select-group.spec.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,9 +231,13 @@ test('atlas shift-click select: toggle membership, group via member right-click,
231231

232232
// Visible selection state (goal 0092 follow-up): both member nodes
233233
// carry a real, non-empty outline/ring, not just React Flow's own
234-
// unstyled .selected class.
235-
await expect.poll(() => cardA.evaluate((el) => getComputedStyle(el).boxShadow)).not.toBe('none')
236-
await expect.poll(() => cardB.evaluate((el) => getComputedStyle(el).boxShadow)).not.toBe('none')
234+
// unstyled .selected class. Measured on the wrapper (the ring's
235+
// own carrier), not the inner card -- Primer's [role="button"]
236+
// focus reset can zero a box-shadow scoped to the inner element.
237+
const cardAWrapper = selected.filter({ has: cardA })
238+
const cardBWrapper = selected.filter({ has: cardB })
239+
await expect.poll(() => cardAWrapper.evaluate((el) => getComputedStyle(el).boxShadow)).not.toBe('none')
240+
await expect.poll(() => cardBWrapper.evaluate((el) => getComputedStyle(el).boxShadow)).not.toBe('none')
237241

238242
// The selection tray replaces the creation tray while 2+ cards are
239243
// selected: count label, Group (2+ cards only), Delete, both with

frontend/src/atlas/AtlasGroupNode.module.css

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,14 @@
1414
cursor: pointer;
1515
}
1616

17-
/* Multi-selection outline (owner-caught follow-up to goal 0092): same
18-
box-shadow ring AtlasNoteCardNode.module.css uses, keyed off React
19-
Flow's own .selected on the node's outer wrapper -- never the
20-
frame's own border-width, so a selected frame's size never shifts. */
21-
:global(.react-flow__node.selected) .frame {
17+
/* Selection ring: on the React Flow node WRAPPER (the type-scoped
18+
class React Flow stamps per nodeTypes key), never an inner element
19+
-- see AtlasNoteCardNode.module.css's own header comment for why
20+
(Primer's [role="button"] focus reset would otherwise zero it right
21+
after every mouse click). Never the frame's own border-width
22+
either, so a selected frame's size never shifts. */
23+
:global(.react-flow__node-atlas-group.selected) {
24+
border-radius: 14px;
2225
box-shadow:
2326
0 0 0 2px var(--borderColor-accent-emphasis),
2427
0 0 0 5px var(--bgColor-accent-muted);

frontend/src/atlas/AtlasNoteCardNode.module.css

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,16 @@
1818
border-color: var(--borderColor-emphasis);
1919
}
2020

21-
/* Multi-selection outline (owner-caught follow-up to goal 0092): keyed
22-
off React Flow's own .selected class on the node's outer wrapper --
23-
box-shadow, not border-width, so it never shifts this card's fixed
24-
190x128 footprint or its own hit-testing box. */
25-
:global(.react-flow__node.selected) .card {
21+
/* Selection ring: on the React Flow node WRAPPER (the type-scoped
22+
class React Flow stamps per nodeTypes key), never an inner element.
23+
Primer's own [role="button"]:focus:not(:focus-visible) reset zeroes
24+
any box-shadow on an inner role="button" element the instant a
25+
plain click leaves it :focus without :focus-visible -- the normal
26+
state right after every mouse click, which is exactly when this
27+
ring needs to show. The wrapper carries no role, so that reset can
28+
never reach it. box-shadow, not border-width, so it never shifts
29+
this card's fixed 190x128 footprint or its own hit-testing box. */
30+
:global(.react-flow__node-atlas-note.selected) {
2631
border-radius: var(--borderRadius-medium);
2732
box-shadow:
2833
0 0 0 2px var(--borderColor-accent-emphasis),

frontend/src/atlas/AtlasRegionChipNode.module.css

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,13 @@
1515
outline-offset: 1px;
1616
}
1717

18-
/* Multi-selection outline (owner-caught follow-up to goal 0092): same
19-
box-shadow ring AtlasNoteCardNode.module.css uses, keyed off React
20-
Flow's own .selected on the node's outer wrapper. */
21-
:global(.react-flow__node.selected) .chip {
18+
/* Selection ring: on the React Flow node WRAPPER (the type-scoped
19+
class React Flow stamps per nodeTypes key), never an inner element
20+
-- see AtlasNoteCardNode.module.css's own header comment for why
21+
(Primer's [role="button"] focus reset would otherwise zero it right
22+
after every mouse click). */
23+
:global(.react-flow__node-atlas-region-chip.selected) {
24+
border-radius: 10px;
2225
box-shadow:
2326
0 0 0 2px var(--borderColor-accent-emphasis),
2427
0 0 0 5px var(--bgColor-accent-muted);

frontend/src/atlas/AtlasStickyNode.module.css

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,24 @@
1717
cursor: text;
1818
}
1919

20-
/* Multi-selection outline (owner-caught follow-up to goal 0092),
21-
heavier than the card ring on purpose: the sticky's attention-tinted
22-
ground swallows the 2px accent line cards get away with on white --
23-
a wider ring plus the border flipping to the accent keeps a selected
24-
note legible at board zoom. */
25-
:global(.react-flow__node.selected) .sticky {
26-
border-color: var(--borderColor-accent-emphasis);
20+
/* Selection ring: heavier than the card ring on purpose -- the
21+
sticky's attention-tinted ground swallows the 2px accent line cards
22+
get away with on white. The ring lives on the React Flow node
23+
WRAPPER (the type-scoped class React Flow stamps per nodeTypes
24+
key), never an inner element -- see AtlasNoteCardNode.module.css's
25+
own header comment for why (Primer's [role="button"] focus reset
26+
would otherwise zero it right after every mouse click). The border
27+
color flip stays on the inner element: Primer's reset only touches
28+
box-shadow, so this half already survives there. */
29+
:global(.react-flow__node-atlas-sticky.selected) {
30+
border-radius: var(--borderRadius-medium);
2731
box-shadow:
2832
0 0 0 3px var(--borderColor-accent-emphasis),
2933
0 0 0 7px var(--bgColor-accent-muted);
3034
}
35+
:global(.react-flow__node.selected) .sticky {
36+
border-color: var(--borderColor-accent-emphasis);
37+
}
3138

3239
/* A pointer click must not flash the webview's OS focus halo (the
3340
"blue flash" over the selection ring); keyboard focus keeps its

0 commit comments

Comments
 (0)