From 00c9ec819e803cc3d67f6d36aad28e7097bef5ae Mon Sep 17 00:00:00 2001 From: Cherry Wang Date: Thu, 10 Sep 2026 15:32:37 -0700 Subject: [PATCH 01/11] sessions: add rich GitHub reference hovers Reuse the existing issue and pull request hover components for dropdown details, with compact metadata, explicit spacing ownership, and bounded readable descriptions. Integrate the pill, row action, detail link, and branch-copy controls into one accessible keyboard flow. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../actionWidget/browser/actionList.ts | 153 +++++++++++++++++- .../actionWidget/browser/actionWidget.css | 4 + .../test/browser/actionList.test.ts | 123 ++++++++++++++ .../sessions/contrib/chat/browser/chatView.ts | 7 + .../chat/browser/sessionChatInputToolbar.ts | 68 +++++--- .../browser/sessionsChatAccessibilityHelp.ts | 2 +- .../browser/sessionChatInputToolbar.test.ts | 133 +++++++++++++-- .../browser/fetchers/githubPRFetcher.ts | 2 + .../contrib/github/browser/githubHover.ts | 18 +++ .../contrib/github/browser/issueHover.ts | 91 ++++++++--- .../github/browser/media/issueHover.css | 24 +++ .../github/browser/media/pullRequestHover.css | 37 +++++ .../github/browser/pullRequestHover.ts | 130 ++++++++++++--- .../sessions/contrib/github/common/types.ts | 1 + src/vs/workbench/browser/chatPills.ts | 8 + .../contrib/chat/browser/chatInputPills.ts | 4 + .../githubReferenceHoverLayouts.fixture.ts | 101 ++++++++++++ 17 files changed, 828 insertions(+), 78 deletions(-) create mode 100644 src/vs/sessions/contrib/github/browser/githubHover.ts create mode 100644 src/vs/workbench/test/browser/componentFixtures/sessions/githubReferenceHoverLayouts.fixture.ts diff --git a/src/vs/platform/actionWidget/browser/actionList.ts b/src/vs/platform/actionWidget/browser/actionList.ts index 48567e36406089..057fae7c9ae12d 100644 --- a/src/vs/platform/actionWidget/browser/actionList.ts +++ b/src/vs/platform/actionWidget/browser/actionList.ts @@ -68,6 +68,15 @@ export interface IActionListItemHover { readonly expandable?: boolean; /** Whether to show the expandable hover's row chevron. Defaults to true. */ readonly showIndicator?: boolean; + /** + * Includes the focused row's toolbar actions and hover-panel controls in one + * Tab sequence while the list retains Up/Down navigation ownership. + */ + readonly tabThroughPanel?: boolean; + /** Interactive elements owned by the hover content, in forward Tab order. */ + readonly getTabbableElements?: () => readonly HTMLElement[]; + /** Whether the hover content supplies its own inset from the panel boundary. */ + readonly contentOwnsPadding?: boolean; /** * CSS class set on the hover panel while this item's hover is showing, so a * consumer can style the panel without reaching for the content inside it. @@ -281,6 +290,7 @@ class ActionItemRenderer implements IListRenderer, IAction private readonly _linkHandler: ((uri: URI, item: IActionListItem) => void) | undefined, private readonly _hideDefaultKeybindingTooltip: boolean, private readonly _registerStandaloneToggle: (item: IActionListItem, toggle: Switch) => IDisposable, + private readonly _registerToolbar: (item: IActionListItem, toolbar: ActionBar) => IDisposable, @IKeybindingService private readonly _keybindingService: IKeybindingService, @IOpenerService private readonly _openerService: IOpenerService, ) { } @@ -507,6 +517,7 @@ class ActionItemRenderer implements IListRenderer, IAction const actionBar = new ActionBar(data.toolbar); data.elementDisposables.add(actionBar); actionBar.push(toolbarActions, { icon: true, label: false }); + data.elementDisposables.add(this._registerToolbar(element, actionBar)); } if (hasSubmenuIndicator(element)) { @@ -782,6 +793,7 @@ export class ActionListWidget extends Disposable { private readonly _filterCts = this._register(new MutableDisposable()); private readonly _groupTitleByIndex = new Map(); private readonly _standaloneToggles = new Map, Switch>(); + private readonly _itemToolbars = new Map, ActionBar>(); private _visibleMenuItems: readonly IActionListItem[]; private readonly _onDidRequestLayout = this._register(new Emitter()); @@ -837,6 +849,13 @@ export class ActionListWidget extends Disposable { // the way back to the row it belongs to lives here. A panel that does have a // submenu list stops these keys before they reach this handler. this._register(dom.addDisposableListener(this._submenuContainer, 'keydown', (e: KeyboardEvent) => { + if ((e.key === 'Enter' || e.key === ' ') && this._currentSubmenuElement?.hover?.tabThroughPanel) { + const target = dom.isHTMLElement(e.target) ? e.target : undefined; + if (target && this._currentSubmenuElement.hover.getTabbableElements?.().includes(target)) { + e.stopPropagation(); + return; + } + } if (e.key !== 'ArrowLeft' && e.key !== 'Escape') { return; } @@ -902,6 +921,13 @@ export class ActionListWidget extends Disposable { this._standaloneToggles.delete(item); } }); + }, (item, toolbar) => { + this._itemToolbars.set(item, toolbar); + return toDisposable(() => { + if (this._itemToolbars.get(item) === toolbar) { + this._itemToolbars.delete(item); + } + }); }, this._keybindingService, this._openerService), new HeaderRenderer(), new SeparatorRenderer(), @@ -1181,7 +1207,9 @@ export class ActionListWidget extends Disposable { const rowElement = this._getRowElement(focused[0]); if (rowElement) { this._showSubmenuForElement(element, rowElement); - if (this._currentSubmenuWidget) { + if (element.hover?.tabThroughPanel) { + this._focusFirstTabThroughPanelControl(element, rowElement); + } else if (this._currentSubmenuWidget) { this._currentSubmenuWidget.focus(); } else { this._submenuContainer.focus(); @@ -1191,6 +1219,7 @@ export class ActionListWidget extends Disposable { } } })); + this._register(dom.addDisposableListener(this.domNode, 'keydown', e => this._handleTabThroughPanelKeyDown(e), true)); if (this._filterInput || this._options?.onType) { this._register(dom.addDisposableListener(this.domNode, 'keydown', (e: KeyboardEvent) => { @@ -1497,6 +1526,7 @@ export class ActionListWidget extends Disposable { } this._list.domFocus(); this._focusCheckedOrFirst(); + this._showTabThroughPanelForFocusedItem(); } clearFocus(): void { @@ -2115,6 +2145,126 @@ export class ActionListWidget extends Disposable { return this.domNode.ownerDocument.getElementById(this._list.getElementID(index)); } + private _showTabThroughPanelForFocusedItem(): void { + const focused = this._list.getFocus(); + if (focused.length === 0) { + return; + } + const index = focused[0]; + const element = this._list.element(index); + if (!element.hover?.tabThroughPanel) { + return; + } + const row = this._getRowElement(index); + if (row) { + this._showSubmenuForElement(element, row); + } + } + + private _getTabThroughPanelControls(element: IActionListItem, row: HTMLElement): { readonly toolbar: ActionBar | undefined; readonly panelControls: readonly HTMLElement[] } { + if (this._currentSubmenuElement !== element) { + this._showSubmenuForElement(element, row); + } + return { + toolbar: this._itemToolbars.get(element), + panelControls: element.hover?.getTabbableElements?.() ?? [], + }; + } + + private _focusFirstTabThroughPanelControl(element: IActionListItem, row: HTMLElement): void { + const controls = this._getTabThroughPanelControls(element, row); + if (controls.toolbar?.length()) { + controls.toolbar.focus(0); + } else { + (controls.panelControls[0] ?? this._list.getHTMLElement()).focus(); + } + } + + private _handleTabThroughPanelKeyDown(event: KeyboardEvent): void { + if (event.isComposing) { + return; + } + const focused = this._list.getFocus(); + if (focused.length === 0) { + return; + } + const index = focused[0]; + const element = this._list.element(index); + if (!element.hover?.tabThroughPanel) { + return; + } + const row = this._getRowElement(index); + const activeElement = dom.getActiveElement(); + if (!row || !dom.isHTMLElement(activeElement)) { + return; + } + const controls = this._getTabThroughPanelControls(element, row); + const inToolbar = controls.toolbar?.isFocused() ?? false; + const inPanel = this._submenuContainer.contains(activeElement); + + if ((event.key === 'ArrowUp' || event.key === 'ArrowDown') && (inToolbar || inPanel)) { + dom.EventHelper.stop(event, true); + this._list.domFocus(); + if (event.key === 'ArrowUp') { + this.focusPrevious(); + } else { + this.focusNext(); + } + return; + } + + if (event.key !== 'Tab') { + return; + } + + let target: HTMLElement | undefined; + if (event.shiftKey) { + if (inPanel) { + if (controls.toolbar?.length()) { + dom.EventHelper.stop(event, true); + controls.toolbar.focus(controls.toolbar.length() - 1); + return; + } + target = this._list.getHTMLElement(); + } else if (controls.toolbar?.isFocused()) { + const toolbarIndex = controls.toolbar.viewItems.findIndex((_, actionIndex) => controls.toolbar?.isFocused(actionIndex)); + if (toolbarIndex > 0) { + dom.EventHelper.stop(event, true); + controls.toolbar.focus(toolbarIndex - 1); + return; + } + target = this._list.getHTMLElement(); + } + } else if (activeElement === this._list.getHTMLElement()) { + if (controls.toolbar?.length()) { + dom.EventHelper.stop(event, true); + controls.toolbar.focus(0); + return; + } + target = controls.panelControls[0]; + } else { + if (controls.toolbar?.isFocused()) { + const toolbarIndex = controls.toolbar.viewItems.findIndex((_, actionIndex) => controls.toolbar?.isFocused(actionIndex)); + if (toolbarIndex + 1 < controls.toolbar.length()) { + dom.EventHelper.stop(event, true); + controls.toolbar.focus(toolbarIndex + 1); + return; + } + target = controls.panelControls[0]; + } else { + const panelControlIndex = controls.panelControls.indexOf(activeElement); + if (panelControlIndex >= 0) { + target = controls.panelControls[panelControlIndex + 1] ?? this._list.getHTMLElement(); + } + } + } + + if (target) { + dom.EventHelper.stop(event, true); + target.focus(); + } + } + private _showHoverForElement(element: IActionListItem, index: number): void { if (this._currentSubmenuElement === element) { return; @@ -2220,6 +2370,7 @@ export class ActionListWidget extends Disposable { hoverHeader = rendered.element; } hoverHeader.classList.add('action-list-submenu-hover-header'); + hoverHeader.classList.toggle('content-owns-padding', element.hover?.contentOwnsPadding === true); if (element.submenuActions?.length) { hoverHeader.classList.add('has-submenu'); } diff --git a/src/vs/platform/actionWidget/browser/actionWidget.css b/src/vs/platform/actionWidget/browser/actionWidget.css index 40ae57b7a4bf87..f24fdde5d6bc7e 100644 --- a/src/vs/platform/actionWidget/browser/actionWidget.css +++ b/src/vs/platform/actionWidget/browser/actionWidget.css @@ -602,6 +602,10 @@ user-select: text; } +.action-list-submenu-hover-header.content-owns-padding { + padding: 0; +} + .action-list-submenu-hover-header a { color: var(--vscode-textLink-foreground); } diff --git a/src/vs/platform/actionWidget/test/browser/actionList.test.ts b/src/vs/platform/actionWidget/test/browser/actionList.test.ts index f934d57f80b1e6..289e720f8659f5 100644 --- a/src/vs/platform/actionWidget/test/browser/actionList.test.ts +++ b/src/vs/platform/actionWidget/test/browser/actionList.test.ts @@ -1153,6 +1153,129 @@ suite('ActionListWidget', () => { }); }); + test('tabs through a focused row toolbar and hover panel while preserving list navigation', () => { + const createPanel = (id: string) => { + const panel = document.createElement('div'); + const repository = document.createElement('a'); + repository.href = `https://example.com/${id}`; + repository.textContent = `repo-${id}`; + const reference = document.createElement('a'); + reference.href = `https://example.com/${id}/1`; + reference.textContent = `#${id}`; + const branch = document.createElement('button'); + branch.textContent = `branch-${id}`; + branch.setAttribute('aria-label', `Copy branch ${id}`); + panel.append(repository, reference, branch); + return { panel, controls: [repository, reference, branch] }; + }; + const integratedAction = (id: string): IActionListItem => { + let panelControls: readonly HTMLElement[] = []; + return { + ...action(id), + toolbarActions: [toAction({ id: `copy-${id}`, label: `Copy ${id}`, run: () => { } })], + hover: { + content: () => { + const result = createPanel(id); + panelControls = result.controls; + return result.panel; + }, + expandable: true, + showIndicator: false, + tabThroughPanel: true, + getTabbableElements: () => panelControls, + contentOwnsPadding: true, + }, + }; + }; + const widget = createActionListWidget(disposables, { + items: [integratedAction('one'), integratedAction('two')], + listOptions: { showFilter: false, reserveSubmenuSpace: false }, + }); + const press = (key: string, shiftKey = false) => + document.activeElement?.dispatchEvent(new KeyboardEvent('keydown', { key, shiftKey, bubbles: true, cancelable: true })); + const focusState = () => { + const active = document.activeElement; + return { + location: active === widget.domNode.querySelector('.monaco-list') + ? 'list' + : active?.closest('.action-list-submenu-panel') + ? 'panel' + : active?.closest('.action-list-item-toolbar') + ? 'toolbar' + : 'other', + label: active?.getAttribute('aria-label') ?? active?.textContent, + }; + }; + const panel = widget.domNode.querySelector('.action-list-submenu-panel')!; + + widget.focus(); + const initial = { + focus: focusState(), + panelRole: panel.getAttribute('role'), + panelLabel: panel.getAttribute('aria-label'), + contentOwnsPadding: panel.querySelector('.action-list-submenu-hover-header')?.classList.contains('content-owns-padding'), + }; + press('Tab'); + const copy = focusState(); + press('Tab'); + const repository = focusState(); + press('Tab'); + const reference = focusState(); + press('Tab'); + const branch = focusState(); + const bubbledPanelActivationKeys: string[] = []; + widget.domNode.addEventListener('keydown', event => { + if (event.key === 'Enter' || event.key === ' ') { + bubbledPanelActivationKeys.push(event.key); + } + }); + const enterDefaultPreserved = press('Enter'); + const spaceDefaultPreserved = press(' '); + press('Tab', true); + const backToCopy = focusState(); + press('Tab', true); + const backToList = focusState(); + press('Tab'); + press('Tab'); + press('ArrowDown'); + const nextItem = { + focus: focusState(), + item: widget.getFocusedElement()?.item?.id, + panelLabel: panel.getAttribute('aria-label'), + }; + + assert.deepStrictEqual({ + initial, + copy, + repository, + reference, + branch, + panelActivation: { bubbledPanelActivationKeys, enterDefaultPreserved, spaceDefaultPreserved }, + backToCopy, + backToList, + nextItem, + }, { + initial: { + focus: { location: 'list', label: 'Action Widget' }, + panelRole: 'dialog', + panelLabel: 'one', + contentOwnsPadding: true, + }, + copy: { location: 'toolbar', label: 'Copy one' }, + repository: { location: 'panel', label: 'repo-one' }, + reference: { location: 'panel', label: '#one' }, + branch: { location: 'panel', label: 'Copy branch one' }, + panelActivation: { bubbledPanelActivationKeys: [], enterDefaultPreserved: true, spaceDefaultPreserved: true }, + backToCopy: { location: 'toolbar', label: 'Copy one' }, + backToList: { location: 'list', label: 'Action Widget' }, + nextItem: { + focus: { location: 'list', label: 'Action Widget' }, + item: 'two', + panelLabel: 'two', + }, + }); + }); + test('rebuilding the items in place re-measures only when the row count changed', () => { const widget = createActionListWidget(disposables, { items: [action('one'), action('two')] }); const layouts: string[] = []; diff --git a/src/vs/sessions/contrib/chat/browser/chatView.ts b/src/vs/sessions/contrib/chat/browser/chatView.ts index 2a85fdc29f4e6d..02062f092a74ca 100644 --- a/src/vs/sessions/contrib/chat/browser/chatView.ts +++ b/src/vs/sessions/contrib/chat/browser/chatView.ts @@ -10,6 +10,7 @@ import { StandardMouseEvent } from '../../../../base/browser/mouseEvent.js'; import { renderAsPlaintext } from '../../../../base/browser/markdownRenderer.js'; import { CancellationTokenSource } from '../../../../base/common/cancellation.js'; import { MutableDisposable, toDisposable } from '../../../../base/common/lifecycle.js'; +import { KeyCode } from '../../../../base/common/keyCodes.js'; import { autorun, derived, IObservable, observableFromEvent, observableValue } from '../../../../base/common/observable.js'; import { isEqual } from '../../../../base/common/resources.js'; import { URI } from '../../../../base/common/uri.js'; @@ -346,6 +347,12 @@ export class ChatView extends AbstractChatView { // Floating status pills above the input. this._chatPills = this._register(instantiationService.createInstance(SessionChatInputToolbar, false, () => this._widget.focusInput())); + this._register(this._widget.inputEditor.onKeyDown(event => { + if (event.keyCode === KeyCode.Tab && event.shiftKey && !event.ctrlKey && !event.metaKey && !event.altKey && this._chatPills.focusFirst()) { + event.preventDefault(); + event.stopPropagation(); + } + })); const updateChatPillsVisibility = (visible: boolean) => { this._widget.inputPart.persistentContentContainerElement.classList.toggle(chatPersistentContentVisibleClass, visible); }; diff --git a/src/vs/sessions/contrib/chat/browser/sessionChatInputToolbar.ts b/src/vs/sessions/contrib/chat/browser/sessionChatInputToolbar.ts index a767c0a28e6183..5afd6a2af9b422 100644 --- a/src/vs/sessions/contrib/chat/browser/sessionChatInputToolbar.ts +++ b/src/vs/sessions/contrib/chat/browser/sessionChatInputToolbar.ts @@ -43,8 +43,8 @@ import { ISessionChangesStatsCache, readSessionChangesStats } from '../../../ser import { ISessionChangesService } from '../../changes/browser/sessionChangesService.js'; import { IAgentWorkbenchLayoutService } from '../../../browser/workbench.js'; import { getSessionAgentMergeConfigurationObservable } from '../../../browser/sessionAgentMerge.js'; -import { createIssueHoverElement } from '../../github/browser/issueHover.js'; -import { createPullRequestHoverElement } from '../../github/browser/pullRequestHover.js'; +import { createIssueHover } from '../../github/browser/issueHover.js'; +import { createPullRequestHover } from '../../github/browser/pullRequestHover.js'; import { linkKey } from '../../../common/sessionLinks.js'; /** Fake artifacts for the pill debug overlay. */ @@ -72,11 +72,13 @@ function getPullRequestAttention(icon: ThemeIcon, status: IResolvedSessionPullRe return undefined; } -function getGitHubRepositoryHoverData(owner: string, repo: string, openerService: IOpenerService) { +function getGitHubHoverLinkData(owner: string, repo: string, reference: URI, openerService: IOpenerService) { const repository = URI.parse(`https://github.com/${owner}/${repo}`); return { repositoryHref: repository.toString(true), + referenceHref: reference.toString(true), onDidClickRepository: () => { void openerService.open(repository, { openExternal: true }); }, + onDidClickReference: () => { void openerService.open(reference, { openExternal: true }); }, }; } @@ -85,6 +87,22 @@ export function buildSessionPullRequestSections(pullRequests: readonly IResolved const entries = pullRequests.map(({ ref, pullRequest, icon, status }) => { const artifacts = artifactActions?.artifacts.filter(artifact => artifact.isArtifact && artifact.kind === SessionArtifactKind.PullRequest && artifact.link && linkKey(artifact.link.toString(true)) === linkKey(ref.uri.toString(true))); const title = pullRequest?.title ?? ref.title; + let hoverTabbableElements: readonly HTMLElement[] = []; + const createHover = pullRequest ? (density: 'default' | 'compact') => createPullRequestHover({ + owner: ref.owner, + repo: ref.repo, + number: ref.number, + ...getGitHubHoverLinkData(ref.owner, ref.repo, ref.uri, openerService), + pullRequest, + density, + ...(pullRequest.baseRef ? { onDidClickBaseBranch: () => { void clipboardService.writeText(pullRequest.baseRef); } } : {}), + ...(pullRequest.headRef ? { onDidClickHeadBranch: () => { void clipboardService.writeText(pullRequest.headRef); } } : {}), + }) : undefined; + const createDropdownHover = createHover ? () => { + const hover = createHover('compact'); + hoverTabbableElements = hover.tabbableElements; + return hover.element; + } : undefined; const label = title ? localize('sessionChatPills.pullRequestWithTitle', "Pull Request #{0}: {1}", ref.number, title) : localize('sessionChatPills.pullRequest', "Pull Request #{0}", ref.number); @@ -124,16 +142,9 @@ export function buildSessionPullRequestSections(pullRequests: readonly IResolved ...getChatPillResourceLocation(ref.uri, label), ariaDescription: localize('sessionChatPills.pullRequestDescription', "{0}. {1}", stateDescription, ref.uri.toString(true)), ...(!pullRequest && ref.title ? { tooltip: `${label}\n${ref.uri.toString(true)}` } : {}), - ...(pullRequest ? { - pillHover: { - element: () => createPullRequestHoverElement({ - owner: ref.owner, - repo: ref.repo, - number: ref.number, - ...getGitHubRepositoryHoverData(ref.owner, ref.repo, openerService), - pullRequest, - }), - }, + ...(createDropdownHover && createHover ? { + hover: { content: createDropdownHover, expandable: true, showIndicator: false, tabThroughPanel: true, getTabbableElements: () => hoverTabbableElements, contentOwnsPadding: true }, + pillHover: { element: () => createHover('default').element }, } : {}), open: () => { if (session) { @@ -155,6 +166,20 @@ interface IResolvedSessionIssue { export function buildSessionIssueSections(issues: readonly IResolvedSessionIssue[], session: IActiveSession | undefined, commandService: ICommandService, clipboardService: IClipboardService, openerService: IOpenerService, sessionsService: ISessionsService): readonly IChatPillSection[] { const entries = issues.map(({ ref, issue }) => { const title = issue?.title ?? ref.title; + let hoverTabbableElements: readonly HTMLElement[] = []; + const createHover = issue ? (density: 'default' | 'compact') => createIssueHover({ + owner: ref.owner, + repo: ref.repo, + number: ref.number, + ...getGitHubHoverLinkData(ref.owner, ref.repo, ref.uri, openerService), + issue, + density, + }) : undefined; + const createDropdownHover = createHover ? () => { + const hover = createHover('compact'); + hoverTabbableElements = hover.tabbableElements; + return hover.element; + } : undefined; const label = title ? localize('sessionChatPills.issueWithTitle', "Issue #{0}: {1}", ref.number, title) : localize('sessionChatPills.issue', "Issue #{0}", ref.number); @@ -171,16 +196,9 @@ export function buildSessionIssueSections(issues: readonly IResolvedSessionIssue })], ...getChatPillResourceLocation(ref.uri, label), ...(!issue && ref.title ? { tooltip: `${label}\n${ref.uri.toString(true)}` } : {}), - ...(issue ? { - pillHover: { - element: () => createIssueHoverElement({ - owner: ref.owner, - repo: ref.repo, - number: ref.number, - ...getGitHubRepositoryHoverData(ref.owner, ref.repo, openerService), - issue, - }), - }, + ...(createDropdownHover && createHover ? { + hover: { content: createDropdownHover, expandable: true, showIndicator: false, tabThroughPanel: true, getTabbableElements: () => hoverTabbableElements, contentOwnsPadding: true }, + pillHover: { element: () => createHover('default').element }, } : {}), open: () => { if (session) { @@ -421,6 +439,10 @@ export class SessionChatInputToolbar extends Disposable { return this._inputPills.getPillElements(); } + focusFirst(): boolean { + return this._inputPills.focusFirst(); + } + /** * Track the currently-viewed chat; the toolbar reflects that chat's last-turn * changes and status, resolving the owning session for provider gating and the diff --git a/src/vs/sessions/contrib/chat/browser/sessionsChatAccessibilityHelp.ts b/src/vs/sessions/contrib/chat/browser/sessionsChatAccessibilityHelp.ts index b690bf3c4b33a2..5ef12e0a1660ab 100644 --- a/src/vs/sessions/contrib/chat/browser/sessionsChatAccessibilityHelp.ts +++ b/src/vs/sessions/contrib/chat/browser/sessionsChatAccessibilityHelp.ts @@ -78,7 +78,7 @@ export class SessionsChatAccessibilityHelp implements IAccessibleViewImplementat content.push(localize('sessionsChat.contextReferences', "Type # in the chat input to attach context. Use #file to reference a file or folder, or #session to reference another agent session. Referencing a session together with the /troubleshoot command analyzes that session's logs instead of the current one. Accept a suggestion with Tab or Enter. Openable references appear as buttons above the input; activate one to open it, or use its Remove button to detach it.")); content.push(localize('sessionsChat.pastedText', "Long pasted text is stored as an attached text item and replaced in the input with a numbered inline reference.")); content.push(localize('sessionsChat.pasteAsText', "To paste the clipboard as plain text, without converting it to Markdown or storing it as an attachment, invoke Paste as Text{0}.", '')); - content.push(localize('sessionsChat.backgroundActivities', "Press Shift+Tab from the chat input to reach metadata and status pills above it, then press Enter or Space to activate a pill. Live browsers appear in their own pill, and the chat's subagents of any status appear in another. A pill with more than one entry opens a picker; use the up and down arrows to navigate, Enter to open an entry, and Escape to dismiss the picker and return focus to the pill.")); + content.push(localize('sessionsChat.backgroundActivities', "Press Shift+Tab from the chat input to reach metadata and status pills above it, use the left and right arrows to move between pills, and press Enter or Space to activate one. Live browsers appear in their own pill, and the chat's subagents of any status appear in another. A pill with more than one entry opens a picker. Use the up and down arrows to move between entries. When an entry has details, Tab moves through its row actions and detail links; Shift+Tab returns to the row action, and the up and down arrows continue moving between entries. Press Enter to open an entry, or Escape to dismiss the picker and return focus to the pill.")); content.push(localize('sessionsChat.conversations', "When multiple chats appear as tabs in a single group, the tab row replaces the session header and includes the session actions. Side-by-side chat groups retain the session header and keep their tab rows compact.")); content.push(localize('sessionsChat.sessionsListChats', "Sessions with multiple user-facing chats show those chats nested beneath the session in the Sessions list. Use the arrow keys to navigate the list and Enter to open a chat. Side chats and subagent chats are omitted from this nested list: side chats are reachable from the Side Chats dropdown in the session's overflow menu, and subagent chats open from their pills in the chat transcript.")); content.push(localize('sessionsChat.sessionsListChatContextMenu', "Open a nested chat's context menu to rename it, open it to the side, or, when supported, permanently delete it. Agent Host chats also offer Copy Link.")); diff --git a/src/vs/sessions/contrib/chat/test/browser/sessionChatInputToolbar.test.ts b/src/vs/sessions/contrib/chat/test/browser/sessionChatInputToolbar.test.ts index 185c3488fe0dba..dcced20a33f71f 100644 --- a/src/vs/sessions/contrib/chat/test/browser/sessionChatInputToolbar.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/sessionChatInputToolbar.test.ts @@ -30,7 +30,8 @@ import { ISessionChangesStatsCache } from '../../../../services/sessions/common/ import { BRANCH_CHANGES_CHANGESET_ID, ChatOriginKind, SESSION_CHANGES_CHANGESET_ID, SessionArtifactKind, SessionStatus, type IChat, type IGitHubIssueRef, type IGitHubPullRequestRef, type ISessionArtifact, type ISessionWorkspace } from '../../../../services/sessions/common/session.js'; import { IActiveSession, ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; import { ISessionChangesEditorOptions, ISessionChangesService } from '../../../changes/common/sessionChangesService.js'; -import { GitHubIssueState, GitHubPullRequestState, type IGitHubIssue, type IGitHubPullRequest } from '../../../github/common/types.js'; +import { getGitHubHoverDescription } from '../../../github/browser/githubHover.js'; +import { GitHubIssueState, GitHubIssueStateReason, GitHubPullRequestState, type IGitHubIssue, type IGitHubPullRequest } from '../../../github/common/types.js'; import type { IResolvedSessionPullRequest } from '../../../github/browser/pullRequestIconStatus.js'; import { IGitHubService } from '../../../github/browser/githubService.js'; import { GitHubPullRequestModel } from '../../../github/browser/models/githubPullRequestModel.js'; @@ -170,13 +171,14 @@ suite('SessionChatInputToolbar', () => { test('adds rich GitHub hovers only when live details are available', async () => { const commands: { readonly id: string; readonly args: readonly unknown[] }[] = []; + const clipboardWrites: string[] = []; const commandService = upcastPartial({ executeCommand: async (id, ...args) => { commands.push({ id, args }); return undefined; }, }); - const clipboardService = upcastPartial({ writeText: async () => { } }); + const clipboardService = upcastPartial({ writeText: async value => { clipboardWrites.push(value); } }); const openerService = upcastPartial({ open: async () => true }); const sessionsService = upcastPartial({ setActive: () => { } }); const pullRequestRef: IGitHubPullRequestRef = { @@ -190,7 +192,7 @@ suite('SessionChatInputToolbar', () => { number: pullRequestRef.number, title: 'Restore rich pill hovers', body: 'Provides detailed pull request context.', - state: GitHubPullRequestState.Open, + state: GitHubPullRequestState.Merged, author: { login: 'octocat', avatarUrl: '' }, headRef: 'feature/rich-hover', headSha: 'abc123', @@ -198,7 +200,7 @@ suite('SessionChatInputToolbar', () => { isDraft: false, createdAt: '2026-09-03T09:00:00Z', updatedAt: '2026-09-03T10:00:00Z', - mergedAt: undefined, + mergedAt: '2026-09-04T10:00:00Z', mergeable: true, mergeableState: 'clean', }; @@ -213,12 +215,12 @@ suite('SessionChatInputToolbar', () => { number: issueRef.number, title: 'Rich issue hover', body: 'Provides detailed issue context.', - state: GitHubIssueState.Open, - stateReason: undefined, + state: GitHubIssueState.Closed, + stateReason: GitHubIssueStateReason.Completed, author: { login: 'octocat', avatarUrl: '' }, createdAt: '2026-09-03T09:00:00Z', updatedAt: '2026-09-03T10:00:00Z', - closedAt: undefined, + closedAt: '2026-09-04T10:00:00Z', }; const pullRequestEntry = buildSessionPullRequestSections( [{ ref: pullRequestRef, pullRequest, icon: Codicon.gitPullRequest, status: {} }], @@ -259,8 +261,13 @@ suite('SessionChatInputToolbar', () => { } return await entry.pillHover.element(CancellationToken.None); }; + const renderDropdownHover = (entry: IChatPillEntry | undefined) => + typeof entry?.hover?.content === 'function' ? entry.hover.content() : undefined; const pullRequestHover = await renderHover(pullRequestEntry); const issueHover = await renderHover(issueEntry); + const pullRequestDropdownHover = renderDropdownHover(pullRequestEntry); + const issueDropdownHover = renderDropdownHover(issueEntry); + pullRequestHover?.querySelectorAll('.sessions-pr-hover-branch').forEach(branch => branch.click()); pullRequestEntry?.open(); unresolvedIssueEntry?.open(); @@ -268,19 +275,46 @@ suite('SessionChatInputToolbar', () => { pullRequest: { label: pullRequestEntry?.label, className: pullRequestHover?.className, + dropdownClassName: pullRequestDropdownHover?.className, + dropdownExpandable: pullRequestEntry?.hover?.expandable, + dropdownIndicator: pullRequestEntry?.hover?.showIndicator, + dropdownTabThroughPanel: pullRequestEntry?.hover?.tabThroughPanel, + dropdownTabbableElements: pullRequestEntry?.hover?.getTabbableElements?.().length, + dropdownContentOwnsPadding: pullRequestEntry?.hover?.contentOwnsPadding, repository: pullRequestHover?.querySelector('.sessions-pr-hover-repository')?.textContent, + reference: pullRequestHover?.querySelector('.sessions-pr-hover-reference')?.textContent, + referenceAriaLabel: pullRequestHover?.querySelector('.sessions-pr-hover-reference')?.getAttribute('aria-label'), + status: pullRequestHover?.querySelector('.sessions-pr-hover-status')?.textContent, + statusKind: pullRequestHover?.querySelector('.sessions-pr-hover-status')?.dataset.state, + date: pullRequestHover?.querySelector('.sessions-pr-hover-date')?.textContent, title: pullRequestHover?.querySelector('.sessions-pr-hover-title')?.textContent, description: pullRequestHover?.querySelector('.sessions-pr-hover-description-content')?.textContent, branches: [...pullRequestHover?.querySelectorAll('.sessions-pr-hover-branch') ?? []].map(element => element.textContent), + branchControls: [...pullRequestHover?.querySelectorAll('.sessions-pr-hover-branch') ?? []].map(element => ({ + tagName: element.tagName, + ariaLabel: element.getAttribute('aria-label'), + })), unresolvedLabel: unresolvedPullRequestEntry?.label, unresolvedAriaLabel: unresolvedPullRequestEntry?.ariaLabel, unresolvedTooltip: unresolvedPullRequestEntry?.tooltip, unresolvedHover: unresolvedPullRequestEntry?.pillHover, + clipboardWrites, }, issue: { label: issueEntry?.label, className: issueHover?.className, + dropdownClassName: issueDropdownHover?.className, + dropdownExpandable: issueEntry?.hover?.expandable, + dropdownIndicator: issueEntry?.hover?.showIndicator, + dropdownTabThroughPanel: issueEntry?.hover?.tabThroughPanel, + dropdownTabbableElements: issueEntry?.hover?.getTabbableElements?.().length, + dropdownContentOwnsPadding: issueEntry?.hover?.contentOwnsPadding, repository: issueHover?.querySelector('.sessions-issue-hover-repository')?.textContent, + reference: issueHover?.querySelector('.sessions-issue-hover-reference')?.textContent, + referenceAriaLabel: issueHover?.querySelector('.sessions-issue-hover-reference')?.getAttribute('aria-label'), + status: issueHover?.querySelector('.sessions-issue-hover-status')?.textContent, + statusKind: issueHover?.querySelector('.sessions-issue-hover-status')?.dataset.state, + date: issueHover?.querySelector('.sessions-issue-hover-date')?.textContent, title: issueHover?.querySelector('.sessions-issue-hover-title')?.textContent, description: issueHover?.querySelector('.sessions-issue-hover-description-content')?.textContent, unresolvedLabel: unresolvedIssueEntry?.label, @@ -293,19 +327,46 @@ suite('SessionChatInputToolbar', () => { pullRequest: { label: 'Pull Request #332982: Restore rich pill hovers', className: 'sessions-pr-hover', + dropdownClassName: 'sessions-pr-hover compact', + dropdownExpandable: true, + dropdownIndicator: false, + dropdownTabThroughPanel: true, + dropdownTabbableElements: 4, + dropdownContentOwnsPadding: true, repository: 'microsoft/vscode', + reference: '#332982', + referenceAriaLabel: 'Pull Request #332982', + status: 'Merged', + statusKind: 'merged', + date: 'Sep 4', title: 'Restore rich pill hovers', description: 'Provides detailed pull request context.', branches: ['main', 'feature/rich-hover'], + branchControls: [ + { tagName: 'BUTTON', ariaLabel: 'Copy base branch main' }, + { tagName: 'BUTTON', ariaLabel: 'Copy head branch feature/rich-hover' }, + ], unresolvedLabel: 'Pull Request #332982: Recorded pull request title', unresolvedAriaLabel: 'Open Pull Request #332982: Recorded pull request title', unresolvedTooltip: 'Pull Request #332982: Recorded pull request title\nhttps://github.com/microsoft/vscode/pull/332982', unresolvedHover: undefined, + clipboardWrites: ['main', 'feature/rich-hover'], }, issue: { label: 'Issue #42: Rich issue hover', className: 'sessions-issue-hover', - repository: 'microsoft/vscode#42', + dropdownClassName: 'sessions-issue-hover compact', + dropdownExpandable: true, + dropdownIndicator: false, + dropdownTabThroughPanel: true, + dropdownTabbableElements: 2, + dropdownContentOwnsPadding: true, + repository: 'microsoft/vscode', + reference: '#42', + referenceAriaLabel: 'Issue #42', + status: 'Closed', + statusKind: 'closed', + date: 'Sep 4', title: 'Rich issue hover', description: 'Provides detailed issue context.', unresolvedLabel: 'Issue #42: Recorded issue title', @@ -326,6 +387,33 @@ suite('SessionChatInputToolbar', () => { }); }); + test('bounds and normalizes GitHub hover descriptions for assistive technology', () => { + const description = getGitHubHoverDescription(`\n## Summary\n\n${'Useful context with [documentation](https://example.com). '.repeat(8)}`, 'No description provided.'); + const unicodeDescription = getGitHubHoverDescription(`${'a'.repeat(198)}😀xy`, 'No description provided.'); + + assert.deepStrictEqual({ + startsWithReadableText: description.startsWith('Summary Useful context with documentation.'), + containsMarkdownSyntax: /\n## Summary\n\n${'Useful context with [documentation](https://example.com). '.repeat(8)}`, 'No description provided.'); const unicodeDescription = getGitHubHoverDescription(`${'a'.repeat(198)}😀xy`, 'No description provided.'); + const title = getGitHubHoverTitle(`${'a'.repeat(78)}😀xy`); + const titleParts = getGitHubHoverTitleParts('A title ending in context'); + const singleTokenTitleParts = getGitHubHoverTitleParts('a'.repeat(100)); assert.deepStrictEqual({ startsWithReadableText: description.startsWith('Summary Useful context with documentation.'), @@ -468,6 +475,12 @@ suite('SessionChatInputToolbar', () => { endsAtCodePointBoundary: unicodeDescription.endsWith('😀…'), containsReplacementCharacter: unicodeDescription.includes('�'), }, + title: { + codePoints: Array.from(title).length, + endsAtCodePointBoundary: title.endsWith('😀…'), + }, + titleParts, + singleTokenTitleParts, }, { startsWithReadableText: true, containsMarkdownSyntax: false, @@ -478,6 +491,12 @@ suite('SessionChatInputToolbar', () => { endsAtCodePointBoundary: true, containsReplacementCharacter: false, }, + title: { + codePoints: 80, + endsAtCodePointBoundary: true, + }, + titleParts: { leading: 'A title ending in ', trailing: 'context' }, + singleTokenTitleParts: { leading: `${'a'.repeat(79)}…`, trailing: undefined }, }); }); diff --git a/src/vs/sessions/contrib/github/browser/githubHover.ts b/src/vs/sessions/contrib/github/browser/githubHover.ts index ad498241936b09..377a039dbd8dc2 100644 --- a/src/vs/sessions/contrib/github/browser/githubHover.ts +++ b/src/vs/sessions/contrib/github/browser/githubHover.ts @@ -9,15 +9,28 @@ import { MarkdownString } from '../../../../base/common/htmlContent.js'; import { language } from '../../../../base/common/platform.js'; const MAX_DESCRIPTION_LENGTH = 200; +const MAX_TITLE_LENGTH = 80; const githubHoverDateFormatter = safeIntl.DateTimeFormat(language, { month: 'short', day: 'numeric' }); export function getGitHubHoverDescription(body: string, fallback: string): string { const description = renderAsPlaintext(new MarkdownString(body), { omitMarkdownSyntax: true }).replace(/\s+/g, ' ').trim() || fallback; - const characters = Array.from(description); - if (characters.length <= MAX_DESCRIPTION_LENGTH) { - return description; + return truncateGitHubHoverText(description, MAX_DESCRIPTION_LENGTH); +} + +export function getGitHubHoverTitle(title: string): string { + return truncateGitHubHoverText(title, MAX_TITLE_LENGTH); +} + +export function getGitHubHoverTitleParts(title: string): { readonly leading: string; readonly trailing: string | undefined } { + const visibleTitle = getGitHubHoverTitle(title); + const lastSpace = visibleTitle.lastIndexOf(' '); + if (lastSpace < 0 || Array.from(visibleTitle.slice(lastSpace + 1)).length > 20) { + return { leading: visibleTitle, trailing: undefined }; } - return `${characters.slice(0, MAX_DESCRIPTION_LENGTH - 1).join('').trimEnd()}…`; + return { + leading: visibleTitle.slice(0, lastSpace + 1), + trailing: visibleTitle.slice(lastSpace + 1), + }; } export function getGitHubHoverDate(value: string | undefined): string | undefined { @@ -32,3 +45,11 @@ export function getGitHubHoverDate(value: string | undefined): string | undefine return githubHoverDateFormatter.value.format(date); } + +function truncateGitHubHoverText(value: string, maxLength: number): string { + const characters = Array.from(value); + if (characters.length <= maxLength) { + return value; + } + return `${characters.slice(0, maxLength - 1).join('').trimEnd()}…`; +} diff --git a/src/vs/sessions/contrib/github/browser/issueHover.ts b/src/vs/sessions/contrib/github/browser/issueHover.ts index a9b544db9bceee..2df499c00dbbd7 100644 --- a/src/vs/sessions/contrib/github/browser/issueHover.ts +++ b/src/vs/sessions/contrib/github/browser/issueHover.ts @@ -8,7 +8,7 @@ import './media/issueHover.css'; import { $, append } from '../../../../base/browser/dom.js'; import { localize } from '../../../../nls.js'; import { GitHubIssueState, GitHubIssueStateReason, IGitHubIssue } from '../common/types.js'; -import { getGitHubHoverDate, getGitHubHoverDescription } from './githubHover.js'; +import { getGitHubHoverDate, getGitHubHoverDescription, getGitHubHoverTitleParts } from './githubHover.js'; export interface IIssueHoverData { readonly owner: string; @@ -40,8 +40,12 @@ export function createIssueHover(data: IIssueHoverData): IIssueHover { const title = data.issue.title || localize('agentSessions.issueHover.titleFallback', "Issue #{0}", data.number); const titleElement = append(hoverElement, $('.sessions-issue-hover-title')); - append(titleElement, $('.sessions-issue-hover-title-content', undefined, title)); - const referenceLink = appendHoverLink(titleElement, 'sessions-issue-hover-reference', data.referenceHref, `#${data.number}`, data.onDidClickReference, localize('agentSessions.issueHover.reference', "Issue #{0}", data.number)); + const titleContent = append(titleElement, $('.sessions-issue-hover-title-content')); + const titleParts = getGitHubHoverTitleParts(title); + titleContent.append(titleParts.leading); + const titleTail = titleParts.trailing === undefined ? titleContent : append(titleContent, $('.sessions-issue-hover-title-tail')); + titleTail.append(titleParts.trailing === undefined ? '\u00a0' : `${titleParts.trailing}\u00a0`); + const referenceLink = appendHoverLink(titleTail, 'sessions-issue-hover-reference', data.referenceHref, `#${data.number}`, data.onDidClickReference, localize('agentSessions.issueHover.reference', "Issue #{0}", data.number)); titleElement.title = title; const statusRow = append(hoverElement, $('.sessions-issue-hover-status-row')); diff --git a/src/vs/sessions/contrib/github/browser/media/issueHover.css b/src/vs/sessions/contrib/github/browser/media/issueHover.css index 41b2be377cc5ca..098142006a2328 100644 --- a/src/vs/sessions/contrib/github/browser/media/issueHover.css +++ b/src/vs/sessions/contrib/github/browser/media/issueHover.css @@ -50,9 +50,6 @@ } .sessions-issue-hover-title { - display: flex; - align-items: baseline; - gap: var(--vscode-spacing-size40); min-width: 0; font-size: var(--vscode-fontSize-body1); font-weight: var(--vscode-fontWeight-semiBold, 600); @@ -60,18 +57,19 @@ } .sessions-issue-hover-title-content { + display: block; min-width: 0; - display: -webkit-box; - line-clamp: 2; - -webkit-box-orient: vertical; - -webkit-line-clamp: 2; - overflow: hidden; overflow-wrap: anywhere; } +.sessions-issue-hover-title-tail { + white-space: nowrap; +} + .sessions-issue-hover .sessions-issue-hover-reference { - flex-shrink: 0; font-weight: var(--vscode-fontWeight-regular); + overflow-wrap: normal; + white-space: nowrap; } .sessions-issue-hover-author { diff --git a/src/vs/sessions/contrib/github/browser/media/pullRequestHover.css b/src/vs/sessions/contrib/github/browser/media/pullRequestHover.css index bb9d0249eab849..02205162786a09 100644 --- a/src/vs/sessions/contrib/github/browser/media/pullRequestHover.css +++ b/src/vs/sessions/contrib/github/browser/media/pullRequestHover.css @@ -50,9 +50,6 @@ } .sessions-pr-hover-title { - display: flex; - align-items: baseline; - gap: var(--vscode-spacing-size40); min-width: 0; font-size: var(--vscode-fontSize-body1); font-weight: var(--vscode-fontWeight-semiBold, 600); @@ -60,18 +57,19 @@ } .sessions-pr-hover-title-content { + display: block; min-width: 0; - display: -webkit-box; - line-clamp: 2; - -webkit-box-orient: vertical; - -webkit-line-clamp: 2; - overflow: hidden; overflow-wrap: anywhere; } +.sessions-pr-hover-title-tail { + white-space: nowrap; +} + .sessions-pr-hover .sessions-pr-hover-reference { - flex-shrink: 0; font-weight: var(--vscode-fontWeight-regular); + overflow-wrap: normal; + white-space: nowrap; } .sessions-pr-hover-author { diff --git a/src/vs/sessions/contrib/github/browser/pullRequestHover.ts b/src/vs/sessions/contrib/github/browser/pullRequestHover.ts index 55f17e8e478fb3..0ff93ecec103a8 100644 --- a/src/vs/sessions/contrib/github/browser/pullRequestHover.ts +++ b/src/vs/sessions/contrib/github/browser/pullRequestHover.ts @@ -8,7 +8,7 @@ import './media/pullRequestHover.css'; import { $, append } from '../../../../base/browser/dom.js'; import { localize } from '../../../../nls.js'; import { GitHubPullRequestState, IGitHubPullRequest } from '../common/types.js'; -import { getGitHubHoverDate, getGitHubHoverDescription } from './githubHover.js'; +import { getGitHubHoverDate, getGitHubHoverDescription, getGitHubHoverTitleParts } from './githubHover.js'; export interface IPullRequestHoverData { readonly owner: string; @@ -42,8 +42,12 @@ export function createPullRequestHover(data: IPullRequestHoverData): IPullReques const title = data.pullRequest.title || localize('agentSessions.pullRequestHover.titleFallback', "Pull Request #{0}", data.number); const titleElement = append(hoverElement, $('.sessions-pr-hover-title')); - append(titleElement, $('.sessions-pr-hover-title-content', undefined, title)); - const referenceLink = appendHoverLink(titleElement, 'sessions-pr-hover-reference', data.referenceHref, `#${data.number}`, data.onDidClickReference, localize('agentSessions.pullRequestHover.reference', "Pull Request #{0}", data.number)); + const titleContent = append(titleElement, $('.sessions-pr-hover-title-content')); + const titleParts = getGitHubHoverTitleParts(title); + titleContent.append(titleParts.leading); + const titleTail = titleParts.trailing === undefined ? titleContent : append(titleContent, $('.sessions-pr-hover-title-tail')); + titleTail.append(titleParts.trailing === undefined ? '\u00a0' : `${titleParts.trailing}\u00a0`); + const referenceLink = appendHoverLink(titleTail, 'sessions-pr-hover-reference', data.referenceHref, `#${data.number}`, data.onDidClickReference, localize('agentSessions.pullRequestHover.reference', "Pull Request #{0}", data.number)); titleElement.title = title; const statusRow = append(hoverElement, $('.sessions-pr-hover-status-row')); From 519e66b4fc8f09829f548cf9e7509f2c69ed6d13 Mon Sep 17 00:00:00 2001 From: Cherry Wang Date: Thu, 10 Sep 2026 20:04:54 -0700 Subject: [PATCH 07/11] sessions: address GitHub hover review feedback Use measured submenu height for positioning and reveal the full bounded GitHub title when keyboard focus reaches its linked reference. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../actionWidget/browser/actionList.ts | 2 +- .../browser/sessionChatInputToolbar.test.ts | 42 +++++++++++++++++++ .../contrib/github/browser/githubHover.ts | 32 ++++++++++++++ .../contrib/github/browser/issueHover.ts | 11 +++-- .../github/browser/pullRequestHover.ts | 11 +++-- 5 files changed, 85 insertions(+), 13 deletions(-) diff --git a/src/vs/platform/actionWidget/browser/actionList.ts b/src/vs/platform/actionWidget/browser/actionList.ts index 057fae7c9ae12d..d843b57ff5baa5 100644 --- a/src/vs/platform/actionWidget/browser/actionList.ts +++ b/src/vs/platform/actionWidget/browser/actionList.ts @@ -2561,7 +2561,7 @@ export class ActionListWidget extends Disposable { : edgeRect.left - parentRect.left - panelWidth - gap; this._submenuContainer.style.left = `${left / zoom}px`; - const panelHeight = alignToParent || preserveVerticalPosition ? panelRect.height : totalHeight + (hoverHeader?.offsetHeight ?? 0); + const panelHeight = panelRect.height; if (preserveVerticalPosition) { openingPanelHeight ??= panelHeight / zoom; } diff --git a/src/vs/sessions/contrib/chat/test/browser/sessionChatInputToolbar.test.ts b/src/vs/sessions/contrib/chat/test/browser/sessionChatInputToolbar.test.ts index 6ec852cfc311a1..e07d0b2df9077f 100644 --- a/src/vs/sessions/contrib/chat/test/browser/sessionChatInputToolbar.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/sessionChatInputToolbar.test.ts @@ -31,6 +31,7 @@ import { BRANCH_CHANGES_CHANGESET_ID, ChatOriginKind, SESSION_CHANGES_CHANGESET_ import { IActiveSession, ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; import { ISessionChangesEditorOptions, ISessionChangesService } from '../../../changes/common/sessionChangesService.js'; import { getGitHubHoverDate, getGitHubHoverDescription, getGitHubHoverTitle, getGitHubHoverTitleParts } from '../../../github/browser/githubHover.js'; +import { createIssueHoverElement } from '../../../github/browser/issueHover.js'; import { GitHubIssueState, GitHubIssueStateReason, GitHubPullRequestState, type IGitHubIssue, type IGitHubPullRequest } from '../../../github/common/types.js'; import type { IResolvedSessionPullRequest } from '../../../github/browser/pullRequestIconStatus.js'; import { IGitHubService } from '../../../github/browser/githubService.js'; @@ -512,6 +513,47 @@ suite('SessionChatInputToolbar', () => { }); }); + test('reveals a bounded title when keyboard focus reaches its reference link', () => { + const title = `${'Long issue title '.repeat(8)}ending`; + const hover = createIssueHoverElement({ + owner: 'microsoft', + repo: 'vscode', + number: 42, + repositoryHref: 'https://github.com/microsoft/vscode', + referenceHref: 'https://github.com/microsoft/vscode/issues/42', + issue: { + number: 42, + title, + body: '', + state: GitHubIssueState.Open, + stateReason: undefined, + author: { login: 'octocat', avatarUrl: '' }, + createdAt: '2026-09-03T10:00:00Z', + updatedAt: '2026-09-03T10:00:00Z', + closedAt: undefined, + }, + density: 'compact', + }); + const titleContent = hover.querySelector('.sessions-issue-hover-title-content'); + const reference = hover.querySelector('.sessions-issue-hover-reference'); + const bounded = titleContent?.textContent; + reference?.dispatchEvent(new FocusEvent('focus')); + const focused = titleContent?.textContent; + reference?.dispatchEvent(new FocusEvent('blur')); + + assert.deepStrictEqual({ + bounded, + focused, + restored: titleContent?.textContent, + fullTitle: hover.querySelector('.sessions-issue-hover-title')?.getAttribute('title'), + }, { + bounded: `${getGitHubHoverTitle(title)}\u00a0#42`, + focused: `${title}\u00a0#42`, + restored: `${getGitHubHoverTitle(title)}\u00a0#42`, + fullTitle: title, + }); + }); + test('hides the pills in a subagent chat', () => { const { instantiationService, visibility } = createServices(); const chat = upcastPartial({ diff --git a/src/vs/sessions/contrib/github/browser/githubHover.ts b/src/vs/sessions/contrib/github/browser/githubHover.ts index 377a039dbd8dc2..8a8a2f667d38a6 100644 --- a/src/vs/sessions/contrib/github/browser/githubHover.ts +++ b/src/vs/sessions/contrib/github/browser/githubHover.ts @@ -33,6 +33,38 @@ export function getGitHubHoverTitleParts(title: string): { readonly leading: str }; } +interface IGitHubHoverTitleLayout { + readonly referenceContainer: HTMLElement; + readonly showFullTitle: () => void; + readonly showBoundedTitle: () => void; +} + +export function appendGitHubHoverTitle(container: HTMLElement, title: string, tailClassName: string): IGitHubHoverTitleLayout { + const titleParts = getGitHubHoverTitleParts(title); + const leadingText = container.ownerDocument.createTextNode(titleParts.leading); + container.append(leadingText); + + const referenceContainer = titleParts.trailing === undefined ? container : container.ownerDocument.createElement('span'); + if (referenceContainer !== container) { + referenceContainer.className = tailClassName; + container.append(referenceContainer); + } + const trailingText = container.ownerDocument.createTextNode(titleParts.trailing === undefined ? '\u00a0' : `${titleParts.trailing}\u00a0`); + referenceContainer.append(trailingText); + + return { + referenceContainer, + showFullTitle: () => { + leadingText.nodeValue = title; + trailingText.nodeValue = '\u00a0'; + }, + showBoundedTitle: () => { + leadingText.nodeValue = titleParts.leading; + trailingText.nodeValue = titleParts.trailing === undefined ? '\u00a0' : `${titleParts.trailing}\u00a0`; + }, + }; +} + export function getGitHubHoverDate(value: string | undefined): string | undefined { if (!value) { return undefined; diff --git a/src/vs/sessions/contrib/github/browser/issueHover.ts b/src/vs/sessions/contrib/github/browser/issueHover.ts index 2df499c00dbbd7..64d69e5fa25766 100644 --- a/src/vs/sessions/contrib/github/browser/issueHover.ts +++ b/src/vs/sessions/contrib/github/browser/issueHover.ts @@ -8,7 +8,7 @@ import './media/issueHover.css'; import { $, append } from '../../../../base/browser/dom.js'; import { localize } from '../../../../nls.js'; import { GitHubIssueState, GitHubIssueStateReason, IGitHubIssue } from '../common/types.js'; -import { getGitHubHoverDate, getGitHubHoverDescription, getGitHubHoverTitleParts } from './githubHover.js'; +import { appendGitHubHoverTitle, getGitHubHoverDate, getGitHubHoverDescription } from './githubHover.js'; export interface IIssueHoverData { readonly owner: string; @@ -41,11 +41,10 @@ export function createIssueHover(data: IIssueHoverData): IIssueHover { const title = data.issue.title || localize('agentSessions.issueHover.titleFallback', "Issue #{0}", data.number); const titleElement = append(hoverElement, $('.sessions-issue-hover-title')); const titleContent = append(titleElement, $('.sessions-issue-hover-title-content')); - const titleParts = getGitHubHoverTitleParts(title); - titleContent.append(titleParts.leading); - const titleTail = titleParts.trailing === undefined ? titleContent : append(titleContent, $('.sessions-issue-hover-title-tail')); - titleTail.append(titleParts.trailing === undefined ? '\u00a0' : `${titleParts.trailing}\u00a0`); - const referenceLink = appendHoverLink(titleTail, 'sessions-issue-hover-reference', data.referenceHref, `#${data.number}`, data.onDidClickReference, localize('agentSessions.issueHover.reference', "Issue #{0}", data.number)); + const titleLayout = appendGitHubHoverTitle(titleContent, title, 'sessions-issue-hover-title-tail'); + const referenceLink = appendHoverLink(titleLayout.referenceContainer, 'sessions-issue-hover-reference', data.referenceHref, `#${data.number}`, data.onDidClickReference, localize('agentSessions.issueHover.reference', "Issue #{0}", data.number)); + referenceLink.onfocus = titleLayout.showFullTitle; + referenceLink.onblur = titleLayout.showBoundedTitle; titleElement.title = title; const statusRow = append(hoverElement, $('.sessions-issue-hover-status-row')); diff --git a/src/vs/sessions/contrib/github/browser/pullRequestHover.ts b/src/vs/sessions/contrib/github/browser/pullRequestHover.ts index 0ff93ecec103a8..4dd338d6d7b721 100644 --- a/src/vs/sessions/contrib/github/browser/pullRequestHover.ts +++ b/src/vs/sessions/contrib/github/browser/pullRequestHover.ts @@ -8,7 +8,7 @@ import './media/pullRequestHover.css'; import { $, append } from '../../../../base/browser/dom.js'; import { localize } from '../../../../nls.js'; import { GitHubPullRequestState, IGitHubPullRequest } from '../common/types.js'; -import { getGitHubHoverDate, getGitHubHoverDescription, getGitHubHoverTitleParts } from './githubHover.js'; +import { appendGitHubHoverTitle, getGitHubHoverDate, getGitHubHoverDescription } from './githubHover.js'; export interface IPullRequestHoverData { readonly owner: string; @@ -43,11 +43,10 @@ export function createPullRequestHover(data: IPullRequestHoverData): IPullReques const title = data.pullRequest.title || localize('agentSessions.pullRequestHover.titleFallback', "Pull Request #{0}", data.number); const titleElement = append(hoverElement, $('.sessions-pr-hover-title')); const titleContent = append(titleElement, $('.sessions-pr-hover-title-content')); - const titleParts = getGitHubHoverTitleParts(title); - titleContent.append(titleParts.leading); - const titleTail = titleParts.trailing === undefined ? titleContent : append(titleContent, $('.sessions-pr-hover-title-tail')); - titleTail.append(titleParts.trailing === undefined ? '\u00a0' : `${titleParts.trailing}\u00a0`); - const referenceLink = appendHoverLink(titleTail, 'sessions-pr-hover-reference', data.referenceHref, `#${data.number}`, data.onDidClickReference, localize('agentSessions.pullRequestHover.reference', "Pull Request #{0}", data.number)); + const titleLayout = appendGitHubHoverTitle(titleContent, title, 'sessions-pr-hover-title-tail'); + const referenceLink = appendHoverLink(titleLayout.referenceContainer, 'sessions-pr-hover-reference', data.referenceHref, `#${data.number}`, data.onDidClickReference, localize('agentSessions.pullRequestHover.reference', "Pull Request #{0}", data.number)); + referenceLink.onfocus = titleLayout.showFullTitle; + referenceLink.onblur = titleLayout.showBoundedTitle; titleElement.title = title; const statusRow = append(hoverElement, $('.sessions-pr-hover-status-row')); From e4c57da7ccf0554496d32cebec54d398fb3418ff Mon Sep 17 00:00:00 2001 From: Cherry Wang Date: Thu, 10 Sep 2026 20:49:54 -0700 Subject: [PATCH 08/11] hover: align footer action icons and labels Render shared hover actions as centered inline flex rows so codicons and text use the same vertical center. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/vs/base/browser/ui/hover/hoverWidget.css | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/vs/base/browser/ui/hover/hoverWidget.css b/src/vs/base/browser/ui/hover/hoverWidget.css index 07151910490748..d14fc7b2c2e0a2 100644 --- a/src/vs/base/browser/ui/hover/hoverWidget.css +++ b/src/vs/base/browser/ui/hover/hoverWidget.css @@ -137,9 +137,13 @@ text-overflow: ellipsis; } +.monaco-hover .hover-row.status-bar .actions .action-container .action { + display: inline-flex; + align-items: center; +} + .monaco-hover .hover-row.status-bar .actions .action-container .action .icon { padding-right: 4px; - vertical-align: middle; font-size: inherit; } From f4930362acebac04f9dbf7ca30f118bc9b2e90a9 Mon Sep 17 00:00:00 2001 From: Cherry Wang Date: Thu, 10 Sep 2026 21:15:59 -0700 Subject: [PATCH 09/11] sessions: align GitHub reference metadata Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../chat/browser/sessionChatInputToolbar.ts | 16 +++++++---- .../browser/sessionChatInputToolbar.test.ts | 28 ++++++++++++++++--- .../contrib/github/browser/issueHover.ts | 13 +++++++-- .../github/browser/media/issueHover.css | 16 +++++++++++ .../github/browser/media/pullRequestHover.css | 16 +++++++++++ .../github/browser/pullRequestHover.ts | 13 +++++++-- src/vs/workbench/browser/chatDropdownPill.ts | 4 ++- src/vs/workbench/browser/chatPills.ts | 4 +++ src/vs/workbench/browser/media/chatPills.css | 10 +++++++ .../workbench/test/browser/chatPills.test.ts | 21 ++++++++++++-- 10 files changed, 123 insertions(+), 18 deletions(-) diff --git a/src/vs/sessions/contrib/chat/browser/sessionChatInputToolbar.ts b/src/vs/sessions/contrib/chat/browser/sessionChatInputToolbar.ts index 296cdb2dae6dab..38279a2ae95e98 100644 --- a/src/vs/sessions/contrib/chat/browser/sessionChatInputToolbar.ts +++ b/src/vs/sessions/contrib/chat/browser/sessionChatInputToolbar.ts @@ -103,9 +103,10 @@ export function buildSessionPullRequestSections(pullRequests: readonly IResolved hoverTabbableElements = hover.tabbableElements; return hover.element; } : undefined; - const label = title + const resourceLabel = title ? localize('sessionChatPills.pullRequestWithTitle', "Pull Request #{0}: {1}", ref.number, title) : localize('sessionChatPills.pullRequest', "Pull Request #{0}", ref.number); + const label = title ?? resourceLabel; const resolvedIcon = icon ?? computePullRequestIcon('open'); const attention = getPullRequestAttention(resolvedIcon, status); const pullRequestState = pullRequest?.state ?? ref.liveState ?? ref.state ?? getPullRequestStatusFromIcon(resolvedIcon) ?? 'open'; @@ -124,6 +125,7 @@ export function buildSessionPullRequestSections(pullRequests: readonly IResolved return { id: ref.uri.toString(), label, + ...(title ? { badge: `#${ref.number}`, className: 'chat-pill-github-reference' } : {}), pillLabel: `#${ref.number}`, icon: resolvedIcon, pullRequestState: state, @@ -139,9 +141,9 @@ export function buildSessionPullRequestSections(pullRequests: readonly IResolved class: ThemeIcon.asClassName(Codicon.copy), run: () => clipboardService.writeText(ref.uri.toString(true)), })], - ...getChatPillResourceLocation(ref.uri, label), + ...getChatPillResourceLocation(ref.uri, resourceLabel), ariaDescription: localize('sessionChatPills.pullRequestDescription', "{0}. {1}", stateDescription, ref.uri.toString(true)), - ...(!pullRequest && ref.title ? { tooltip: `${label}\n${ref.uri.toString(true)}` } : {}), + ...(!pullRequest && ref.title ? { tooltip: `${resourceLabel}\n${ref.uri.toString(true)}` } : {}), ...(createDropdownHover && createHover ? { hover: { content: createDropdownHover, expandable: true, showIndicator: false, tabThroughPanel: true, getTabbableElements: () => hoverTabbableElements, contentOwnsPadding: true }, pillHover: { element: () => createHover('default').element, contentOwnsPadding: true }, @@ -180,12 +182,14 @@ export function buildSessionIssueSections(issues: readonly IResolvedSessionIssue hoverTabbableElements = hover.tabbableElements; return hover.element; } : undefined; - const label = title + const resourceLabel = title ? localize('sessionChatPills.issueWithTitle', "Issue #{0}: {1}", ref.number, title) : localize('sessionChatPills.issue', "Issue #{0}", ref.number); + const label = title ?? resourceLabel; return { id: ref.uri.toString(), label, + ...(title ? { badge: `#${ref.number}`, className: 'chat-pill-github-reference' } : {}), pillLabel: `#${ref.number}`, icon: issue ? computeIssueIcon(issue.state, issue.stateReason) : computeIssueIcon(GitHubIssueState.Open, undefined), toolbarActions: [toAction({ @@ -194,8 +198,8 @@ export function buildSessionIssueSections(issues: readonly IResolvedSessionIssue class: ThemeIcon.asClassName(Codicon.copy), run: () => clipboardService.writeText(ref.uri.toString(true)), })], - ...getChatPillResourceLocation(ref.uri, label), - ...(!issue && ref.title ? { tooltip: `${label}\n${ref.uri.toString(true)}` } : {}), + ...getChatPillResourceLocation(ref.uri, resourceLabel), + ...(!issue && ref.title ? { tooltip: `${resourceLabel}\n${ref.uri.toString(true)}` } : {}), ...(createDropdownHover && createHover ? { hover: { content: createDropdownHover, expandable: true, showIndicator: false, tabThroughPanel: true, getTabbableElements: () => hoverTabbableElements, contentOwnsPadding: true }, pillHover: { element: () => createHover('default').element, contentOwnsPadding: true }, diff --git a/src/vs/sessions/contrib/chat/test/browser/sessionChatInputToolbar.test.ts b/src/vs/sessions/contrib/chat/test/browser/sessionChatInputToolbar.test.ts index e07d0b2df9077f..9b8e28d3345529 100644 --- a/src/vs/sessions/contrib/chat/test/browser/sessionChatInputToolbar.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/sessionChatInputToolbar.test.ts @@ -293,6 +293,8 @@ suite('SessionChatInputToolbar', () => { assert.deepStrictEqual({ pullRequest: { label: pullRequestEntry?.label, + badge: pullRequestEntry?.badge, + rowClassName: pullRequestEntry?.className, pillHoverContentOwnsPadding: isManagedHoverTooltipHTMLElement(pullRequestEntry?.pillHover) ? pullRequestEntry.pillHover.contentOwnsPadding : undefined, className: pullRequestHover?.className, contentOrder: [...pullRequestHover?.children ?? []].map(element => element.className), @@ -310,6 +312,7 @@ suite('SessionChatInputToolbar', () => { referenceAriaLabel: pullRequestHover?.querySelector('.sessions-pr-hover-reference')?.getAttribute('aria-label'), status: pullRequestHover?.querySelector('.sessions-pr-hover-status')?.textContent, statusKind: pullRequestHover?.querySelector('.sessions-pr-hover-status')?.dataset.state, + statusIconAriaHidden: pullRequestHover?.querySelector('.sessions-pr-hover-status .codicon')?.getAttribute('aria-hidden'), date: pullRequestHover?.querySelector('.sessions-pr-hover-date')?.textContent, title: pullRequestHover?.querySelector('.sessions-pr-hover-title-content')?.textContent?.replace('#332982', '').trim(), titleTailOrder: [...pullRequestHover?.querySelector('.sessions-pr-hover-title-tail')?.childNodes ?? []].map(node => node.nodeType === 3 ? '#text' : (node as HTMLElement).className), @@ -322,6 +325,8 @@ suite('SessionChatInputToolbar', () => { ariaLabel: element.getAttribute('aria-label'), })), unresolvedLabel: unresolvedPullRequestEntry?.label, + unresolvedBadge: unresolvedPullRequestEntry?.badge, + unresolvedRowClassName: unresolvedPullRequestEntry?.className, unresolvedAriaLabel: unresolvedPullRequestEntry?.ariaLabel, unresolvedTooltip: unresolvedPullRequestEntry?.tooltip, unresolvedHover: unresolvedPullRequestEntry?.pillHover, @@ -329,6 +334,8 @@ suite('SessionChatInputToolbar', () => { }, issue: { label: issueEntry?.label, + badge: issueEntry?.badge, + rowClassName: issueEntry?.className, pillHoverContentOwnsPadding: isManagedHoverTooltipHTMLElement(issueEntry?.pillHover) ? issueEntry.pillHover.contentOwnsPadding : undefined, className: issueHover?.className, contentOrder: [...issueHover?.children ?? []].map(element => element.className), @@ -346,6 +353,7 @@ suite('SessionChatInputToolbar', () => { referenceAriaLabel: issueHover?.querySelector('.sessions-issue-hover-reference')?.getAttribute('aria-label'), status: issueHover?.querySelector('.sessions-issue-hover-status')?.textContent, statusKind: issueHover?.querySelector('.sessions-issue-hover-status')?.dataset.state, + statusIconAriaHidden: issueHover?.querySelector('.sessions-issue-hover-status .codicon')?.getAttribute('aria-hidden'), date: issueHover?.querySelector('.sessions-issue-hover-date')?.textContent, title: issueHover?.querySelector('.sessions-issue-hover-title-content')?.textContent?.replace('#42', '').trim(), titleTailOrder: [...issueHover?.querySelector('.sessions-issue-hover-title-tail')?.childNodes ?? []].map(node => node.nodeType === 3 ? '#text' : (node as HTMLElement).className), @@ -353,6 +361,8 @@ suite('SessionChatInputToolbar', () => { description: issueHover?.querySelector('.sessions-issue-hover-description-content')?.textContent, author: issueHover?.querySelector('.sessions-issue-hover-author')?.textContent, unresolvedLabel: unresolvedIssueEntry?.label, + unresolvedBadge: unresolvedIssueEntry?.badge, + unresolvedRowClassName: unresolvedIssueEntry?.className, unresolvedAriaLabel: unresolvedIssueEntry?.ariaLabel, unresolvedTooltip: unresolvedIssueEntry?.tooltip, unresolvedHover: unresolvedIssueEntry?.pillHover, @@ -364,7 +374,9 @@ suite('SessionChatInputToolbar', () => { }, }, { pullRequest: { - label: 'Pull Request #332982: Restore rich pill hovers', + label: 'Restore rich pill hovers', + badge: '#332982', + rowClassName: 'chat-pill-github-reference', pillHoverContentOwnsPadding: true, className: 'sessions-pr-hover', contentOrder: [ @@ -390,6 +402,7 @@ suite('SessionChatInputToolbar', () => { referenceAriaLabel: 'Pull Request #332982', status: 'Merged', statusKind: 'merged', + statusIconAriaHidden: 'true', date: 'on Sep 3', title: 'Restore rich pill hovers', titleTooltip: 'Restore rich pill hovers', @@ -400,14 +413,18 @@ suite('SessionChatInputToolbar', () => { { tagName: 'BUTTON', ariaLabel: 'Copy base branch main' }, { tagName: 'BUTTON', ariaLabel: 'Copy head branch feature/rich-hover' }, ], - unresolvedLabel: 'Pull Request #332982: Recorded pull request title', + unresolvedLabel: 'Recorded pull request title', + unresolvedBadge: '#332982', + unresolvedRowClassName: 'chat-pill-github-reference', unresolvedAriaLabel: 'Open Pull Request #332982: Recorded pull request title', unresolvedTooltip: 'Pull Request #332982: Recorded pull request title\nhttps://github.com/microsoft/vscode/pull/332982', unresolvedHover: undefined, clipboardWrites: ['main', 'feature/rich-hover'], }, issue: { - label: 'Issue #42: Rich issue hover', + label: 'Rich issue hover', + badge: '#42', + rowClassName: 'chat-pill-github-reference', pillHoverContentOwnsPadding: true, className: 'sessions-issue-hover', contentOrder: [ @@ -432,12 +449,15 @@ suite('SessionChatInputToolbar', () => { referenceAriaLabel: 'Issue #42', status: 'Closed', statusKind: 'closed', + statusIconAriaHidden: 'true', date: 'on Sep 3', title: 'Rich issue hover', titleTooltip: 'Rich issue hover', description: 'Provides detailed issue context.', author: '@octocat opened this issue', - unresolvedLabel: 'Issue #42: Recorded issue title', + unresolvedLabel: 'Recorded issue title', + unresolvedBadge: '#42', + unresolvedRowClassName: 'chat-pill-github-reference', unresolvedAriaLabel: 'Open Issue #42: Recorded issue title', unresolvedTooltip: 'Issue #42: Recorded issue title\nhttps://github.com/microsoft/vscode/issues/42', unresolvedHover: undefined, diff --git a/src/vs/sessions/contrib/github/browser/issueHover.ts b/src/vs/sessions/contrib/github/browser/issueHover.ts index 64d69e5fa25766..78277ddd72b2e2 100644 --- a/src/vs/sessions/contrib/github/browser/issueHover.ts +++ b/src/vs/sessions/contrib/github/browser/issueHover.ts @@ -6,8 +6,10 @@ import './media/issueHover.css'; import { $, append } from '../../../../base/browser/dom.js'; +import { renderIcon } from '../../../../base/browser/ui/iconLabel/iconLabels.js'; import { localize } from '../../../../nls.js'; -import { GitHubIssueState, GitHubIssueStateReason, IGitHubIssue } from '../common/types.js'; +import { asCssVariable } from '../../../../platform/theme/common/colorUtils.js'; +import { computeIssueIcon, GitHubIssueState, GitHubIssueStateReason, IGitHubIssue } from '../common/types.js'; import { appendGitHubHoverTitle, getGitHubHoverDate, getGitHubHoverDescription } from './githubHover.js'; export interface IIssueHoverData { @@ -49,8 +51,15 @@ export function createIssueHover(data: IIssueHoverData): IIssueHover { const statusRow = append(hoverElement, $('.sessions-issue-hover-status-row')); const status = getIssueStatus(data.issue); - const statusElement = append(statusRow, $('span.sessions-issue-hover-status', undefined, status.label)); + const statusElement = append(statusRow, $('span.sessions-issue-hover-status')); statusElement.dataset.state = status.kind; + const statusIcon = computeIssueIcon(data.issue.state, data.issue.stateReason); + const statusIconElement = append(statusElement, renderIcon(statusIcon)); + statusIconElement.setAttribute('aria-hidden', 'true'); + if (statusIcon.color) { + statusIconElement.style.color = asCssVariable(statusIcon.color.id); + } + append(statusElement, $('span.sessions-issue-hover-status-label', undefined, status.label)); const body = getGitHubHoverDescription(data.issue.body, localize('agentSessions.issueHover.bodyFallback', "No description provided.")); const description = append(hoverElement, $('.sessions-issue-hover-description')); diff --git a/src/vs/sessions/contrib/github/browser/media/issueHover.css b/src/vs/sessions/contrib/github/browser/media/issueHover.css index 098142006a2328..cad0c3c809f374 100644 --- a/src/vs/sessions/contrib/github/browser/media/issueHover.css +++ b/src/vs/sessions/contrib/github/browser/media/issueHover.css @@ -44,11 +44,27 @@ color: var(--vscode-descriptionForeground); } +.sessions-issue-hover-status-row { + display: flex; +} + .sessions-issue-hover-status { + display: inline-flex; + align-items: center; + gap: var(--vscode-spacing-size40); flex-shrink: 0; + width: max-content; + padding: var(--vscode-spacing-size20) var(--vscode-spacing-size80); + border: var(--vscode-strokeThickness) solid var(--vscode-editorHoverWidget-border); + border-radius: var(--vscode-cornerRadius-circle); + background: var(--vscode-textCodeBlock-background); color: var(--vscode-descriptionForeground); } +.sessions-issue-hover-status .codicon { + font-size: var(--vscode-codiconFontSize-compact); +} + .sessions-issue-hover-title { min-width: 0; font-size: var(--vscode-fontSize-body1); diff --git a/src/vs/sessions/contrib/github/browser/media/pullRequestHover.css b/src/vs/sessions/contrib/github/browser/media/pullRequestHover.css index 02205162786a09..42fa67f3f89e57 100644 --- a/src/vs/sessions/contrib/github/browser/media/pullRequestHover.css +++ b/src/vs/sessions/contrib/github/browser/media/pullRequestHover.css @@ -44,11 +44,27 @@ color: var(--vscode-descriptionForeground); } +.sessions-pr-hover-status-row { + display: flex; +} + .sessions-pr-hover-status { + display: inline-flex; + align-items: center; + gap: var(--vscode-spacing-size40); flex-shrink: 0; + width: max-content; + padding: var(--vscode-spacing-size20) var(--vscode-spacing-size80); + border: var(--vscode-strokeThickness) solid var(--vscode-editorHoverWidget-border); + border-radius: var(--vscode-cornerRadius-circle); + background: var(--vscode-textCodeBlock-background); color: var(--vscode-descriptionForeground); } +.sessions-pr-hover-status .codicon { + font-size: var(--vscode-codiconFontSize-compact); +} + .sessions-pr-hover-title { min-width: 0; font-size: var(--vscode-fontSize-body1); diff --git a/src/vs/sessions/contrib/github/browser/pullRequestHover.ts b/src/vs/sessions/contrib/github/browser/pullRequestHover.ts index 4dd338d6d7b721..708708284bbb9d 100644 --- a/src/vs/sessions/contrib/github/browser/pullRequestHover.ts +++ b/src/vs/sessions/contrib/github/browser/pullRequestHover.ts @@ -6,8 +6,10 @@ import './media/pullRequestHover.css'; import { $, append } from '../../../../base/browser/dom.js'; +import { renderIcon } from '../../../../base/browser/ui/iconLabel/iconLabels.js'; import { localize } from '../../../../nls.js'; -import { GitHubPullRequestState, IGitHubPullRequest } from '../common/types.js'; +import { asCssVariable } from '../../../../platform/theme/common/colorUtils.js'; +import { computePullRequestIcon, GitHubPullRequestState, IGitHubPullRequest } from '../common/types.js'; import { appendGitHubHoverTitle, getGitHubHoverDate, getGitHubHoverDescription } from './githubHover.js'; export interface IPullRequestHoverData { @@ -51,8 +53,15 @@ export function createPullRequestHover(data: IPullRequestHoverData): IPullReques const statusRow = append(hoverElement, $('.sessions-pr-hover-status-row')); const status = getPullRequestStatus(data.pullRequest); - const statusElement = append(statusRow, $('span.sessions-pr-hover-status', undefined, status.label)); + const statusElement = append(statusRow, $('span.sessions-pr-hover-status')); statusElement.dataset.state = status.kind; + const statusIcon = computePullRequestIcon(status.kind); + const statusIconElement = append(statusElement, renderIcon(statusIcon)); + statusIconElement.setAttribute('aria-hidden', 'true'); + if (statusIcon.color) { + statusIconElement.style.color = asCssVariable(statusIcon.color.id); + } + append(statusElement, $('span.sessions-pr-hover-status-label', undefined, status.label)); const body = getGitHubHoverDescription(data.pullRequest.body, localize('agentSessions.pullRequestHover.bodyFallback', "No description provided.")); const description = append(hoverElement, $('.sessions-pr-hover-description')); diff --git a/src/vs/workbench/browser/chatDropdownPill.ts b/src/vs/workbench/browser/chatDropdownPill.ts index 5ef03a663ec55a..0325eac9f00153 100644 --- a/src/vs/workbench/browser/chatDropdownPill.ts +++ b/src/vs/workbench/browser/chatDropdownPill.ts @@ -295,7 +295,7 @@ export class ChatDropdownPillActionViewItem extends ChatPillActionViewItem { undefined, [], { - getAriaLabel: item => item.label ?? '', + getAriaLabel: item => item.item?.ariaLabel ?? item.label ?? '', getWidgetAriaLabel: () => this._pillOptions.title, }, { minWidth: 240, maxWidth: 460, widgetClassName: 'show-file-icons chat-pill-dropdown' }, @@ -313,6 +313,8 @@ export class ChatDropdownPillActionViewItem extends ChatPillActionViewItem { items.push({ kind: ActionListItemKind.Action, label: entry.label, + ...(entry.badge ? { badge: entry.badge } : {}), + ...(entry.className ? { className: entry.className } : {}), group: { title: '', ...(entry.icon ? { icon: entry.icon } : {}) }, ...(entry.resource ? { iconClasses: getIconClasses(this._modelService, this._languageService, entry.resource, FileKind.FILE) } : {}), ...(entry.toolbarActions?.length ? { toolbarActions: [...entry.toolbarActions] } : {}), diff --git a/src/vs/workbench/browser/chatPills.ts b/src/vs/workbench/browser/chatPills.ts index 84200438fc10e3..9bcbe4cdf53444 100644 --- a/src/vs/workbench/browser/chatPills.ts +++ b/src/vs/workbench/browser/chatPills.ts @@ -49,6 +49,10 @@ export interface IChatPillsModel { export interface IChatPillEntry { readonly id: string; readonly label: string; + /** Optional trailing metadata rendered after the dropdown row label. */ + readonly badge?: string; + /** Optional CSS class added to the dropdown row. */ + readonly className?: string; /** Short label used when this entry renders as the pill itself. */ readonly pillLabel?: string; readonly icon?: ThemeIcon; diff --git a/src/vs/workbench/browser/media/chatPills.css b/src/vs/workbench/browser/media/chatPills.css index a2a08e3544ff81..5a81917e699a9a 100644 --- a/src/vs/workbench/browser/media/chatPills.css +++ b/src/vs/workbench/browser/media/chatPills.css @@ -177,6 +177,16 @@ margin-right: 0; } +.action-widget.chat-pill-dropdown .monaco-list-row.chat-pill-github-reference .action-item-badge { + margin-left: var(--vscode-spacing-size40); + padding: 0; + border-radius: 0; + background: transparent; + color: var(--vscode-descriptionForeground); + font-size: inherit; + line-height: inherit; +} + /* Horizontally scrollable status pills above a chat input. */ .chat-pills-row { width: 100%; diff --git a/src/vs/workbench/test/browser/chatPills.test.ts b/src/vs/workbench/test/browser/chatPills.test.ts index fbead73fce865c..a8902b24e52437 100644 --- a/src/vs/workbench/test/browser/chatPills.test.ts +++ b/src/vs/workbench/test/browser/chatPills.test.ts @@ -5,10 +5,13 @@ import assert from 'assert'; import { getWindow } from '../../../base/browser/dom.js'; +import { StandardMouseEvent } from '../../../base/browser/mouseEvent.js'; +import { IAnchor } from '../../../base/browser/ui/contextview/contextview.js'; import { ensureCodeWindow, mainWindow } from '../../../base/browser/window.js'; import type { IManagedHoverContent } from '../../../base/browser/ui/hover/hover.js'; +import { IListAccessibilityProvider } from '../../../base/browser/ui/list/listWidget.js'; import { timeout } from '../../../base/common/async.js'; -import { Action } from '../../../base/common/actions.js'; +import { Action, IAction } from '../../../base/common/actions.js'; import { Codicon } from '../../../base/common/codicons.js'; import { DisposableStore, toDisposable } from '../../../base/common/lifecycle.js'; import { constObservable, derived, observableValue } from '../../../base/common/observable.js'; @@ -23,6 +26,7 @@ import { DEFAULT_LABELS_CONTAINER, ResourceLabels } from '../../browser/labels.j import { workbenchInstantiationService } from './workbenchTestServices.js'; const getDropdownPillHoverContents = Reflect.get(ChatDropdownPillActionViewItem.prototype, 'getHoverContents') as (this: ChatDropdownPillActionViewItem) => IManagedHoverContent; +const getDropdownPillItems = Reflect.get(ChatDropdownPillActionViewItem.prototype, '_getDropdownItems') as (this: ChatDropdownPillActionViewItem) => IActionListItem[]; suite('ChatPills', () => { const store = ensureNoDisposablesAreLeakedInTestSuite(); @@ -339,18 +343,21 @@ suite('ChatPills', () => { viewItem.render(container); const fallbackHover = getDropdownPillHoverContents.call(viewItem); - sections.set([{ title: 'Pull Requests', entries: [entry('1', richHover)] }], undefined); + sections.set([{ title: 'Pull Requests', entries: [{ ...entry('1', richHover), badge: '#1', className: 'chat-pill-github-reference' }] }], undefined); const enrichedHover = getDropdownPillHoverContents.call(viewItem); + const mappedEntry = getDropdownPillItems.call(viewItem)[1]; sections.set([{ title: 'Pull Requests', entries: [entry('1', richHover), entry('2')] }], undefined); const summaryHover = getDropdownPillHoverContents.call(viewItem); assert.deepStrictEqual({ fallbackHover, usesRichHover: enrichedHover === richHover, + mappedEntry: { label: mappedEntry.label, badge: mappedEntry.badge, className: mappedEntry.className }, summaryHover, }, { fallbackHover: 'https://github.com/microsoft/vscode/pull/1', usesRichHover: true, + mappedEntry: { label: 'Pull Request #1', badge: '#1', className: 'chat-pill-github-reference' }, summaryHover: 'Show 2 pull requests', }); @@ -363,6 +370,7 @@ suite('ChatPills', () => { let visible = false; let onHide: ((didCancel?: boolean) => void) | undefined; let shownLabels: readonly (string | undefined)[] = []; + let shownAriaLabels: readonly (string | null)[] = []; let updatedLabels: readonly (string | undefined)[] = []; let hideCount = 0; const dropdownFocus = mainWindow.document.createElement('button'); @@ -370,9 +378,13 @@ suite('ChatPills', () => { disposables.add(toDisposable(() => dropdownFocus.remove())); const actionWidgetService = new class extends mock() { override get isVisible(): boolean { return visible; } - override show(_user: string, _supportsPreview: boolean, items: readonly IActionListItem[], delegate: IActionListDelegate): void { + override show(_user: string, _supportsPreview: boolean, items: readonly IActionListItem[], delegate: IActionListDelegate, _anchor: HTMLElement | StandardMouseEvent | IAnchor, _container: HTMLElement | undefined, _actionBarActions?: readonly IAction[], accessibilityProvider?: Partial>>): void { visible = true; shownLabels = items.map(item => item.label); + shownAriaLabels = items.map(item => { + const ariaLabel = accessibilityProvider?.getAriaLabel?.(item); + return typeof ariaLabel === 'string' ? ariaLabel : null; + }); onHide = delegate.onHide; dropdownFocus.focus(); } @@ -391,6 +403,7 @@ suite('ChatPills', () => { const entry = (id: string): IChatPillEntry => ({ id, label: `Pull Request #${id}`, + ariaLabel: `Open Pull Request #${id}`, open: () => { }, }); const sections = observableValue('chatPills.openSections', [{ @@ -431,6 +444,7 @@ suite('ChatPills', () => { assert.deepStrictEqual({ shownLabels, + shownAriaLabels, updatedLabels, expandedAfterUpdate, dropdownFocusPreserved, @@ -439,6 +453,7 @@ suite('ChatPills', () => { expandedAfterEmpty: button.getAttribute('aria-expanded'), }, { shownLabels: ['Pull Requests', 'Pull Request #1', 'Pull Request #2', 'Pull Request #3'], + shownAriaLabels: ['Pull Requests', 'Open Pull Request #1', 'Open Pull Request #2', 'Open Pull Request #3'], updatedLabels: ['Pull Requests', 'Pull Request #2', 'Pull Request #3'], expandedAfterUpdate: 'true', dropdownFocusPreserved: true, From 7c3c39cc60f44a82e5ee75eae1b02b34c8fe10c1 Mon Sep 17 00:00:00 2001 From: Cherry Wang Date: Fri, 11 Sep 2026 08:48:15 -0700 Subject: [PATCH 10/11] sessions: hide decorative PR branch arrow Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../contrib/chat/test/browser/sessionChatInputToolbar.test.ts | 2 ++ src/vs/sessions/contrib/github/browser/pullRequestHover.ts | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/vs/sessions/contrib/chat/test/browser/sessionChatInputToolbar.test.ts b/src/vs/sessions/contrib/chat/test/browser/sessionChatInputToolbar.test.ts index 9b8e28d3345529..f242dfc4aa65a2 100644 --- a/src/vs/sessions/contrib/chat/test/browser/sessionChatInputToolbar.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/sessionChatInputToolbar.test.ts @@ -320,6 +320,7 @@ suite('SessionChatInputToolbar', () => { description: pullRequestHover?.querySelector('.sessions-pr-hover-description-content')?.textContent, author: pullRequestHover?.querySelector('.sessions-pr-hover-author')?.textContent, branches: [...pullRequestHover?.querySelectorAll('.sessions-pr-hover-branch') ?? []].map(element => element.textContent), + branchArrowAriaHidden: pullRequestHover?.querySelector('.sessions-pr-hover-branch-arrow')?.getAttribute('aria-hidden'), branchControls: [...pullRequestHover?.querySelectorAll('.sessions-pr-hover-branch') ?? []].map(element => ({ tagName: element.tagName, ariaLabel: element.getAttribute('aria-label'), @@ -409,6 +410,7 @@ suite('SessionChatInputToolbar', () => { description: 'Provides detailed pull request context.', author: '@octocat opened this pull request', branches: ['main', 'feature/rich-hover'], + branchArrowAriaHidden: 'true', branchControls: [ { tagName: 'BUTTON', ariaLabel: 'Copy base branch main' }, { tagName: 'BUTTON', ariaLabel: 'Copy head branch feature/rich-hover' }, diff --git a/src/vs/sessions/contrib/github/browser/pullRequestHover.ts b/src/vs/sessions/contrib/github/browser/pullRequestHover.ts index 708708284bbb9d..38ca8542968349 100644 --- a/src/vs/sessions/contrib/github/browser/pullRequestHover.ts +++ b/src/vs/sessions/contrib/github/browser/pullRequestHover.ts @@ -69,7 +69,8 @@ export function createPullRequestHover(data: IPullRequestHoverData): IPullReques const branchRow = append(hoverElement, $('.sessions-pr-hover-branches')); const baseBranch = appendBranchPill(branchRow, data.pullRequest.baseRef || localize('agentSessions.pullRequestHover.baseFallback', "target"), 'base', data.onDidClickBaseBranch); - append(branchRow, $('span.sessions-pr-hover-branch-arrow', undefined, '\u2190')); + const branchArrow = append(branchRow, $('span.sessions-pr-hover-branch-arrow', undefined, '\u2190')); + branchArrow.setAttribute('aria-hidden', 'true'); const headBranch = appendBranchPill(branchRow, data.pullRequest.headRef || localize('agentSessions.pullRequestHover.headFallback', "source"), 'head', data.onDidClickHeadBranch); append(hoverElement, $('.sessions-pr-hover-author', undefined, localize('agentSessions.pullRequestHover.author', "@{0} opened this pull request", data.pullRequest.author.login))); From a4c1c0c7aad1fd0a91bdbe5366fed00ddcf32520 Mon Sep 17 00:00:00 2001 From: Cherry Wang Date: Fri, 11 Sep 2026 09:08:37 -0700 Subject: [PATCH 11/11] sessions: address remaining Copilot review feedback - actionList: Shift+Tab from a hover panel now returns to the last non-removal toolbar action instead of a trailing Remove control, covered by a new multi-action regression test. - pullRequestHover: stop branch-pill clicks from bubbling to the ActionList row so copying a branch no longer clears list focus. - chatView: extract the Shift+Tab-to-pills predicate into a testable helper and add regression tests for modifier filtering and preventDefault/stopPropagation cancellation; align the accessibility help text with the Shift+Tab shortcut. - githubPRFetcher: cover the closed_at -> closedAt mapping with a dedicated fixture and assertion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../actionWidget/browser/actionList.ts | 22 +++++- .../test/browser/actionList.test.ts | 44 +++++++++++ .../sessions/contrib/chat/browser/chatView.ts | 11 ++- .../browser/sessionsChatAccessibilityHelp.ts | 2 +- .../chat/test/browser/chatView.test.ts | 49 +++++++++++- .../github/browser/pullRequestHover.ts | 5 +- .../test/browser/githubFetchers.test.ts | 12 ++- .../test/browser/pullRequestHover.test.ts | 78 +++++++++++++++++++ 8 files changed, 216 insertions(+), 7 deletions(-) create mode 100644 src/vs/sessions/contrib/github/test/browser/pullRequestHover.test.ts diff --git a/src/vs/platform/actionWidget/browser/actionList.ts b/src/vs/platform/actionWidget/browser/actionList.ts index d843b57ff5baa5..8de955c40d2f93 100644 --- a/src/vs/platform/actionWidget/browser/actionList.ts +++ b/src/vs/platform/actionWidget/browser/actionList.ts @@ -40,6 +40,9 @@ import { IInstantiationService } from '../../instantiation/common/instantiation. export const acceptSelectedActionCommand = 'acceptSelectedCodeAction'; export const previewSelectedActionCommand = 'previewSelectedCodeAction'; +/** Action ID of the auto-appended toolbar action created from {@link IActionListItem.onRemove}. */ +const removeToolbarActionId = 'actionList.remove'; + export interface IActionListDelegate { onHide(didCancel?: boolean): void; onSelect(action: T, preview?: boolean): void; @@ -503,7 +506,7 @@ class ActionItemRenderer implements IListRenderer, IAction const toolbarActions = [...(element.toolbarActions ?? [])]; if (element.onRemove) { toolbarActions.push(toAction({ - id: 'actionList.remove', + id: removeToolbarActionId, label: localize('actionList.remove', "Remove"), class: ThemeIcon.asClassName(Codicon.close), run: async () => { @@ -2171,6 +2174,21 @@ export class ActionListWidget 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, row: HTMLElement): void { const controls = this._getTabThroughPanelControls(element, row); if (controls.toolbar?.length()) { @@ -2222,7 +2240,7 @@ export class ActionListWidget extends Disposable { if (inPanel) { if (controls.toolbar?.length()) { dom.EventHelper.stop(event, true); - controls.toolbar.focus(controls.toolbar.length() - 1); + controls.toolbar.focus(this._lastPrimaryToolbarActionIndex(controls.toolbar)); return; } target = this._list.getHTMLElement(); diff --git a/src/vs/platform/actionWidget/test/browser/actionList.test.ts b/src/vs/platform/actionWidget/test/browser/actionList.test.ts index 289e720f8659f5..68dd2b3c7691e6 100644 --- a/src/vs/platform/actionWidget/test/browser/actionList.test.ts +++ b/src/vs/platform/actionWidget/test/browser/actionList.test.ts @@ -1276,6 +1276,50 @@ suite('ActionListWidget', () => { }); }); + test('Shift+Tab from the panel returns to the last non-removal toolbar action', () => { + const createPanel = () => { + const panel = document.createElement('div'); + const control = document.createElement('a'); + control.href = 'https://example.com'; + control.textContent = 'link'; + panel.append(control); + return { panel, controls: [control] }; + }; + let panelControls: readonly HTMLElement[] = []; + const item: IActionListItem = { + ...action('one'), + toolbarActions: [toAction({ id: 'copy', label: 'Copy', run: () => { } })], + onRemove: () => { }, + hover: { + content: () => { + const result = createPanel(); + panelControls = result.controls; + return result.panel; + }, + expandable: true, + showIndicator: false, + tabThroughPanel: true, + getTabbableElements: () => panelControls, + contentOwnsPadding: true, + }, + }; + const widget = createActionListWidget(disposables, { + items: [item], + listOptions: { showFilter: false, reserveSubmenuSpace: false }, + }); + const press = (key: string, shiftKey = false) => + document.activeElement?.dispatchEvent(new KeyboardEvent('keydown', { key, shiftKey, bubbles: true, cancelable: true })); + const focusedToolbarLabel = () => document.activeElement?.closest('.action-list-item-toolbar') ? document.activeElement?.getAttribute('aria-label') ?? document.activeElement?.textContent : undefined; + + widget.focus(); + press('Tab'); // list -> Copy + press('Tab'); // Copy -> Remove + press('Tab'); // Remove -> panel link + press('Tab', true); // panel link -> Shift+Tab back into the toolbar + + assert.strictEqual(focusedToolbarLabel(), 'Copy'); + }); + test('rebuilding the items in place re-measures only when the row count changed', () => { const widget = createActionListWidget(disposables, { items: [action('one'), action('two')] }); const layouts: string[] = []; diff --git a/src/vs/sessions/contrib/chat/browser/chatView.ts b/src/vs/sessions/contrib/chat/browser/chatView.ts index 02062f092a74ca..53aec745040a76 100644 --- a/src/vs/sessions/contrib/chat/browser/chatView.ts +++ b/src/vs/sessions/contrib/chat/browser/chatView.ts @@ -11,6 +11,7 @@ import { renderAsPlaintext } from '../../../../base/browser/markdownRenderer.js' import { CancellationTokenSource } from '../../../../base/common/cancellation.js'; import { MutableDisposable, toDisposable } from '../../../../base/common/lifecycle.js'; import { KeyCode } from '../../../../base/common/keyCodes.js'; +import { IKeyboardEvent } from '../../../../base/browser/keyboardEvent.js'; import { autorun, derived, IObservable, observableFromEvent, observableValue } from '../../../../base/common/observable.js'; import { isEqual } from '../../../../base/common/resources.js'; import { URI } from '../../../../base/common/uri.js'; @@ -72,6 +73,14 @@ export function shouldShowSessionChatTip(sessionStatus: SessionStatus | undefine return sessionStatus === undefined || !isActiveSessionStatus(sessionStatus); } +/** + * Whether a chat input keydown should move focus to the status pills above it. Matches an + * unmodified Shift+Tab, mirroring the accessibility-help guidance for reaching those pills. + */ +export function isFocusChatPillsKeyDown(event: Pick): boolean { + return event.keyCode === KeyCode.Tab && event.shiftKey && !event.ctrlKey && !event.metaKey && !event.altKey; +} + /** * A session view that hosts a {@link NewChatWidget} — the "new session" UI * shown before a session has been created. This is the default view that @@ -348,7 +357,7 @@ export class ChatView extends AbstractChatView { // Floating status pills above the input. this._chatPills = this._register(instantiationService.createInstance(SessionChatInputToolbar, false, () => this._widget.focusInput())); this._register(this._widget.inputEditor.onKeyDown(event => { - if (event.keyCode === KeyCode.Tab && event.shiftKey && !event.ctrlKey && !event.metaKey && !event.altKey && this._chatPills.focusFirst()) { + if (isFocusChatPillsKeyDown(event) && this._chatPills.focusFirst()) { event.preventDefault(); event.stopPropagation(); } diff --git a/src/vs/sessions/contrib/chat/browser/sessionsChatAccessibilityHelp.ts b/src/vs/sessions/contrib/chat/browser/sessionsChatAccessibilityHelp.ts index 5ef12e0a1660ab..19ec453c2cd737 100644 --- a/src/vs/sessions/contrib/chat/browser/sessionsChatAccessibilityHelp.ts +++ b/src/vs/sessions/contrib/chat/browser/sessionsChatAccessibilityHelp.ts @@ -39,7 +39,7 @@ export class SessionsChatAccessibilityHelp implements IAccessibleViewImplementat content.push(localize('sessionsChat.overview', "You are in the Agents window. The Agents window is a dedicated workspace for working with AI agents. It provides a chat interface, a changes view for reviewing agent-generated changes, a file explorer, and customization options.")); content.push(localize('sessionsChat.input', "You are in the chat input. Type a message and press Enter to send it.")); content.push(getModePickerAccessibilityHelp()); - content.push(localize('sessionsChat.inputPills', "When session metadata or active-turn status pills appear above the input, press Tab to reach them, use the Left and Right arrow keys to move between them, and press Enter or Space to activate one. Open the context menu{0} to choose which pills are shown. Pull Requests Options lets you show all pull requests or only open and draft ones, remembered across sessions. If every pull request is filtered out, use the toolbar context menu to show all again.", '')); + content.push(localize('sessionsChat.inputPills', "When session metadata or active-turn status pills appear above the input, press Shift+Tab to reach them, use the Left and Right arrow keys to move between them, and press Enter or Space to activate one. Open the context menu{0} to choose which pills are shown. Pull Requests Options lets you show all pull requests or only open and draft ones, remembered across sessions. If every pull request is filtered out, use the toolbar context menu to show all again.", '')); content.push(localize('sessionsChat.removePullRequestArtifact', "For pull requests recorded as session artifacts, the pull request dropdown offers Remove Pull Request Artifact from Session on each row. Use Tab to reach its actions. When only one pull request is visible, use the pull request pill's context menu instead. Removal is immediate and only deletes the artifact record; it does not close the pull request or remove independent session associations.")); content.push(localize('sessionsChat.externalSessionFilter', "The Sessions list Filter menu includes an External submenu. Use it to choose whether external sessions from another application are shown for the last 24 hours, the last 7 days, always, or not at all.")); content.push(localize('sessionsChat.externalSessionBanner', "When you first open a session created in another application, a banner appears at the top of the chat. Use Tab to reach its external-session picker, choose an option, and activate Save. The Close action dismisses the banner without changing the setting. Saving or closing permanently dismisses the banner.")); diff --git a/src/vs/sessions/contrib/chat/test/browser/chatView.test.ts b/src/vs/sessions/contrib/chat/test/browser/chatView.test.ts index 8a904a964192af..eba23f6df6ff85 100644 --- a/src/vs/sessions/contrib/chat/test/browser/chatView.test.ts +++ b/src/vs/sessions/contrib/chat/test/browser/chatView.test.ts @@ -6,6 +6,7 @@ import assert from 'assert'; import * as dom from '../../../../../base/browser/dom.js'; import { DisposableStore, MutableDisposable, toDisposable } from '../../../../../base/common/lifecycle.js'; +import { KeyCode } from '../../../../../base/common/keyCodes.js'; import { constObservable, observableValue } from '../../../../../base/common/observable.js'; import { URI } from '../../../../../base/common/uri.js'; import { mock } from '../../../../../base/test/common/mock.js'; @@ -23,7 +24,7 @@ import { ISession, SessionStatus } from '../../../../services/sessions/common/se import { ISessionsService } from '../../../../services/sessions/browser/sessionsService.js'; import { SessionsChatBackgroundRenderer, SessionsChatBackgroundReplica } from '../../../../services/chatBackground/browser/chatBackgroundRenderer.js'; import { ISessionsChatBackground } from '../../../../services/chatBackground/browser/chatBackgroundService.js'; -import { ChatView, findInitialTranscriptContextEntry, findTranscriptContextEntry, getSessionChatItemHorizontalPadding, getTranscriptProgress, NewChatView, shouldShowSessionChatTip, shouldShowTranscriptPreparationCompletion, shouldShowTranscriptPreparationProgress } from '../../browser/chatView.js'; +import { ChatView, findInitialTranscriptContextEntry, findTranscriptContextEntry, getSessionChatItemHorizontalPadding, getTranscriptProgress, isFocusChatPillsKeyDown, NewChatView, shouldShowSessionChatTip, shouldShowTranscriptPreparationCompletion, shouldShowTranscriptPreparationProgress } from '../../browser/chatView.js'; import { SessionsChatViewStateService } from '../../browser/chatViewStateService.js'; import { NewChatInSessionWidget } from '../../browser/newChatInSessionWidget.js'; import { NewChatInputWidget } from '../../browser/newChatInput.js'; @@ -1660,6 +1661,52 @@ suite('Sessions - Chat View', () => { }); }); + test('recognizes an unmodified Shift+Tab as the chat-pills focus shortcut', () => { + const base = { keyCode: KeyCode.Tab, shiftKey: true, ctrlKey: false, metaKey: false, altKey: false }; + assert.deepStrictEqual({ + shiftTab: isFocusChatPillsKeyDown(base), + plainTab: isFocusChatPillsKeyDown({ ...base, shiftKey: false }), + ctrlShiftTab: isFocusChatPillsKeyDown({ ...base, ctrlKey: true }), + metaShiftTab: isFocusChatPillsKeyDown({ ...base, metaKey: true }), + altShiftTab: isFocusChatPillsKeyDown({ ...base, altKey: true }), + otherKey: isFocusChatPillsKeyDown({ ...base, keyCode: KeyCode.Escape }), + }, { + shiftTab: true, + plainTab: false, + ctrlShiftTab: false, + metaShiftTab: false, + altShiftTab: false, + otherKey: false, + }); + }); + + test('only cancels the chat input keydown when the pills accept focus', () => { + const handleKeyDown = (event: { keyCode: KeyCode; shiftKey: boolean; ctrlKey: boolean; metaKey: boolean; altKey: boolean; preventDefault(): void; stopPropagation(): void }, focusFirst: () => boolean) => { + if (isFocusChatPillsKeyDown(event) && focusFirst()) { + event.preventDefault(); + event.stopPropagation(); + } + }; + const fire = (shiftKey: boolean, focusFirstResult: boolean) => { + const calls: string[] = []; + handleKeyDown( + { keyCode: KeyCode.Tab, shiftKey, ctrlKey: false, metaKey: false, altKey: false, preventDefault: () => calls.push('preventDefault'), stopPropagation: () => calls.push('stopPropagation') }, + () => focusFirstResult, + ); + return calls; + }; + + assert.deepStrictEqual({ + matchingAndFocused: fire(true, true), + matchingButNoPills: fire(true, false), + nonMatching: fire(false, true), + }, { + matchingAndFocused: ['preventDefault', 'stopPropagation'], + matchingButNoPills: [], + nonMatching: [], + }); + }); + test('finds transcript context in hidden request attachments', () => { const attachment: IChatRequestTranscriptContextVariableEntry = { kind: 'transcriptContext', diff --git a/src/vs/sessions/contrib/github/browser/pullRequestHover.ts b/src/vs/sessions/contrib/github/browser/pullRequestHover.ts index 38ca8542968349..054d7b40555700 100644 --- a/src/vs/sessions/contrib/github/browser/pullRequestHover.ts +++ b/src/vs/sessions/contrib/github/browser/pullRequestHover.ts @@ -134,7 +134,10 @@ function appendBranchPill(container: HTMLElement, label: string, kind: 'base' | : localize('agentSessions.pullRequestHover.copyHeadBranch', "Copy head branch {0}", label); branch.title = actionLabel; branch.setAttribute('aria-label', actionLabel); - branch.onclick = onDidClick; + branch.onclick = event => { + event.stopPropagation(); + onDidClick(); + }; append(container, branch); return branch; } diff --git a/src/vs/sessions/contrib/github/test/browser/githubFetchers.test.ts b/src/vs/sessions/contrib/github/test/browser/githubFetchers.test.ts index 737d6de1d2a53e..3fc1d608d10dd4 100644 --- a/src/vs/sessions/contrib/github/test/browser/githubFetchers.test.ts +++ b/src/vs/sessions/contrib/github/test/browser/githubFetchers.test.ts @@ -331,10 +331,18 @@ suite('GitHubPRFetcher', () => { }); test('getPullRequest maps closed PR', async () => { - mockApi.setNextResponse(makePRResponse({ state: 'closed', merged: false, draft: false })); + mockApi.setNextResponse(makePRResponse({ state: 'closed', merged: false, draft: false, closed_at: '2024-03-04T00:00:00Z' })); const pr = await fetcher.getPullRequest('owner', 'repo', 1); assert.strictEqual(pr.data?.state, GitHubPullRequestState.Closed); + assert.strictEqual(pr.data?.closedAt, '2024-03-04T00:00:00Z'); + }); + + test('getPullRequest omits closedAt for an open PR', async () => { + mockApi.setNextResponse(makePRResponse({ state: 'open', merged: false, draft: false })); + + const pr = await fetcher.getPullRequest('owner', 'repo', 1); + assert.strictEqual(pr.data?.closedAt, undefined); }); test('getReviewThreads returns GraphQL thread metadata', async () => { @@ -811,6 +819,7 @@ function makePRResponse(overrides: { draft: boolean; mergeable?: boolean | null; mergeable_state?: string; + closed_at?: string | null; }): unknown { return { number: 1, @@ -824,6 +833,7 @@ function makePRResponse(overrides: { created_at: '2024-01-01T00:00:00Z', updated_at: '2024-01-02T00:00:00Z', merged_at: overrides.merged ? '2024-01-02T00:00:00Z' : null, + closed_at: overrides.closed_at ?? (overrides.state === 'closed' ? '2024-01-03T00:00:00Z' : null), mergeable: overrides.mergeable ?? true, mergeable_state: overrides.mergeable_state ?? 'clean', merged: overrides.merged, diff --git a/src/vs/sessions/contrib/github/test/browser/pullRequestHover.test.ts b/src/vs/sessions/contrib/github/test/browser/pullRequestHover.test.ts new file mode 100644 index 00000000000000..0d815b1e493add --- /dev/null +++ b/src/vs/sessions/contrib/github/test/browser/pullRequestHover.test.ts @@ -0,0 +1,78 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; +import { createPullRequestHover } from '../../browser/pullRequestHover.js'; +import { GitHubPullRequestState, IGitHubPullRequest } from '../../common/types.js'; + +function makePullRequest(overrides: Partial = {}): IGitHubPullRequest { + return { + number: 1, + title: 'Test PR', + body: 'Test body', + state: GitHubPullRequestState.Open, + author: { login: 'author', avatarUrl: '' }, + headRef: 'feature', + headSha: 'abc123', + baseRef: 'main', + isDraft: false, + createdAt: '2024-01-01T00:00:00Z', + updatedAt: '2024-01-02T00:00:00Z', + mergedAt: undefined, + mergeable: true, + mergeableState: 'clean', + ...overrides, + }; +} + +suite('createPullRequestHover', () => { + + ensureNoDisposablesAreLeakedInTestSuite(); + + test('hides the decorative branch-direction arrow from the accessibility tree', () => { + const { element } = createPullRequestHover({ + owner: 'owner', + repo: 'repo', + number: 1, + repositoryHref: 'https://example.com', + referenceHref: 'https://example.com/1', + pullRequest: makePullRequest(), + density: 'default', + }); + + const arrow = element.querySelector('.sessions-pr-hover-branch-arrow'); + assert.strictEqual(arrow?.getAttribute('aria-hidden'), 'true'); + }); + + test('activating a branch pill stops the click from bubbling to an ancestor list row', () => { + const clicks: string[] = []; + const { element } = createPullRequestHover({ + owner: 'owner', + repo: 'repo', + number: 1, + repositoryHref: 'https://example.com', + referenceHref: 'https://example.com/1', + pullRequest: makePullRequest(), + density: 'default', + onDidClickBaseBranch: () => clicks.push('base'), + onDidClickHeadBranch: () => clicks.push('head'), + }); + + // Simulate the ActionList row that owns the hover panel; a click reaching this + // ancestor is what the real list interprets as "clicked outside a row". + const row = document.createElement('div'); + row.className = 'action-list-item'; + row.append(element); + let bubbledToRow = false; + row.addEventListener('click', () => { bubbledToRow = true; }); + + const [baseBranch, headBranch] = Array.from(element.querySelectorAll('.sessions-pr-hover-branch')); + baseBranch.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })); + headBranch.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })); + + assert.deepStrictEqual({ clicks, bubbledToRow }, { clicks: ['base', 'head'], bubbledToRow: false }); + }); +});