Skip to content

Commit 960a38b

Browse files
fix(focus-trap): align containment contract with implementation (#307)
* fix(focus-trap): align containment contract with implementation * test(focus-trap): address review feedback
1 parent 0d32cbd commit 960a38b

File tree

3 files changed

+47
-11
lines changed

3 files changed

+47
-11
lines changed

docs/widgets/catalog.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -241,9 +241,9 @@
241241
},
242242
{
243243
"name": "focusTrap",
244-
"requiredProps": ["id"],
245-
"commonProps": ["children"],
246-
"example": "ui.focusTrap({ id: 'trap' }, [ui.input('q', '')])"
244+
"requiredProps": ["id", "active"],
245+
"commonProps": ["initialFocus", "returnFocusTo", "children"],
246+
"example": "ui.focusTrap({ id: 'trap', active: true }, [ui.input({ id: 'q', value: '' })])"
247247
},
248248
{
249249
"name": "virtualList",

docs/widgets/focus-trap.md

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,34 @@ Constrains focus within a subtree when active. Used by modals and overlays to pr
88
ui.focusTrap(
99
{ id: "modal-trap", active: state.modalOpen },
1010
[
11-
ui.text("Are you sure?"),
12-
ui.button({ id: "confirm", label: "Confirm" }),
13-
ui.button({ id: "cancel", label: "Cancel" }),
11+
ui.column({ gap: 1 }, [
12+
ui.text("Are you sure?"),
13+
ui.row({ gap: 1 }, [
14+
ui.button({ id: "confirm", label: "Confirm" }),
15+
ui.button({ id: "cancel", label: "Cancel" }),
16+
]),
17+
]),
1418
]
1519
)
1620
```
1721

1822
## Layout behavior
1923

20-
`focusTrap` is layout-transparent and does not impose a hidden column layout.
21-
Children keep their native layout semantics (`row`, `column`, `grid`, etc.).
24+
`focusTrap` is layout-transparent when it wraps a single child.
25+
That child keeps its native layout semantics (`row`, `column`, `grid`, etc.).
26+
27+
For legacy multi-child usage, direct children fall back to an implicit column layout.
28+
Prefer an explicit container inside the trap when you need more than one child:
29+
30+
```typescript
31+
ui.focusTrap(
32+
{ id: "trap-legacy", active: true },
33+
[
34+
ui.button({ id: "a", label: "A" }),
35+
ui.button({ id: "b", label: "B" }),
36+
]
37+
)
38+
```
2239

2340
When you want explicit arrangement, compose a normal layout container inside the trap:
2441

@@ -48,12 +65,14 @@ ui.focusTrap(
4865

4966
When `active` is `true`:
5067

51-
- Tab/Shift+Tab cycles only through focusable elements inside the trap
52-
- Focus cannot escape to elements outside the trap
68+
- If the trap contains focusable elements, Tab/Shift+Tab cycles only through those elements
69+
- Containment is focus-system based; `focusTrap` alone does not define an outside click boundary
5370
- Attempting to Tab past the last element wraps to the first
5471
- Attempting to Shift+Tab before the first element wraps to the last
5572
- The trap is collected into the focus system (`CollectedTrap`) so modal overlays
5673
consistently block/contain background focus.
74+
- If the active trap has no focusable descendants, it preserves the current focus unless
75+
`initialFocus` names a valid nested target.
5776

5877
When `active` becomes `false`:
5978

@@ -128,7 +147,8 @@ ui.focusTrap({ id: "outer", active: true }, [
128147

129148
## Mouse Behavior
130149

131-
When a focus trap is active, mouse clicks outside the trap region are blocked. Clicking widgets inside the trap works normally — the clicked widget receives focus.
150+
`focusTrap` does not block outside clicks by itself.
151+
Use it with `ui.modal()` or a layer-backed overlay when you need pointer containment as well as Tab containment.
132152

133153
## Related
134154

packages/core/src/layout/__tests__/layout.focusZone.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,4 +97,20 @@ describe("focusZone/focusTrap layout transparency", () => {
9797
assert.equal(first.rect.y, second.rect.y);
9898
assert.equal(second.rect.x > first.rect.x, true);
9999
});
100+
101+
test("focusTrap with multiple direct children keeps legacy column fallback", () => {
102+
const vnode = ui.focusTrap({ id: "trap-legacy", active: true }, [
103+
ui.button({ id: "trap-legacy-a", label: "A" }),
104+
ui.button({ id: "trap-legacy-b", label: "B" }),
105+
]);
106+
107+
const laidOut = mustLayout(vnode, 20, 8);
108+
const first = laidOut.children[0];
109+
const second = laidOut.children[1];
110+
assert.ok(first && second, "legacy fallback should lay out both direct children");
111+
if (!first || !second) return;
112+
113+
assert.equal(second.rect.y > first.rect.y, true);
114+
assert.equal(second.rect.x, first.rect.x);
115+
});
100116
});

0 commit comments

Comments
 (0)