sessions: fix hover reverse navigation

Traverse rich-hover controls and row actions symmetrically with Shift+Tab, and preserve GitHub's distinct Duplicate issue terminology.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: chryw

src/vs/platform/actionWidget/browser/actionList.ts

@@ -2198,21 +2198,6 @@ export class ActionListWidget<T> extends Disposable {
 		};
 	}
 
-	/**
-	 * The toolbar index Shift+Tab should return to when leaving the panel. Skips a trailing
-	 * removal action (appended after the item's own {@link IActionListItem.toolbarActions})
-	 * so the destructive Remove control isn't the panel's silent return target.
-	 */
-	private _lastPrimaryToolbarActionIndex(toolbar: ActionBar): number {
-		const items = toolbar.viewItems;
-		for (let i = items.length - 1; i >= 0; i--) {
-			if (items[i].action.id !== removeToolbarActionId) {
-				return i;
-			}
-		}
-		return items.length - 1;
-	}
-
 	private _focusFirstTabThroughPanelControl(element: IActionListItem<T>, row: HTMLElement): void {
 		const controls = this._getTabThroughPanelControls(element, row);
 		if (controls.toolbar?.length()) {
@@ -2262,12 +2247,16 @@ export class ActionListWidget<T> extends Disposable {
 		let target: HTMLElement | undefined;
 		if (event.shiftKey) {
 			if (inPanel) {
-				if (controls.toolbar?.length()) {
+				const panelControlIndex = controls.panelControls.indexOf(activeElement);
+				if (panelControlIndex > 0) {
+					target = controls.panelControls[panelControlIndex - 1];
+				} else if (controls.toolbar?.length()) {
 					dom.EventHelper.stop(event, true);
-					controls.toolbar.focus(this._lastPrimaryToolbarActionIndex(controls.toolbar));
+					controls.toolbar.focus(controls.toolbar.length() - 1);
 					return;
+				} else {
+					target = this._list.getHTMLElement();
 				}
-				target = this._list.getHTMLElement();
 			} else if (controls.toolbar?.isFocused()) {
 				const toolbarIndex = controls.toolbar.viewItems.findIndex((_, actionIndex) => controls.toolbar?.isFocused(actionIndex));
 				if (toolbarIndex > 0) {

src/vs/platform/actionWidget/test/browser/actionList.test.ts

@@ -1293,6 +1293,10 @@ suite('ActionListWidget', () => {
 		const enterDefaultPreserved = press('Enter');
 		const spaceDefaultPreserved = press(' ');
 		press('Tab', true);
+		const backToReference = focusState();
+		press('Tab', true);
+		const backToRepository = focusState();
+		press('Tab', true);
 		const backToCopy = focusState();
 		press('Tab', true);
 		const backToList = focusState();
@@ -1312,6 +1316,8 @@ suite('ActionListWidget', () => {
 			reference,
 			branch,
 			panelActivation: { bubbledPanelActivationKeys, enterDefaultPreserved, spaceDefaultPreserved },
+			backToReference,
+			backToRepository,
 			backToCopy,
 			backToList,
 			nextItem,
@@ -1327,6 +1333,8 @@ suite('ActionListWidget', () => {
 			reference: { location: 'panel', label: '#one' },
 			branch: { location: 'panel', label: 'Copy branch one' },
 			panelActivation: { bubbledPanelActivationKeys: [], enterDefaultPreserved: true, spaceDefaultPreserved: true },
+			backToReference: { location: 'panel', label: '#one' },
+			backToRepository: { location: 'panel', label: 'repo-one' },
 			backToCopy: { location: 'toolbar', label: 'Copy one' },
 			backToList: { location: 'list', label: 'Action Widget' },
 			nextItem: {
@@ -1337,7 +1345,7 @@ suite('ActionListWidget', () => {
 		});
 	});
 
-	test('Shift+Tab from the panel returns to the last non-removal toolbar action', () => {
+	test('Shift+Tab traverses the panel and toolbar controls in reverse order', () => {
 		const createPanel = () => {
 			const panel = document.createElement('div');
 			const control = document.createElement('a');
@@ -1377,8 +1385,13 @@ suite('ActionListWidget', () => {
 		press('Tab'); // Copy -> Remove
 		press('Tab'); // Remove -> panel link
 		press('Tab', true); // panel link -> Shift+Tab back into the toolbar
+		const firstReverseTarget = focusedToolbarLabel();
+		press('Tab', true); // Remove -> Copy
 
-		assert.strictEqual(focusedToolbarLabel(), 'Copy');
+		assert.deepStrictEqual({ firstReverseTarget, secondReverseTarget: focusedToolbarLabel() }, {
+			firstReverseTarget: 'Remove',
+			secondReverseTarget: 'Copy',
+		});
 	});
 
 	test('rebuilding the items in place re-measures only when the row count changed', () => {

src/vs/sessions/contrib/chat/test/browser/sessionChatInputToolbar.test.ts

@@ -264,6 +264,14 @@ suite('SessionChatInputToolbar', () => {
 			openerService,
 			sessionsService,
 		).flatMap(section => section.entries)[0];
+		const duplicateIssueEntry = buildSessionIssueSections(
+			[{ ref: issueRef, issue: { ...issue, stateReason: GitHubIssueStateReason.Duplicate } }],
+			undefined,
+			commandService,
+			clipboardService,
+			openerService,
+			sessionsService,
+		).flatMap(section => section.entries)[0];
 		const unresolvedIssueEntry = buildSessionIssueSections(
 			[{ ref: issueRef, issue: undefined }],
 			undefined,
@@ -284,6 +292,7 @@ suite('SessionChatInputToolbar', () => {
 		const pullRequestHover = await renderHover(pullRequestEntry);
 		const issueHover = await renderHover(issueEntry);
 		const activeIssueHover = await renderHover(activeIssueEntry);
+		const duplicateIssueHover = await renderHover(duplicateIssueEntry);
 		const pullRequestDropdownHover = renderDropdownHover(pullRequestEntry);
 		const issueDropdownHover = renderDropdownHover(issueEntry);
 		pullRequestHover?.querySelectorAll<HTMLButtonElement>('.sessions-pr-hover-branch').forEach(branch => branch.click());
@@ -373,6 +382,10 @@ suite('SessionChatInputToolbar', () => {
 				status: activeIssueHover?.querySelector('.sessions-issue-hover-status')?.textContent,
 				date: activeIssueHover?.querySelector('.sessions-issue-hover-date')?.textContent,
 			},
+			duplicateIssue: {
+				status: duplicateIssueHover?.querySelector('.sessions-issue-hover-status')?.textContent,
+				statusKind: duplicateIssueHover?.querySelector<HTMLElement>('.sessions-issue-hover-status')?.dataset.state,
+			},
 		}, {
 			pullRequest: {
 				label: 'Restore rich pill hovers',
@@ -478,6 +491,10 @@ suite('SessionChatInputToolbar', () => {
 				status: 'Open',
 				date: 'on Sep 3',
 			},
+			duplicateIssue: {
+				status: 'Duplicate',
+				statusKind: 'duplicate',
+			},
 		});
 	});
 

src/vs/sessions/contrib/github/browser/issueHover.ts

@@ -94,11 +94,14 @@ function appendHoverLink(container: HTMLElement, className: string, href: string
 	return link;
 }
 
-function getIssueStatus(issue: IGitHubIssue): { readonly kind: 'open' | 'closed' | 'notPlanned'; readonly label: string } {
+function getIssueStatus(issue: IGitHubIssue): { readonly kind: 'open' | 'closed' | 'notPlanned' | 'duplicate'; readonly label: string } {
 	if (issue.state === GitHubIssueState.Open) {
 		return { kind: 'open', label: localize('agentSessions.issueHover.open', "Open") };
 	}
-	if (issue.stateReason === GitHubIssueStateReason.NotPlanned || issue.stateReason === GitHubIssueStateReason.Duplicate) {
+	if (issue.stateReason === GitHubIssueStateReason.Duplicate) {
+		return { kind: 'duplicate', label: localize('agentSessions.issueHover.duplicate', "Duplicate") };
+	}
+	if (issue.stateReason === GitHubIssueStateReason.NotPlanned) {
 		return { kind: 'notPlanned', label: localize('agentSessions.issueHover.notPlanned', "Not planned") };
 	}
 	return { kind: 'closed', label: localize('agentSessions.issueHover.closed', "Closed") };