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.

Author: mtsgrd

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

@@ -329,13 +329,6 @@
 		return isVisible;
 	}
 
-	function handleKeyNavigation(e: KeyboardEvent) {
-		if (e.key === 'Escape') {
-			e.preventDefault();
-			close();
-		}
-	}
-
 	$effect(() => {
 		if (!menuContainer) return;
 		const config = { attributes: false, childList: true, subtree: true };
@@ -358,17 +351,26 @@
 </script>
 
 {#if isVisible}
-	<div class="portal-wrap" use:portal={'body'}>
-		<!-- svelte-ignore a11y_autofocus -->
+	<!-- `use:focusable` must come before `use:portal`-->
+	<div
+		class="portal-wrap"
+		use:focusable={{
+			activate: true,
+			trap: true,
+			vertical: true,
+			onEsc: () => {
+				close();
+				return true;
+			}
+		}}
+		use:portal={'body'}
+	>
 		<div
 			data-testid={testId}
 			bind:this={menuContainer}
 			tabindex="-1"
-			use:focusable={{ activate: true, isolate: true, focusable: true, dim: true, trap: true }}
-			autofocus
 			{onclick}
 			{onkeypress}
-			onkeydown={handleKeyNavigation}
 			class="context-menu hide-native-scrollbar"
 			class:top-oriented={side === 'top'}
 			class:bottom-oriented={side === 'bottom'}

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

@@ -1,6 +1,7 @@
 <script lang="ts">
 	import Icon from '$components/Icon.svelte';
 	import Tooltip from '$components/Tooltip.svelte';
+	import { focusable } from '$lib/focus/focusable';
 	import { keysStringToArr } from '$lib/utils/hotkeys';
 	import { getContext } from 'svelte';
 	import type iconsJson from '@gitbutler/ui/data/icons.json';
@@ -62,12 +63,13 @@
 	<button
 		data-testid={testId}
 		type="button"
-		class="menu-item focus-state no-select"
+		class="menu-item no-select"
 		style:--item-height={caption ? 'auto' : '1.625rem'}
 		class:disabled
 		{disabled}
 		onclick={handleClick}
 		onmouseenter={handleMouseEnter}
+		use:focusable={{ focusable: true, button: true }}
 	>
 		<div class="menu-item__content">
 			{#if icon}
@@ -119,6 +121,7 @@
 		padding: 6px 8px;
 		gap: 4px;
 		border-radius: var(--radius-s);
+		outline: none;
 		color: var(--clr-text-1);
 		text-align: left;
 		cursor: pointer;

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

@@ -110,39 +110,6 @@
 		isSubmenuOpen = false;
 		contextMenu?.close();
 	}
-
-	// Handle arrow key navigation
-	function handleKeyDown(e: KeyboardEvent) {
-		if (disabled) return;
-
-		switch (e.key) {
-			case 'ArrowRight':
-				if (submenuSide === 'right') {
-					e.preventDefault();
-					e.stopPropagation();
-					isSubmenuOpen = true;
-					contextMenu?.open();
-				}
-				break;
-			case 'ArrowLeft':
-				if (submenuSide === 'left') {
-					e.preventDefault();
-					e.stopPropagation();
-					isSubmenuOpen = true;
-					contextMenu?.open();
-				}
-				break;
-			case 'Enter':
-			case ' ':
-				e.preventDefault();
-				e.stopPropagation();
-				if (disabled) return;
-
-				isSubmenuOpen = !isSubmenuOpen;
-				contextMenu?.toggle();
-				break;
-		}
-	}
 </script>
 
 <div
@@ -151,11 +118,7 @@
 	class:active={isSubmenuOpen}
 	onmouseenter={handleMouseEnter}
 	onmouseleave={handleMouseLeave}
-	onkeydown={handleKeyDown}
-	role="menuitem"
-	aria-haspopup="menu"
-	aria-expanded={isSubmenuOpen}
-	tabindex="-1"
+	role="group"
 >
 	<ContextMenuItem
 		{icon}
@@ -182,7 +145,6 @@
 	align={submenuVerticalAlign === 'top' ? 'start' : 'end'}
 	onclose={() => {
 		isSubmenuOpen = false;
-		menuItemElement?.focus();
 	}}
 >
 	{@render submenu({ close: closeSubmenu })}

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

@@ -68,6 +68,17 @@
 		onclick?.(e.currentTarget as HTMLElement);
 	}
 
+	function onKeyDown(e: KeyboardEvent) {
+		if (e.key !== 'Enter') {
+			return;
+		}
+		e.stopPropagation();
+		e.preventDefault();
+		isContextMenuOpen = !isContextMenuOpen;
+		openedViaClick = isContextMenuOpen; // Track if opened via click
+		onclick?.(e.currentTarget as HTMLElement);
+	}
+
 	$effect(() => {
 		if (contextElement) {
 			contextElement.addEventListener('contextmenu', onContextMenu);
@@ -110,6 +121,7 @@
 		class:visible={visible || isContextMenuOpen}
 		class:activated={activated || (isContextMenuOpen && openedViaClick)}
 		onclick={onClick}
+		onkeypress={onKeyDown}
 		data-testid={testId}
 	>
 		<Icon name="kebab" />
@@ -123,6 +135,7 @@
 		kind="ghost"
 		activated={activated || (isContextMenuOpen && openedViaClick)}
 		onclick={onClick}
+		onkeydown={onKeyDown}
 	/>
 {/if}
 

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

@@ -154,7 +154,7 @@ export class FocusManager {
 
 		this.unregisterElement(element);
 
-		const parentNode = options.isolate ? undefined : this.findParentNode(element);
+		const parentNode = this.findParentNode(element);
 
 		const newNode: FocusableNode = {
 			element,
@@ -166,14 +166,14 @@ export class FocusManager {
 		this.nodeMap.set(element, newNode);
 		this.establishParentChildRelationships(element, newNode, parentNode);
 		this.resolvePendingRelationships();
-		this.handlePendingRegistration(element, parentNode, options);
+		this.handlePendingRegistration(element, parentNode);
 
 		if (this.fModeManager.active) {
 			this.fModeManager.addElement(element, newNode);
 		}
 
 		if (options.activate) {
-			this.setActiveNode(newNode);
+			this.setActiveNode(this.findNavigableDescendant(newNode));
 		}
 	}
 
@@ -254,7 +254,7 @@ export class FocusManager {
 			(forward && currentIndex === navigableSiblings.length - 1) ||
 			(!forward && currentIndex === 0);
 
-		if (isAtBoundary) {
+		if (isAtBoundary && !parentNode.options.trap) {
 			// Traverse parents while vertical: true
 			const linkedTarget = this.findNextInColumnNode(currentNode, forward);
 			if (!linkedTarget) return false;
@@ -266,7 +266,7 @@ export class FocusManager {
 
 	focusNextVertical(forward: boolean): boolean {
 		if (!this.currentNode?.element) return false;
-		const nextChild = this.findInNextColumn(this.currentNode.element, forward);
+		const nextChild = this.findInNextColumn(this.currentNode, forward);
 		if (nextChild) {
 			return this.setActiveNode(nextChild);
 		}
@@ -326,6 +326,9 @@ export class FocusManager {
 				if (navigableChild) {
 					return navigableChild;
 				}
+				if (node.options.trap) {
+					return;
+				}
 			}
 			pointer = pointer.parentElement;
 		}
@@ -521,7 +524,7 @@ export class FocusManager {
 			case 'right':
 				if (trap) return true;
 				if (!this.focusNextVertical(true)) {
-					this.focusAnyNode();
+					// this.focusAnyNode();
 				}
 				return true;
 
@@ -620,8 +623,7 @@ export class FocusManager {
 	}
 
 	// Finds focusable in adjacent column for horizontal navigation
-	findInNextColumn(element: HTMLElement, forward: boolean): FocusableNode | undefined {
-		const node = this.getNode(element);
+	findInNextColumn(node: FocusableNode, forward: boolean): FocusableNode | undefined {
 		if (!node) return undefined;
 
 		let searchNode: FocusableNode | undefined = node.parent;
@@ -631,7 +633,7 @@ export class FocusManager {
 			if (!ancestorNode) return;
 
 			// Find the current branch from the non-list parent's perspective
-			const ancestorChild = this.findChildNodeByDescendent(ancestorNode, element);
+			const ancestorChild = this.findChildNodeByDescendent(ancestorNode, node);
 			if (!ancestorChild) return;
 
 			// Find non-button siblings of the current branch for navigation
@@ -663,20 +665,22 @@ export class FocusManager {
 			if (!current.options.vertical) {
 				return current;
 			}
+			if (current.options.trap) {
+				return;
+			}
 			current = current.parent;
 		}
-		return undefined;
 	}
 
 	// Finds which child branch contains the given element
 	private findChildNodeByDescendent(
 		ancestor: FocusableNode,
-		element: HTMLElement
+		node: FocusableNode
 	): FocusableNode | undefined {
 		// Check each child (including buttons) to see which one contains our current element
 		// We need to check all children here because we're looking for containment, not navigation
 		for (const child of ancestor.children) {
-			if (this.isNodeDescendantOf(child, element)) {
+			if (this.isNodeDescendantOf(child, node.element)) {
 				return child;
 			}
 		}
@@ -773,12 +777,8 @@ export class FocusManager {
 		});
 	}
 
-	private handlePendingRegistration(
-		element: HTMLElement,
-		parentNode: FocusableNode | undefined,
-		options: FocusableOptions
-	) {
-		if (!parentNode && !options.isolate) {
+	private handlePendingRegistration(element: HTMLElement, parentNode: FocusableNode | undefined) {
+		if (!parentNode) {
 			this.pendingRelationships.push(element);
 		}
 	}
@@ -1044,7 +1044,6 @@ export class FocusManager {
 		if (node.options.disabled) flags.push('disabled');
 		if (node.options.trap) flags.push('trap');
 		if (node.options.vertical) flags.push('vertical');
-		if (node.options.isolate) flags.push('isolate');
 		if (this.isContainerElement(node)) flags.push('container');
 		const flagsStr = flags.length > 0 ? ` [${flags.join(', ')}]` : '';
 

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

@@ -23,12 +23,10 @@ export interface FocusableOptions {
 	disabled?: boolean;
 	// Treat children as vertically oriented (changes arrow key behavior, automatically skips during navigation)
 	vertical?: boolean;
-	// Prevent keyboard navigation from leaving this element
-	trap?: boolean;
 	// Automatically focus this element when registered
 	activate?: boolean;
-	// Don't establish parent-child relationships with other focusables
-	isolate?: boolean;
+	// Prevent keyboard navigation from leaving this element
+	trap?: boolean;
 	// Never highlight the element
 	dim?: boolean;
 	// Automatically trigger onAction when this element becomes active