Sessions: Refactor and enhance session status widget in titlebar#304424
Sessions: Refactor and enhance session status widget in titlebar#304424mrleemurray merged 5 commits intomainfrom
Conversation
…date its functionality in the titlebar Co-authored-by: Copilot <copilot@github.com>
…tracking Co-authored-by: Copilot <copilot@github.com>
…improved accessibility and styling Co-authored-by: Copilot <copilot@github.com>
…ounting in sidebar and title bar Co-authored-by: Copilot <copilot@github.com>
…styles Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Pull request overview
This PR refactors the Agent Sessions window titlebar/session status UI by replacing the old collapsed sidebar widget approach with new inline sidebar toggle/count widgets and a shared unread-session counting helper.
Changes:
- Add a shared
countUnreadSessionshelper and use it to drive unread badges in the titlebar and sidebar. - Introduce new titlebar and sidebar toggle/count buttons that show/hide the sidebar and display unread counts.
- Remove the old
CollapsedSidebarWidgetimplementation and related wiring, plus adjust styling/layout CSS accordingly.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/browser/agentSessions/agentSessionsModel.ts | Adds a reusable helper to count unread sessions (used by multiple UI surfaces). |
| src/vs/sessions/contrib/sessions/browser/sessionsTitleBarWidget.ts | Adds a titlebar count button that toggles the sidebar and shows unread count. |
| src/vs/sessions/contrib/sessions/browser/media/sessionsTitleBarWidget.css | Styles the new titlebar count button and unread label. |
| src/vs/sessions/browser/workbench.ts | Removes the old collapsed-sidebar widget creation/toggling from the sessions workbench. |
| src/vs/sessions/browser/parts/sidebarPart.ts | Adds a sidebar title-area button to hide the sidebar and show an unread badge. |
| src/vs/sessions/browser/parts/media/sidebarPart.css | Styles the new sidebar title-area toggle/badge and updates drag-region exclusions. |
| src/vs/sessions/browser/media/style.css | Adjusts overall sessions layout padding. |
| src/vs/sessions/browser/media/collapsedPanelWidget.css | Updates collapsed-widget styling; still contains sidebar-widget selectors. |
| src/vs/sessions/browser/layoutActions.ts | Removes the sidebar-title menu entry for the sidebar toggle action. |
| src/vs/sessions/browser/collapsedPartWidgets.ts | Deletes CollapsedSidebarWidget, leaving only the auxiliary bar widget implementation. |
| export function countUnreadSessions(sessions: IAgentSession[]): number { | ||
| let unread = 0; | ||
| for (const session of sessions) { | ||
| if (!session.isArchived() && session.status === AgentSessionStatus.Completed && !session.isRead()) { | ||
| unread++; | ||
| } | ||
| } | ||
| return unread; |
There was a problem hiding this comment.
countUnreadSessions introduces new shared logic that drives UI badges/indicators, but it isn’t covered by unit tests. Adding a small test for the expected counting rules (ignores archived sessions, counts only Completed + unread, etc.) would help prevent regressions since multiple UI surfaces now depend on this helper.
| // Session count widget (to the left of the pill) — toggles sidebar | ||
| const countWidget = $('button.agent-sessions-titlebar-count') as HTMLButtonElement; | ||
| countWidget.type = 'button'; | ||
| countWidget.tabIndex = 0; | ||
| const countIcon = $(ThemeIcon.asCSSSelector(Codicon.tasklist)); |
There was a problem hiding this comment.
Adding a nested inside the widget container introduces an interaction/accessibility issue: the existing keydown handler on the container (Enter/Space opens the sessions picker) will also receive bubbled key events from this button, which can prevent the count button from being keyboard-activated. Consider stopping propagation for key events on the count button, or updating the container keydown handler to ignore events that originate from nested interactive elements (and/or avoid role=button/tabIndex on a container that contains other buttons).
| font: inherit; | ||
| outline: none; | ||
| } | ||
|
|
There was a problem hiding this comment.
The new count button removes the focus outline (outline: none) but doesn’t provide an alternative :focus-visible style. This makes keyboard focus hard/impossible to see. Please add an explicit focus indicator consistent with the container’s focus styling (e.g., use outline on :focus-visible).
| .command-center .agent-sessions-titlebar-container .agent-sessions-titlebar-count:focus-visible { | |
| outline: 1px solid var(--vscode-focusBorder); | |
| outline-offset: -1px; | |
| } |
| // Update badge on session changes (deferred to avoid service unavailability) | ||
| const updateBadge = (svc: IAgentSessionsService) => { | ||
| const unread = countUnreadSessions(svc.model.sessions); | ||
| badge.textContent = unread > 0 ? `${unread}` : ''; | ||
| badge.style.display = unread > 0 ? '' : 'none'; | ||
| widget.setAttribute('aria-label', unread > 0 | ||
| ? localize('hideSidebarUnread', "Hide Side Bar, {0} unread session(s)", unread) | ||
| : localize('hideSidebar', "Hide Side Bar")); | ||
| }; | ||
|
|
||
| setTimeout(() => { | ||
| try { | ||
| const svc = this.instantiationService.invokeFunction(accessor => accessor.get(IAgentSessionsService)); | ||
| updateBadge(svc); | ||
| widgetDisposables.add(svc.model.onDidChangeSessions(() => updateBadge(svc))); |
There was a problem hiding this comment.
The unread badge wiring relies on setTimeout(..., 0) + try/catch around accessor.get(IAgentSessionsService). If that lookup ever fails at that moment, the badge will stay hidden permanently (no retry), and the async callback can also run after the part is disposed. Prefer injecting IAgentSessionsService into the SidebarPart constructor (or otherwise obtaining it synchronously) and registering onDidChangeSessions directly, avoiding setTimeout/exception-driven control flow here.
| border: none; | ||
| border-radius: 4px; | ||
| cursor: pointer; | ||
| color: inherit; | ||
| font: inherit; | ||
| background: var(--vscode-toolbar-activeBackground); | ||
| outline: none; | ||
| position: relative; |
There was a problem hiding this comment.
.session-status-toggle sets outline: none but doesn’t add any :focus-visible styling, so keyboard users won’t get a visible focus indicator for the new button. Please add an explicit focus style (e.g. outline using --vscode-focusBorder) consistent with the rest of the UI.
| /* ---- Sidebar widget (in titlebar-left, always visible, far left) ---- */ | ||
|
|
||
| .agent-sessions-workbench .collapsed-sidebar-widget { | ||
| order: 10; | ||
| order: 0; | ||
| padding-left: 8px; | ||
| } | ||
|
|
||
| /* When inside the sidebar title area (panel open), no left padding */ | ||
| .agent-sessions-workbench .part.sidebar .collapsed-sidebar-widget { | ||
| padding-left: 0; |
There was a problem hiding this comment.
CollapsedSidebarWidget was removed, and there are no remaining code references to the .collapsed-sidebar-widget markup, but this stylesheet still includes (and modifies) .collapsed-sidebar-widget rules. Consider removing the unused sidebar-widget selectors to avoid dead CSS and reduce future confusion.
Rename and update the functionality of the session status widget, implement a session count widget with unread session tracking, and improve accessibility and styling. Remove unused components to streamline the codebase.