Skip to content

debt - cleanup from sidebar support in modal editors#306141

Merged
bpasero merged 6 commits intomainfrom
ben/colossal-mockingbird
Mar 29, 2026
Merged

debt - cleanup from sidebar support in modal editors#306141
bpasero merged 6 commits intomainfrom
ben/colossal-mockingbird

Conversation

@bpasero
Copy link
Copy Markdown
Member

@bpasero bpasero commented Mar 29, 2026

Removes leftover modal-editor sidebar/min-width configurability plumbing and centralizes "changes list" rendering/styling so the same list can be used in the modal editor sidebar.

Changes Made

  • Removed workbench.editor.modalMinWidth configuration and simplified modal min-width handling.
  • Refactored ChangesViewPane to reuse a shared createChangesTree helper for both the main Changes view and the modal sidebar list; adjusted CSS scoping accordingly.
  • Parameterized createChangesTree with an optional getSelection callback so the ChangesViewActionRunner for the sidebar tree reads from the sidebar tree's own selection instead of the main view's selection.
  • Added a new unit test suite to validate modal sidebar behaviors.
  • Fixed JSDoc comment on IModalEditorPartOptions.sidebar to accurately describe that the sidebar cannot be added, removed, or updated after the modal editor is opened.

Testing

  • TypeScript compilation passes with no errors.
  • Verify that row toolbar actions in the sidebar list operate on the sidebar tree's selection rather than the main Changes view's selection.

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Copilot AI review requested due to automatic review settings March 29, 2026 19:05
@bpasero bpasero enabled auto-merge (squash) March 29, 2026 19:05
@bpasero bpasero self-assigned this Mar 29, 2026
@vs-code-engineering vs-code-engineering bot added this to the Backlog milestone Mar 29, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Removes leftover modal-editor sidebar/min-width configurability plumbing and centralizes “changes list” rendering/styling so the same list can be used in the modal editor sidebar.

Changes:

  • Removed workbench.editor.modalMinWidth configuration and simplified modal min-width handling.
  • Refactored ChangesViewPane to reuse a shared tree-creation helper for both the main Changes view and the modal sidebar list; adjusted CSS scoping accordingly.
  • Added a new unit test suite intended to validate modal sidebar behaviors.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/vs/workbench/test/browser/parts/editor/modalEditorSidebar.test.ts Adds a new test harness + tests for modal sidebar width/content behaviors.
src/vs/workbench/browser/workbench.contribution.ts Removes the workbench.editor.modalMinWidth setting contribution.
src/vs/workbench/browser/parts/editor/modalEditorPart.ts Simplifies effective min-width calculation and removes sidebar dynamic update wiring.
src/vs/workbench/browser/parts/editor/media/modalEditorPart.css Removes modal-editor sidebar styling that was specific to changes-list badges/stats.
src/vs/sessions/contrib/configuration/browser/configuration.contribution.ts Removes sessions default override for workbench.editor.modalMinWidth.
src/vs/sessions/contrib/changes/browser/media/changesView.css Broadens selectors to style .chat-editing-session-list outside .changes-view-body.
src/vs/sessions/contrib/changes/browser/changesView.ts Extracts createChangesTree and reuses it for the modal editor sidebar list.
src/vs/platform/editor/common/editor.ts Updates sidebar option documentation to reflect reduced dynamic behavior.
Comments suppressed due to low confidence (2)

src/vs/sessions/contrib/changes/browser/changesView.ts:1258

  • createChangesTree hard-codes the tree id as 'ChangesViewTree'. This method is now used to create both the main Changes tree and the modal editor sidebar tree, so the ids will collide if both exist (state/telemetry/context can become ambiguous). Consider accepting the id as a parameter (e.g. 'ChangesViewTree' vs 'ModalEditorSidebar').
		return disposables.add(this.instantiationService.createInstance(
			WorkbenchCompressibleObjectTree<ChangesTreeElement>,
			'ChangesViewTree',
			container,

src/vs/sessions/contrib/changes/browser/changesView.ts:1266

  • createChangesTree uses a widget aria label of “Changes Tree” for all instances. This regresses the modal sidebar list accessibility label (previously “Files”). Consider parameterizing the aria label (and related sidebar-specific options like multipleSelectionSupport, compressionEnabled, and dnd) so the sidebar tree can keep an appropriate, context-specific label.
				accessibilityProvider: {
					getAriaLabel: (element: ChangesTreeElement) => isChangesFileItem(element) ? basename(element.uri.path) : element.name,
					getWidgetAriaLabel: () => localize('changesViewTree', "Changes Tree")
				},

bpasero and others added 2 commits March 29, 2026 21:13
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@bpasero bpasero merged commit 9a6bf81 into main Mar 29, 2026
18 checks passed
@bpasero bpasero deleted the ben/colossal-mockingbird branch March 29, 2026 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants