Skip to content

Commit 629007b

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 629007b

File tree

3 files changed

+23
-9
lines changed

3 files changed

+23
-9
lines changed

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

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -358,17 +358,26 @@
358358
</script>
359359

360360
{#if isVisible}
361-
<div class="portal-wrap" use:portal={'body'}>
362-
<!-- svelte-ignore a11y_autofocus -->
361+
<!-- `use:focusable` must come before `use:portal`-->
362+
<div
363+
class="portal-wrap"
364+
use:focusable={{
365+
activate: true,
366+
dim: true,
367+
trap: true,
368+
onEsc: () => {
369+
close();
370+
return true;
371+
}
372+
}}
373+
use:portal={'body'}
374+
>
363375
<div
364376
data-testid={testId}
365377
bind:this={menuContainer}
366378
tabindex="-1"
367-
use:focusable={{ activate: true, isolate: true, focusable: true, dim: true, trap: true }}
368-
autofocus
369379
{onclick}
370380
{onkeypress}
371-
onkeydown={handleKeyNavigation}
372381
class="context-menu hide-native-scrollbar"
373382
class:top-oriented={side === 'top'}
374383
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)