Add separator option to select box by mrleemurray · Pull Request #297703 · microsoft/vscode

mrleemurray · 2026-02-25T15:29:39Z

Introduce a separator option for the select box, enhancing the dropdown's visual organization. Update focus outlines and styling for separators to improve user experience and maintain consistency with the select button's font size.

Co-authored-by: Copilot <copilot@github.com>

…ewItem

…nsistency

…avior Co-authored-by: Copilot <copilot@github.com>

…ive select box

Copilot

Pull request overview

This PR introduces separator support for select box components to improve visual organization in dropdowns. The implementation adds an isSeparator property to select options, provides visual styling via CSS pseudo-elements, updates keyboard navigation to properly skip disabled/separator items, and aligns the select box styling with the action widget design pattern.

Changes:

Added isSeparator property to ISelectOptionItem interface and updated SeparatorSelectOption constant
Enhanced keyboard navigation (arrow keys, PageUp/PageDown, Home/End) to skip over contiguous disabled/separator options
Updated visual styling to match action widget patterns: solid outlines, larger border radius, font-size inheritance, and CSS-based separator rendering

Reviewed changes

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

Show a summary per file

File	Description
src/vs/base/browser/ui/selectBox/selectBox.ts	Added `isSeparator` property to ISelectOptionItem interface and SeparatorSelectOption constant
src/vs/base/browser/ui/selectBox/selectBoxNative.ts	Updated createOption to handle isSeparator flag with role attribute for native select elements
src/vs/base/browser/ui/selectBox/selectBoxCustom.ts	Enhanced keyboard navigation for disabled options, updated focus outline styles to match action widget, added font-size inheritance from select button, added accessibility label for separators
src/vs/base/browser/ui/selectBox/selectBoxCustom.css	Added separator styling with CSS pseudo-element border, removed list-level focus ring, changed border-radius to cornerRadius-large
src/vs/workbench/contrib/debug/browser/debugActionViewItems.ts	Updated debug configuration dropdown to mark separators with isSeparator flag

Copilot · 2026-02-25T15:44:33Z

@mrleemurray I've opened a new pull request, #297709, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2026-02-25T15:45:12Z

@mrleemurray I've opened a new pull request, #297710, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2026-02-25T15:45:16Z

@mrleemurray I've opened a new pull request, #297711, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2026-02-25T15:45:21Z

@mrleemurray I've opened a new pull request, #297712, to work on those changes. Once the pull request is ready, I'll request review from you.

…igation Co-authored-by: Copilot <copilot@github.com>

bpasero

@mrleemurray some review feedback from AI, but I am not sure how relevant:

Invalid ARIA role on <option> — selectBoxNative.ts sets role="separator" on an <option> element, which is not a valid ARIA role for that element. Remove it or use a different approach for the native path.
Menu selection colors silently changed — defaultStyles.ts replaces menuSelectionForeground/menuSelectionBackground with listHoverForeground/listHoverBackground. This breaks theme customizations for menu.selectionForeground and menu.selectionBackground. Should be reverted or split into a separate intentional change.
No tests — The keyboard navigation refactoring (onDownArrow, onUpArrow, onHome, onEnd, onPageUp, onPageDown) is non-trivial and needs tests, especially for edge cases: all options disabled, separators at boundaries, multiple contiguous separators.
onPointerUp doesn't guard against disabled/separator items — Mouse-over now correctly skips disabled items, but clicking directly on a separator row could still trigger selection. onPointerUp needs the same guard.
Separator ::after needs position: relative on parent — In selectBoxCustom.css, the absolute-positioned ::after line relies on the list widget setting position on .monaco-list-row. Add position: relative to .option-separator to make it self-contained.
getComputedStyle called on every dropdown render — selectBoxCustom.ts forces a style recalc each time. Use font-size: inherit in CSS instead, or cache the value.
isSeparator should imply isDisabled — The renderer in selectBoxCustom.ts manually adds option-disabled for separators, but this invariant isn't enforced. Either enforce it at the type/setter level, or document it clearly. As-is, a consumer could set isSeparator: true without isDisabled: true and get inconsistent behavior (CSS shows disabled, but navigation doesn't skip it).
Unrelated visual changes should be split out — Menu border-radius, item height (2em → 24px), focus outline style (1.6px dotted → 1px solid), dropdown corner radius (small → large), theme-2026 backdrop-filter additions, and dropdown container outline removal are all unrelated to the separator feature and should be in a separate PR for clean bisection.

Copilot

Pull request overview

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

Copilot · 2026-03-01T08:29:57Z

+			let candidate = this.selectList.getFocus()[0];

-			// Shift selection up if we land on a disabled option
-			if (this.options[this.selected].isDisabled && this.selected > 0) {
-				this.selected--;
-				this.selectList.setFocus([this.selected]);
+			// Shift selection down if we land on a disabled option
+			while (candidate < this.options.length - 1 && this.options[candidate].isDisabled) {
+				candidate++;
 			}
+			if (this.options[candidate].isDisabled) {
+				return;
+			}


In onPageDown, if the focused row lands on a disabled option (e.g. a separator) and there is no enabled option below it, the callback returns without restoring focus. That leaves focus on a disabled row while this.selected remains unchanged, which can confuse focus/selection state.

Consider resetting focus back to the last valid selection (or nearest enabled option in the opposite direction) instead of returning with the list focused on a disabled item.

Copilot · 2026-03-01T08:29:57Z

 	backgroundColor: asCssVariable(menuBackground),
-	selectionForegroundColor: asCssVariable(menuSelectionForeground),
-	selectionBackgroundColor: asCssVariable(menuSelectionBackground),
+	selectionForegroundColor: asCssVariable(listHoverForeground),
+	selectionBackgroundColor: asCssVariable(listHoverBackground),


defaultMenuStyles now uses listHoverForeground/background for the selected (focused) menu item. This breaks theming for menu.selectionForeground / menu.selectionBackground because those color tokens will no longer have any effect on menus and the menubar.

Use the menu-specific tokens (menuSelectionForeground/menuSelectionBackground) for selection colors so menu theming stays consistent and configurable.

Copilot · 2026-03-01T08:29:57Z

+		if (isSeparator) {
+			option.setAttribute('role', 'separator');
+		}



Setting role="separator" on an <option> is not a valid ARIA pattern (an <option> has an implicit option role and many screen readers/browsers ignore or warn on role overrides). This can lead to accessibility tooling warnings without improving separator semantics.

Suggestion: remove the role attribute and rely on disabled + the separator text for native select rendering, or switch to a different native structure (e.g. <optgroup>) if true native separators are required.

Suggested change

if (isSeparator) {

option.setAttribute('role', 'separator');

}

Copilot · 2026-03-01T08:29:58Z

+			let candidate = this.selectList.getFocus()[0];

-			// Shift selection down if we land on a disabled option
-			if (this.options[this.selected].isDisabled && this.selected < this.options.length - 1) {
-				this.selected++;
-				this.selectList.setFocus([this.selected]);
+			// Shift selection up if we land on a disabled option
+			while (candidate > 0 && this.options[candidate].isDisabled) {
+				candidate--;
+			}
+			if (this.options[candidate].isDisabled) {
+				return;
 			}


In onPageUp, if the focused row lands on a disabled option (e.g. a separator) and there is no enabled option above it, the callback returns without restoring focus. This can leave the list focus sitting on a disabled row while this.selected still points elsewhere, which makes subsequent keyboard interactions inconsistent.

Consider falling back to the previous this.selected/_currentSelection (or the nearest enabled option in the other direction) and explicitly resetting list focus before returning.

mrleemurray and others added 5 commits February 24, 2026 17:04

feat(selectBox): add separator option styling and update focus outlines

b834690

Co-authored-by: Copilot <copilot@github.com>

feat(debug): add separator option to select box in StartDebugActionVi…

0f53f0e

…ewItem

feat(selectBox): inherit font size from select button for dropdown co…

40db213

…nsistency

feat(selectBox): enhance separator handling and update focus ring beh…

7f9ec73

…avior Co-authored-by: Copilot <copilot@github.com>

feat(selectBox): enhance option creation to support separators in nat…

5039f6a

…ive select box

Copilot AI review requested due to automatic review settings February 25, 2026 15:29

mrleemurray enabled auto-merge February 25, 2026 15:29

mrleemurray self-assigned this Feb 25, 2026

mrleemurray requested a review from bpasero February 25, 2026 15:29

Copilot started reviewing on behalf of mrleemurray February 25, 2026 15:30 View session

vs-code-engineering bot added this to the February 2026 milestone Feb 25, 2026

Copilot AI reviewed Feb 25, 2026

View reviewed changes

Copilot AI mentioned this pull request Feb 25, 2026

Fix: guard against all-disabled options in select box page/home/end navigation #297709

Draft

Copilot AI mentioned this pull request Feb 25, 2026

fix: guard against all-disabled options in select box keyboard navigation #297710

Closed

This was referenced Feb 25, 2026

fix(selectBox): guard against all-disabled options in page/home/end navigation #297711

Closed

Fix edge case where all options disabled causes select box to land on disabled option #297712

Closed

Merge branch 'main' into mrleemurray/custom-menu-update

49ad861

austindyoung mentioned this pull request Feb 25, 2026

[mirror] microsoft/vscode#297703 Add separator option to select box austindyoung/vscode-driftfence-mirror-fork#84

Open

bpasero modified the milestones: February 2026, March 2026 Feb 26, 2026

mrleemurray and others added 5 commits February 26, 2026 11:21

fix(selectBox): improve handling of disabled options and separators

911b82a

fix(selectBox): enhance handling of disabled options in selection nav…

aed186d

…igation Co-authored-by: Copilot <copilot@github.com>

Align custom menu styling with chat picker visuals

a6bcce1

fix(imports): reorder imports for better readability

b6843d2

Merge branch 'main' into mrleemurray/custom-menu-update

fe85be6

bpasero requested a review from Copilot March 1, 2026 08:24

Copilot started reviewing on behalf of bpasero March 1, 2026 08:25 View session

bpasero approved these changes Mar 1, 2026

View reviewed changes

mrleemurray merged commit e46f6f2 into main Mar 1, 2026
22 checks passed

mrleemurray deleted the mrleemurray/custom-menu-update branch March 1, 2026 08:27

Copilot AI reviewed Mar 1, 2026

View reviewed changes

vs-code-engineering bot locked and limited conversation to collaborators Apr 15, 2026

	if (isSeparator) {
	option.setAttribute('role', 'separator');
	}

Conversation

mrleemurray commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Feb 25, 2026

Uh oh!

Copilot AI commented Feb 25, 2026

Uh oh!

Copilot AI commented Feb 25, 2026

Uh oh!

Copilot AI commented Feb 25, 2026

Uh oh!

bpasero left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mrleemurray commented Feb 25, 2026 •

edited

Loading

bpasero left a comment •

edited

Loading