Skip to content

Commit aa5dc47

Browse files
meganroggemeganrogge
authored
sessions: Disable unavailable session type picker (#333603)
* sessions: Disable unavailable session type picker Keep the sole session type visible for context while preventing interaction when no alternative can be selected. Re-enable the picker reactively when another type becomes available.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * sessions: Skip disabled harness in onboarding Gate the V2 harness spotlight on the picker interaction context so a visible but disabled sole session type is not presented as an actionable tour step.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 752a80f commit aa5dc47

4 files changed

Lines changed: 64 additions & 12 deletions

File tree

‎src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import { ISessionsProvidersService } from '../../../services/sessions/browser/se
1818
import { autorun, IObservable, observableValue } from '../../../../base/common/observable.js';
1919
import { ISession, SessionStatus } from '../../../services/sessions/common/session.js';
2020
import { Emitter } from '../../../../base/common/event.js';
21-
import { isWeb } from '../../../../base/common/platform.js';
2221
import { isEqual } from '../../../../base/common/resources.js';
2322
import { URI } from '../../../../base/common/uri.js';
2423
import { IStorageService, StorageScope, StorageTarget } from '../../../../platform/storage/common/storage.js';
@@ -150,8 +149,7 @@ export class SessionTypePicker extends Disposable {
150149
protected _triggerElement: HTMLElement | undefined;
151150

152151
/**
153-
* Tracks whether the harness picker trigger is currently visible. Mirrors
154-
* the `.hidden` state computed in {@link _updateTriggerLabel}, so the
152+
* Tracks whether the harness picker trigger is currently interactive, so the
155153
* new-session-view onboarding tour can skip the harness step when only a
156154
* single harness can serve the selected workspace.
157155
*/
@@ -637,18 +635,19 @@ export class SessionTypePicker extends Disposable {
637635

638636
dom.clearNode(this._triggerElement);
639637

640-
// In web (vscode.dev/agents) the host filter already scopes the
641-
// workbench to a single agent host, so when that host advertises only
642-
// one harness there is nothing to pick — hide the trigger entirely.
643-
const hideForSingleHarness = isWeb && this._folderSessionTypes.length <= 1 && this._pickServedByFolder(this._picked);
644-
if (this._folderSessionTypes.length === 0 || hideForSingleHarness) {
638+
if (this._folderSessionTypes.length === 0) {
645639
this._triggerElement.classList.add('hidden');
640+
this._triggerElement.parentElement?.classList.remove('disabled');
646641
this._visibleKey.set(false);
647642
return;
648643
}
649644

645+
const disabled = this._folderSessionTypes.length === 1 && this._pickServedByFolder(this._picked);
650646
this._triggerElement.classList.remove('hidden');
651-
this._visibleKey.set(true);
647+
this._triggerElement.parentElement?.classList.toggle('disabled', disabled);
648+
this._triggerElement.tabIndex = disabled ? -1 : 0;
649+
this._triggerElement.setAttribute('aria-disabled', String(disabled));
650+
this._visibleKey.set(!disabled);
652651
const currentType = this._folderSessionTypes.find(t =>
653652
t.providerId === this._picked?.providerId && t.sessionType.id === this._picked?.sessionTypeId)?.sessionType
654653
?? this._folderSessionTypes.find(t => t.sessionType.id === this._picked?.sessionTypeId)?.sessionType;
@@ -659,11 +658,13 @@ export class SessionTypePicker extends Disposable {
659658
const labelSpan = dom.append(this._triggerElement, dom.$('span.sessions-chat-dropdown-label'));
660659
labelSpan.textContent = modeLabel;
661660

662-
if (this._options?.showChevron !== false) {
661+
if (!disabled && this._options?.showChevron !== false) {
663662
const chevron = dom.append(this._triggerElement, renderIcon(Codicon.chevronDownCompact));
664663
chevron.classList.add('sessions-chat-dropdown-chevron');
665664
}
666665

667-
this._triggerElement.ariaLabel = localize('sessionTypePicker.triggerAriaLabel', "Pick Session Type, {0}", modeLabel);
666+
this._triggerElement.ariaLabel = disabled
667+
? localize('sessionTypePicker.disabledTriggerAriaLabel', "Session Type, {0}", modeLabel)
668+
: localize('sessionTypePicker.triggerAriaLabel', "Pick Session Type, {0}", modeLabel);
668669
}
669670
}

‎src/vs/sessions/contrib/chat/test/browser/sessionTypePicker.test.ts‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,51 @@ suite('SessionTypePicker', () => {
264264
});
265265
});
266266

267+
test('disables the trigger when the selected workspace has only one session type', () => {
268+
management.setSessionTypes([
269+
sessionType('copilot', 'cloud', 'Cloud'),
270+
]);
271+
const picker = createPicker(disposables, session, management, storage);
272+
session.set(createFakeSession('copilot', 'cloud', folder), undefined);
273+
const container = document.createElement('div');
274+
picker.render(container);
275+
const trigger = container.querySelector<HTMLElement>('.action-label');
276+
const singleType = {
277+
hidden: trigger?.classList.contains('hidden'),
278+
disabled: trigger?.getAttribute('aria-disabled'),
279+
tabIndex: trigger?.tabIndex,
280+
label: trigger?.getAttribute('aria-label'),
281+
};
282+
283+
management.setSessionTypes([
284+
sessionType('copilot', 'cloud', 'Cloud'),
285+
sessionType('local-agent-host', 'local', 'Local'),
286+
]);
287+
288+
assert.deepStrictEqual({
289+
singleType,
290+
multipleTypes: {
291+
hidden: trigger?.classList.contains('hidden'),
292+
disabled: trigger?.getAttribute('aria-disabled'),
293+
tabIndex: trigger?.tabIndex,
294+
label: trigger?.getAttribute('aria-label'),
295+
},
296+
}, {
297+
singleType: {
298+
hidden: false,
299+
disabled: 'true',
300+
tabIndex: -1,
301+
label: 'Session Type, Cloud',
302+
},
303+
multipleTypes: {
304+
hidden: false,
305+
disabled: 'false',
306+
tabIndex: 0,
307+
label: 'Pick Session Type, Cloud',
308+
},
309+
});
310+
});
311+
267312
test('re-selecting the default (first) session type clears the stored pick', () => {
268313
management.setSessionTypes([
269314
sessionType('local-1', 'local', 'Local'),

‎src/vs/sessions/contrib/onboardingTours/browser/tours/newSessionViewV2Tour.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { IObservable } from '../../../../../base/common/observable.js';
77
import { localize } from '../../../../../nls.js';
88
import { ISpotlightPayload, SPOTLIGHT_PRESENTATION_KIND } from '../../../../../workbench/contrib/onboarding/browser/spotlight/spotlightTypes.js';
99
import { IOnboardingScenario } from '../../../../../workbench/contrib/onboarding/common/onboardingScenario.js';
10+
import { SessionHarnessPickerVisibleContext } from '../../../../common/contextkeys.js';
1011
import { NEW_SESSION_ONBOARDING_SEEN_KEY } from './newSessionTour.js';
1112
import { createNewSessionViewRecentTourWhen, createNewSessionViewWorkspaceStep } from './newSessionViewTourShared.js';
1213

@@ -30,6 +31,7 @@ const newSessionViewV2Payload: ISpotlightPayload = {
3031
placement: 'above',
3132
missingTarget: WAIT_FOR_PICKER,
3233
openTarget: false,
34+
when: SessionHarnessPickerVisibleContext,
3335
allowTargetInteraction: true,
3436
},
3537
{

‎src/vs/sessions/contrib/onboardingTours/test/browser/newSessionViewV2Tour.test.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import assert from 'assert';
77
import { observableValue } from '../../../../../base/common/observable.js';
88
import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js';
9-
import { AgentHostSessionTypesAvailableContext, IsNewChatSessionContext, SessionHasWorkspaceContext } from '../../../../common/contextkeys.js';
9+
import { AgentHostSessionTypesAvailableContext, IsNewChatSessionContext, SessionHarnessPickerVisibleContext, SessionHasWorkspaceContext } from '../../../../common/contextkeys.js';
1010
import { createNewSessionViewV2Tour, NEW_SESSION_VIEW_V2_TOUR_ID } from '../../browser/tours/newSessionViewV2Tour.js';
1111
import { createNewSessionViewV3Tour } from '../../browser/tours/newSessionViewV3Tour.js';
1212
import { NEW_SESSION_ONBOARDING_SEEN_KEY } from '../../browser/tours/newSessionTour.js';
@@ -34,6 +34,7 @@ suite('NewSessionViewV2Tour', () => {
3434
openTarget: step.openTarget,
3535
allowTargetInteraction: step.allowTargetInteraction,
3636
advanceWhenWorkspaceSelected: step.advanceWhen === SessionHasWorkspaceContext,
37+
requiresInteractiveHarnessPicker: step.when === SessionHarnessPickerVisibleContext,
3738
})),
3839
}, {
3940
id: NEW_SESSION_VIEW_V2_TOUR_ID,
@@ -52,6 +53,7 @@ suite('NewSessionViewV2Tour', () => {
5253
openTarget: true,
5354
allowTargetInteraction: true,
5455
advanceWhenWorkspaceSelected: true,
56+
requiresInteractiveHarnessPicker: false,
5557
},
5658
{
5759
id: 'harnessPicker',
@@ -60,6 +62,7 @@ suite('NewSessionViewV2Tour', () => {
6062
openTarget: false,
6163
allowTargetInteraction: true,
6264
advanceWhenWorkspaceSelected: false,
65+
requiresInteractiveHarnessPicker: true,
6366
},
6467
{
6568
id: 'modelPicker',
@@ -68,6 +71,7 @@ suite('NewSessionViewV2Tour', () => {
6871
openTarget: true,
6972
allowTargetInteraction: true,
7073
advanceWhenWorkspaceSelected: false,
74+
requiresInteractiveHarnessPicker: false,
7175
},
7276
],
7377
});

0 commit comments

Comments
 (0)