Make spotlight info cards accessible to keyboard and screen reader users

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);

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 {

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<HTMLElement>('.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<HTMLElement>('.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<HTMLElement>('.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', () => {