From ba4b6a25b891c78f7f6748c7432ac431338f40a4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 16 Sep 2026 10:41:04 +0000 Subject: [PATCH 1/2] Initial plan From 07150682c6f6056b523e247902fb8b63d2d4e05a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 16 Sep 2026 10:51:47 +0000 Subject: [PATCH 2/2] Make spotlight info cards keyboard accessible Co-authored-by: benibenj <44439583+benibenj@users.noreply.github.com> --- .../onboarding/browser/media/spotlight.css | 5 -- .../browser/spotlight/spotlightOverlay.ts | 17 +---- .../test/browser/spotlightOverlay.test.ts | 74 ++++++++++++++++--- 3 files changed, 67 insertions(+), 29 deletions(-) diff --git a/src/vs/workbench/contrib/onboarding/browser/media/spotlight.css b/src/vs/workbench/contrib/onboarding/browser/media/spotlight.css index a9a0777f544e5..f2e578f6c35f2 100644 --- a/src/vs/workbench/contrib/onboarding/browser/media/spotlight.css +++ b/src/vs/workbench/contrib/onboarding/browser/media/spotlight.css @@ -94,11 +94,6 @@ pointer-events: auto; } -.spotlight-callout:focus, -.spotlight-callout:focus-visible { - outline: none; -} - .spotlight-callout-title { margin: 0 0 var(--vscode-spacing-size80) 0; font-size: var(--vscode-bodyFontSize); diff --git a/src/vs/workbench/contrib/onboarding/browser/spotlight/spotlightOverlay.ts b/src/vs/workbench/contrib/onboarding/browser/spotlight/spotlightOverlay.ts index fa4c23b92efff..f966323039930 100644 --- a/src/vs/workbench/contrib/onboarding/browser/spotlight/spotlightOverlay.ts +++ b/src/vs/workbench/contrib/onboarding/browser/spotlight/spotlightOverlay.ts @@ -122,7 +122,7 @@ export class SpotlightOverlay extends Disposable { this._callout = append(this._root, $('.spotlight-callout')); this._callout.setAttribute('role', 'dialog'); this._callout.setAttribute('aria-modal', 'true'); - this._callout.tabIndex = -1; + this._callout.tabIndex = 0; const header = append(this._callout, $('.spotlight-callout-header')); this._title = append(header, $('h2.spotlight-callout-title')); @@ -428,9 +428,7 @@ export class SpotlightOverlay extends Disposable { const active = getActiveElement(); const currentIndex = focusable.findIndex(element => element === active); - // When focus isn't currently on a tracked element (e.g. it landed on the - // callout container itself), start from the appropriate end so Tab goes to - // the first element and Shift+Tab to the last. + // Start from the appropriate end when focus isn't currently on a tracked element. let nextIndex: number; if (currentIndex === -1) { nextIndex = event.shiftKey ? focusable.length - 1 : 0; @@ -444,14 +442,7 @@ export class SpotlightOverlay extends Disposable { focusable[nextIndex].focus(); } - /** - * The focusable elements participating in the focus trap, in DOM order: the - * spotlighted target (when it is interactive or the Next button is hidden), then any - * interactive content in the (possibly markdown) description, then the visible - * action buttons. Including the target keeps the spotlighted control - * keyboard-reachable, and querying the description keeps markdown links - * reachable despite `aria-modal`. - */ + /** Collects the interactive target, callout, description links, and visible actions in focus order. */ private _collectFocusable(): HTMLElement[] { const targetFocusables = (this._options.allowTargetInteraction || this._options.advanceOnTargetClick || this._options.hideNext) && this._target // eslint-disable-next-line no-restricted-syntax -- querying the spotlight target subtree for focusable controls @@ -464,7 +455,7 @@ export class SpotlightOverlay extends Disposable { const buttons = [this._skipButton, this._backButton, this._nextButton] .filter(button => button.element.style.display !== 'none') .map(button => button.element); - return [...targetFocusables, ...descriptionFocusables, ...buttons].filter(element => this._isTabbable(element)); + return [...targetFocusables, this._callout, ...descriptionFocusables, ...buttons].filter(element => this._isTabbable(element)); } private _isTabbable(element: HTMLElement): boolean { diff --git a/src/vs/workbench/contrib/onboarding/test/browser/spotlightOverlay.test.ts b/src/vs/workbench/contrib/onboarding/test/browser/spotlightOverlay.test.ts index 25db42bad8c93..c534174a92248 100644 --- a/src/vs/workbench/contrib/onboarding/test/browser/spotlightOverlay.test.ts +++ b/src/vs/workbench/contrib/onboarding/test/browser/spotlightOverlay.test.ts @@ -92,6 +92,59 @@ suite('SpotlightOverlay', () => { }); }); + test('callout is keyboard-reachable and exposes the current title and description', () => { + const container = createContainer(); + const overlay = disposables.add(new SpotlightOverlay(container, FakeResizeObserver)); + const target = createTarget(container, 0, 0, 50, 50); + const callout = container.querySelector('.spotlight-callout')!; + const steps = [ + { title: 'First title', description: 'Plain description' }, + { title: 'Second title', description: new MarkdownString('**Markdown** description') }, + ]; + + const snapshots = steps.map(step => { + overlay.show(target, content({ ...step, stepIndex: 0, stepCount: 1, canGoBack: false, isLastStep: true })); + const next = getButtons(container).at(-1)!; + const primaryFocused = mainWindow.document.activeElement === next; + next.dispatchEvent(new KeyboardEvent('keydown', { key: 'Tab', keyCode: 9, bubbles: true, cancelable: true })); + + return { + primaryFocused, + calloutFocused: mainWindow.document.activeElement === callout, + tabIndex: callout.tabIndex, + role: callout.getAttribute('role'), + title: mainWindow.document.getElementById(callout.getAttribute('aria-labelledby')!)?.textContent, + description: mainWindow.document.getElementById(callout.getAttribute('aria-describedby')!)?.textContent, + }; + }); + + assert.deepStrictEqual(snapshots, steps.map(step => ({ + primaryFocused: true, + calloutFocused: true, + tabIndex: 0, + role: 'dialog', + title: step.title, + description: typeof step.description === 'string' ? step.description : 'Markdown description', + }))); + }); + + test('focus trap includes the callout in both directions', () => { + const container = createContainer(); + const overlay = disposables.add(new SpotlightOverlay(container, FakeResizeObserver)); + const target = createTarget(container, 0, 0, 50, 50); + overlay.show(target, content()); + + const callout = container.querySelector('.spotlight-callout')!; + const [skip, back, next] = getButtons(container); + const focused: (Element | null)[] = []; + for (const shiftKey of [false, false, false, false, true, true, true, true]) { + mainWindow.document.activeElement!.dispatchEvent(new KeyboardEvent('keydown', { key: 'Tab', keyCode: 9, shiftKey, bubbles: true, cancelable: true })); + focused.push(mainWindow.document.activeElement); + } + + assert.deepStrictEqual(focused, [callout, skip, back, next, back, skip, callout, next]); + }); + test('vertical placement centers the callout over the target', () => { const container = createContainer(); const overlay = disposables.add(new SpotlightOverlay(container, FakeResizeObserver as unknown as typeof ResizeObserver)); @@ -350,19 +403,20 @@ suite('SpotlightOverlay', () => { target.tabIndex = 0; overlay.show(target, content(), { hideNext: true }); - const [skip, , next] = getButtons(container); + const [, , next] = getButtons(container); + const callout = container.querySelector('.spotlight-callout')!; const event = new KeyboardEvent('keydown', { bubbles: true, cancelable: true }); Object.defineProperty(event, 'keyCode', { get: () => 9 /* Tab */ }); target.dispatchEvent(event); assert.deepStrictEqual({ nextHidden: next.style.display === 'none', - ariaModal: container.getElementsByClassName('spotlight-callout')[0].getAttribute('aria-modal'), + ariaModal: callout.getAttribute('aria-modal'), activeElement: mainWindow.document.activeElement, }, { nextHidden: true, ariaModal: 'false', - activeElement: skip, + activeElement: callout, }); }); @@ -391,16 +445,14 @@ suite('SpotlightOverlay', () => { const link = callout.getElementsByTagName('a')[0] as HTMLAnchorElement | undefined; assert.ok(link, 'expected a link to be rendered from the markdown description'); - // With the link focused, Shift+Tab should cycle to the last button (Next), - // proving the link participates in the trap rather than being skipped. link!.focus(); - const event = new KeyboardEvent('keydown', { shiftKey: true, bubbles: true, cancelable: true }); - // The KeyboardEvent constructor does not honor `keyCode` from the init dict - // in all engines, so set it explicitly (StandardKeyboardEvent reads keyCode). - Object.defineProperty(event, 'keyCode', { get: () => 9 /* Tab */ }); - callout.dispatchEvent(event); + const focused: (Element | null)[] = []; + for (const shiftKey of [true, true, false, false]) { + callout.dispatchEvent(new KeyboardEvent('keydown', { key: 'Tab', keyCode: 9, shiftKey, bubbles: true, cancelable: true })); + focused.push(mainWindow.document.activeElement); + } - assert.strictEqual(mainWindow.document.activeElement, getButtons(container).at(-1)); + assert.deepStrictEqual(focused, [callout, getButtons(container).at(-1), callout, link]); }); test('dispose removes the overlay from the DOM', () => {