Conversation
Screenshot ChangesBase: Changed (1)
|
| Before | After |
|---|---|
 |
 |
There was a problem hiding this comment.
Pull request overview
Adjusts the Sessions “session type” (CLI/Cloud) picker UI in the agents-first Sessions window (vs/sessions) to address issue #306455 around a disabled-looking picker.
Changes:
- Removes the slot element reference and the
.disabledCSS class toggling fromSessionTypePicker. - Leaves picker rendering intact but no longer visually disables it when there’s only one session type.
Show a summary per file
| File | Description |
|---|---|
src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts |
Removes disabled-state styling logic for the session type picker. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 1
| dom.clearNode(this._triggerElement); | ||
|
|
||
| const currentType = this._sessionTypes.find(t => t.id === this._sessionType); | ||
| const modeIcon = currentType?.icon ?? Codicon.terminal; | ||
| const modeLabel = currentType?.label ?? this._sessionType ?? ''; | ||
|
|
||
| dom.append(this._triggerElement, renderIcon(modeIcon)); | ||
| const labelSpan = dom.append(this._triggerElement, dom.$('span.sessions-chat-dropdown-label')); | ||
| labelSpan.textContent = modeLabel; | ||
|
|
||
| const hasMultipleTypes = this._sessionTypes.length > 1; | ||
| this._slotElement?.classList.toggle('disabled', !hasMultipleTypes); | ||
| dom.append(this._triggerElement, renderIcon(Codicon.chevronDown)); | ||
| } |
There was a problem hiding this comment.
With the removal of the .disabled styling, the session type control now appears interactive even when there is only one (or zero) available session type, but _showPicker() immediately returns in that case. This results in a focusable “button” that does nothing on click/Enter/Space (misleading UX and confusing for screen readers). To match the linked issue, hide the picker entirely when it isn’t actionable (e.g., toggle hidden on the slot/parent element when this._sessionTypes.length <= 1 and consider also hiding the chevron in that case).
fix #306455