Skip to content

Commit 5c72e1d

Browse files
committed
Use focus manager for navigation context menus
Removes custom keyboard navigation for context menus, and replaces it with the new focus manager functionality. Note that ContextMenuItem contains two focusables, one for navigation and one to support f-keys.
1 parent c7241a0 commit 5c72e1d

File tree

3 files changed

+23
-16
lines changed

3 files changed

+23
-16
lines changed

packages/ui/src/lib/components/ContextMenu.svelte

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -329,13 +329,6 @@
329329
return isVisible;
330330
}
331331
332-
function handleKeyNavigation(e: KeyboardEvent) {
333-
if (e.key === 'Escape') {
334-
e.preventDefault();
335-
close();
336-
}
337-
}
338-
339332
$effect(() => {
340333
if (!menuContainer) return;
341334
const config = { attributes: false, childList: true, subtree: true };
@@ -358,17 +351,26 @@
358351
</script>
359352

360353
{#if isVisible}
361-
<div class="portal-wrap" use:portal={'body'}>
362-
<!-- svelte-ignore a11y_autofocus -->
354+
<!-- `use:focusable` must come before `use:portal`-->
355+
<div
356+
class="portal-wrap"
357+
use:focusable={{
358+
activate: true,
359+
dim: true,
360+
trap: true,
361+
onEsc: () => {
362+
close();
363+
return true;
364+
}
365+
}}
366+
use:portal={'body'}
367+
>
363368
<div
364369
data-testid={testId}
365370
bind:this={menuContainer}
366371
tabindex="-1"
367-
use:focusable={{ activate: true, isolate: true, focusable: true, dim: true, trap: true }}
368-
autofocus
369372
{onclick}
370373
{onkeypress}
371-
onkeydown={handleKeyNavigation}
372374
class="context-menu hide-native-scrollbar"
373375
class:top-oriented={side === 'top'}
374376
class:bottom-oriented={side === 'bottom'}

packages/ui/src/lib/components/ContextMenuItem.svelte

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<script lang="ts">
22
import Icon from '$components/Icon.svelte';
33
import Tooltip from '$components/Tooltip.svelte';
4+
import { focusable } from '$lib/focus/focusable';
45
import { keysStringToArr } from '$lib/utils/hotkeys';
56
import { getContext } from 'svelte';
67
import type iconsJson from '@gitbutler/ui/data/icons.json';
@@ -20,6 +21,7 @@
2021
onclick: (e: MouseEvent) => void;
2122
testId?: string;
2223
tooltip?: string;
24+
autofocus?: boolean;
2325
}
2426
2527
const {
@@ -32,7 +34,8 @@
3234
keyboardShortcut,
3335
caption,
3436
testId,
35-
tooltip
37+
tooltip,
38+
autofocus
3639
}: Props = $props();
3740
3841
// Get submenu coordination context if available
@@ -62,14 +65,15 @@
6265
<button
6366
data-testid={testId}
6467
type="button"
65-
class="menu-item focus-state no-select"
68+
class="menu-item no-select"
6669
style:--item-height={caption ? 'auto' : '1.625rem'}
6770
class:disabled
6871
{disabled}
6972
onclick={handleClick}
7073
onmouseenter={handleMouseEnter}
74+
use:focusable={{ focusable: true }}
7175
>
72-
<div class="menu-item__content">
76+
<div class="menu-item__content" use:focusable={{ button: true, activate: autofocus }}>
7377
{#if icon}
7478
<div class="menu-item__icon">
7579
<Icon name={icon} />
@@ -119,6 +123,7 @@
119123
padding: 6px 8px;
120124
gap: 4px;
121125
border-radius: var(--radius-s);
126+
outline: none;
122127
color: var(--clr-text-1);
123128
text-align: left;
124129
cursor: pointer;

packages/ui/src/lib/focus/focusManager.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ export class FocusManager {
173173
}
174174

175175
if (options.activate) {
176-
this.setActiveNode(newNode);
176+
this.setActiveNode(this.findNavigableDescendant(newNode));
177177
}
178178
}
179179

0 commit comments

Comments
 (0)