Unify isolation checkbox into shared BranchPicker#326414
Draft
benvillalobos wants to merge 18 commits into
Draft
Unify isolation checkbox into shared BranchPicker#326414benvillalobos wants to merge 18 commits into
benvillalobos wants to merge 18 commits into
Conversation
Largely a migration from the standalone IsolationPicker ahead of its deletion. The copilot adapter now drives the isolation checkbox added to the shared BranchPicker, connecting session state (config gating, git repo detection, context key, telemetry) to the new checkbox slot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The branch picker now renders the isolation checkbox internally, so the separate isolationPicker action, view-item factory, and import are no longer needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The type was defined in isolationPicker.ts which is being deleted. Relocated next to the STORAGE_KEY_ISOLATION_MODE constant where all isolation-mode session logic lives. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The chip-and-dropdown isolation UI is replaced by the same checkbox the shared BranchPicker now renders, unifying the isolation control across copilotChatSessions and automations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The chip element no longer exists — its styles are dead code. Branch- slot styles and the divider box-shadow between slots are preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fold isolation picker checkbox tests into the copilot branchPicker test. Rewrite automation dialog tests to assert checkbox state instead of the removed chip/dropdown. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fully replaced by the isolation checkbox in the shared BranchPicker. The IsolationMode type now lives in copilotChatSessionsProvider.ts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When isolation is configured, wrap both slots in a flex-row container so the checkbox and branch trigger sit side by side. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
BaseActionViewItem.render() registers a MOUSE_DOWN listener that adds an .active class to the container, which triggers the workbench toolbar rule that highlights all .action-label descendants. Since the container now holds two action-labels (the isolation checkbox and the branch trigger), clicking either one highlighted both. The copilot PickerActionViewItem already skips super.render() for the same reason. The automation dialog called it but immediately wiped the DOM with clearNode — the only lasting side effect was the unwanted .active listener. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Removing super.render() dropped the MOUSE_DOWN preventDefault that was suppressing native text selection. Add user-select: none to the wrapper instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Unifies worktree isolation into the shared branch picker across Copilot sessions and automation dialogs.
Changes:
- Adds an optional isolation checkbox to
BranchPicker. - Migrates Copilot and automation isolation controls to it.
- Removes the standalone isolation picker and consolidates tests.
Show a summary per file
| File | Description |
|---|---|
aiCustomizationManagement.css |
Removes obsolete automation chip styling. |
isolationPicker.test.ts |
Deletes standalone picker tests. |
branchPicker.test.ts |
Adds Copilot isolation checkbox tests. |
isolationPicker.css |
Deletes standalone picker styles. |
isolationPicker.ts |
Removes the standalone picker. |
copilotChatSessionsProvider.ts |
Relocates IsolationMode. |
copilotChatSessionsActions.ts |
Removes standalone picker registration. |
Copilot branchPicker.ts |
Adapts session isolation state to the shared picker. |
Shared branchPicker.css |
Adds grouped checkbox styling. |
Shared branchPicker.ts |
Implements optional isolation checkbox support. |
automationDialog.test.ts |
Updates automation isolation assertions. |
automationDialog.ts |
Replaces the isolation dropdown with the checkbox. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 5
- Review effort level: Medium
The onboarding tours reference 'sessions.newSession.isolation' and need a DOM element registered as the target. Wire markOnboardingTarget through the IBranchPickerIsolationOptions.markTarget option. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use 'a.action-label' to disambiguate the branch trigger (an <a>) from the isolation checkbox row (a <div>), matching codebase convention. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Checkbox.disable() sets tabIndex=-1, making it unreachable via keyboard. Override to tabIndex=0 so users can tab to it and discover the disabled reason via the tooltip. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Restore span.sessions-chat-branch-picker-group as the layout owner (inline-flex + min-width:0) instead of inline styles on the caller. - Fix automation divider: drop direct-child selector so box-shadow matches through the wrapper. - Add min-width:0 so labels can ellipsize in narrow toolbars. - Revert PickerActionViewItem inline styles (not its responsibility). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tion-checkbox # Conflicts: # src/vs/sessions/contrib/providers/copilotChatSessions/browser/copilotChatSessionsActions.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The standalone
IsolationPickerwidget and the automation dialog's chip/dropdown were two separate implementations of the same isolation mode control. This PR merges them into a single optional checkbox rendered by the sharedBranchPickerwidget.What changed
The shared
BranchPicker(contrib/chat/browser/branchPicker.ts) now accepts optionalisolationconfiguration. When supplied, it renders a "New Worktree" checkbox before the branch trigger. When omitted, behavior is unchanged.AutomationIsolationModel. TheIActionWidgetServicedependency andsuper.render()call are removed since neither is needed.isolationPicker.ts,isolationPicker.css,isolationPicker.test.ts. TheIsolationModetype moved tocopilotChatSessionsProvider.ts.branchPicker.test.ts. Two automation dialog tests rewritten to assert checkbox state.Notes
SessionIsolationPickerVisibleContextis preserved — ownership moved to the copilot adapter.cc @sandy081