Carry over what only the win7 tree had: middle-click close, ARIA, menu fix

The deleted win7 tree held a few behaviours the modern tree never got: - Context menu: the first ArrowUp in a fresh menu skipped the last item (Delete in the row menu, Disconnect in the tab menu) — stepping from -1 landed one short. The index math moves to core/menu-nav.ts stepIndex(), checked by menu-nav.spec.ts (bun test; verified to fail on the old logic). Items get ids and the menu exposes aria-activedescendant, so screen readers follow the arrow keys while focus stays on the menu. - Middle-click closes a tab, as in browsers. auxclick also fires for the right button, so only button 1 counts; closeWorkspace already refuses the last tab. A middle press is swallowed so WebView2 doesn't start autoscroll over the scrollable strip. The hover ✕ stays. - ARIA: the tab strip is a tablist of role=tab buttons with aria-selected (the in-tab ✕ is taken out of the a11y tree and tab order — Ctrl/Cmd+W and the tab menu remain the keyboard paths); transfer bars are progressbars with value and a name. Co-Authored-By: Claude <noreply@anthropic.com>
2026-09-23 16:22:30 +02:00
parent 2bac7678ca
commit f7f02083fe
5 changed files with 80 additions and 14 deletions
@@ -0,0 +1,27 @@
+import { expect, test } from 'bun:test';
+import { stepIndex } from './menu-nav';
+
+// item, separator, item, disabled, item — the shape of a real row menu
+const menu = [true, false, true, false, true];
+
+test('a fresh menu: down lands on the first entry, up on the last', () => {
+  expect(stepIndex(menu, -1, 1)).toBe(0);
+  expect(stepIndex(menu, -1, -1)).toBe(4);
+});
+
+test('separators and disabled entries are skipped', () => {
+  expect(stepIndex(menu, 0, 1)).toBe(2);
+  expect(stepIndex(menu, 2, 1)).toBe(4);
+  expect(stepIndex(menu, 4, -1)).toBe(2);
+});
+
+test('wraps both ways', () => {
+  expect(stepIndex(menu, 4, 1)).toBe(0);
+  expect(stepIndex(menu, 0, -1)).toBe(4);
+});
+
+test('nothing selectable leaves the highlight alone', () => {
+  expect(stepIndex([false, false], -1, 1)).toBe(-1);
+  expect(stepIndex([false, false], -1, -1)).toBe(-1);
+  expect(stepIndex([], -1, -1)).toBe(-1);
+});
@@ -0,0 +1,15 @@
+/**
+ * Next highlighted menu index: skips unselectable entries and wraps. From -1
+ * (nothing highlighted yet), down lands on the first selectable entry and up
+ * on the LAST — stepping up from -1 directly would land one short of it.
+ * Returns `from` unchanged when nothing is selectable.
+ */
+export function stepIndex(selectable: boolean[], from: number, delta: 1 | -1): number {
+  const count = selectable.length;
+  let index = from < 0 && delta < 0 ? 0 : from;
+  for (let hops = 0; hops < count; hops++) {
+    index = (index + delta + count) % count;
+    if (selectable[index]) return index;
+  }
+  return from;
+}
@@ -10,6 +10,7 @@ import {
 import { NgIcon } from '@ng-icons/core';
 import { ContextMenuService, type MenuItem } from '@core/context-menu.service';
 import { FocusService } from '@core/focus.service';
+import { stepIndex } from '@core/menu-nav';
 import { remPx } from '@core/metrics';

 /** Rough per-item metrics in REM, used only to decide whether the menu must
@@ -38,6 +39,7 @@ const EDGE_GAP = 6; // px on purpose: a viewport-edge margin, not a UI size
        #menuEl
        role="menu"
        tabindex="-1"
+        [attr.aria-activedescendant]="activeIndex() >= 0 ? 'ctx-item-' + activeIndex() : null"
        (keydown)="onKeydown($event)"
        [style.left.px]="position().x"
        [style.top.px]="position().y"
@@ -50,6 +52,7 @@ const EDGE_GAP = 6; // px on purpose: a viewport-edge margin, not a UI size
            <button
              type="button"
              role="menuitem"
+              [attr.id]="'ctx-item-' + $index"
              [title]="item.tooltip ?? ''"
              [disabled]="!!item.disabled"
              [class.bg-accent]="$index === activeIndex() && !item.disabled"
@@ -163,18 +166,9 @@ export class ContextMenu {
    }
  }

-  /** Move the highlight by one selectable item, skipping separators and
-   *  disabled entries, wrapping. */
-  private step(items: MenuItem[], delta: number): void {
-    const count = items.length;
-    if (!count) return;
-    let index = this.activeIndex();
-    for (let hops = 0; hops < count; hops++) {
-      index = (index + delta + count) % count;
-      if (items[index].type !== 'separator' && !items[index].disabled) {
-        this.activeIndex.set(index);
-        return;
-      }
-    }
+  /** Move the highlight by one selectable item (see stepIndex). */
+  private step(items: MenuItem[], delta: 1 | -1): void {
+    const selectable = items.map((i) => i.type !== 'separator' && !i.disabled);
+    this.activeIndex.set(stepIndex(selectable, this.activeIndex(), delta));
  }
 }
@@ -49,6 +49,7 @@ const CONCURRENCY_CHOICES = [2, 3, 4, 6, 8] as const;
    >
      <div
        #tabScroll
+        role="tablist"
        class="flex min-w-0 items-center gap-0.5 overflow-x-auto [scrollbar-width:none] [&::-webkit-scrollbar]:hidden"
      >
      @for (ws of panels.workspaces(); track ws.id; let i = $index) {
@@ -59,6 +60,8 @@ const CONCURRENCY_CHOICES = [2, 3, 4, 6, 8] as const;
          [class.opacity-50]="draggedId() === ws.id"
          [style.box-shadow]="dropIndicatorShadow(i)"
          (contextmenu)="onTabContextMenu($event, ws, i)"
+          (mousedown)="onTabMouseDown($event)"
+          (auxclick)="onAuxClick($event, ws.id)"
          draggable="true"
          (dragstart)="onDragStart($event, ws.id)"
          (dragover)="onDragOver($event, i)"
@@ -66,6 +69,8 @@ const CONCURRENCY_CHOICES = [2, 3, 4, 6, 8] as const;
          (dragend)="onDragEnd()"
        >
          <button
+            role="tab"
+            [attr.aria-selected]="ws.id === panels.activeWorkspaceId()"
            (click)="panels.setActiveWorkspace(ws.id)"
            class="flex min-w-0 items-center gap-1.5 py-1.5 pr-1 pl-2.5 text-xs"
            [class.text-foreground]="ws.id === panels.activeWorkspaceId()"
@@ -83,6 +88,8 @@ const CONCURRENCY_CHOICES = [2, 3, 4, 6, 8] as const;
          @if (panels.workspaces().length > 1) {
            <button
              (click)="close($event, ws.id)"
+              tabindex="-1"
+              aria-hidden="true"
              [title]="i18n.t('tabBar.closeTab', { mod: modLabel })"
              class="mr-1 grid size-4 shrink-0 place-items-center rounded text-muted-foreground opacity-0 group-hover:opacity-100 hover:bg-border hover:text-foreground"
            >
@@ -484,6 +491,22 @@ export class TabBar {
    this.panels.closeWorkspace(id);
  }

+  /**
+   * Middle-click closes the tab, as in browsers. `auxclick` also fires for the
+   * right button, hence the button check; the last tab is refused by
+   * closeWorkspace itself.
+   */
+  onAuxClick(event: MouseEvent, id: number): void {
+    if (event.button !== 1) return;
+    event.preventDefault();
+    this.close(event, id);
+  }
+
+  /** A middle press over the scrollable strip would start WebView2's autoscroll. */
+  onTabMouseDown(event: MouseEvent): void {
+    if (event.button === 1) event.preventDefault();
+  }
+
  /**
   * Right-click a tab. The menu targets THIS tab, not the active one — its
   * actions all carry the id — so right-clicking a background tab does not
@@ -19,7 +19,14 @@
        <div class="min-w-0 flex-1">
          <div class="truncate text-[0.6875rem] text-muted-foreground">{{ progressLabel(job) }}</div>
          @if (isActive(job)) {
-            <div class="mt-0.5 h-1 overflow-hidden rounded-full bg-muted">
+            <div
+              role="progressbar"
+              aria-valuemin="0"
+              aria-valuemax="100"
+              [attr.aria-valuenow]="jobPercent(job)"
+              [attr.aria-label]="job.name"
+              class="mt-0.5 h-1 overflow-hidden rounded-full bg-muted"
+            >
              <div
                class="h-full rounded-full bg-primary transition-[width] duration-100"
                [style.width.%]="jobPercent(job)"