Skip to content

clean up confirmation carousel - no more collapse and better focus logic#311270

Merged
ulugbekna merged 5 commits intomainfrom
justin/slurpuff
Apr 20, 2026
Merged

clean up confirmation carousel - no more collapse and better focus logic#311270
ulugbekna merged 5 commits intomainfrom
justin/slurpuff

Conversation

@justschen
Copy link
Copy Markdown
Collaborator

fix #309535

Copilot AI review requested due to automatic review settings April 19, 2026 21:09
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 19, 2026

Screenshot Changes

Base: 0c576529 Current: 7b3b702d

Changed (1)

editor/inlineCompletions/other/JumpToHint/Dark
Before After
before after

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

This PR updates the chat tool confirmation carousel UI to resolve focus-outline clipping/stacking issues reported in #309535, while also simplifying the carousel by removing the collapse/expand behavior and adding a “dismiss/skip all” action.

Changes:

  • Removes the carousel’s collapse + content expansion logic and related resize observation/sizing heuristics.
  • Adds a new dismiss button that skips/denies all pending confirmations.
  • Adjusts carousel focus styling to avoid outline clipping by switching to border-based focus indication.
Show a summary per file
File Description
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/toolInvocationParts/chatToolConfirmationCarouselPart.ts Simplifies carousel behavior (no collapse/expand) and adds a dismiss (“skip all”) action.
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatToolConfirmationCarousel.css Updates focus styling and removes CSS related to collapsed/expanded carousel states.

Copilot's findings

Comments suppressed due to low confidence (3)

src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatToolConfirmationCarousel.css:155

  • .monaco-list:focus { outline: none !important; } removes the focus indicator for lists inside the carousel. If the goal is to avoid non-keyboard focus rings, limit this to :focus:not(:focus-visible) or provide an alternative :focus-visible style so keyboard users can still see focus.
	.monaco-list:focus {
		outline: none !important;
	}

src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatToolConfirmationCarousel.css:42

  • .chat-tool-carousel-content .chat-confirmation-widget-container is explicitly focusable (tabIndex=0), but this rule removes its focus outline for all focus states. Unless it’s replaced by another visible focus style, this makes keyboard focus effectively invisible. Prefer limiting outline removal to :focus:not(:focus-visible) or adding a dedicated :focus-visible style (and/or rely on a :focus-within border on the carousel).
	.chat-tool-carousel-content .chat-confirmation-widget-container:focus {
		outline: none;
	}

src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatToolConfirmationCarousel.css:127

  • This rule removes outline/box-shadow for all button:focus / .monaco-button:focus, including :focus-visible. That will eliminate the keyboard focus indicator on the Allow/Skip/Dismiss buttons and nav arrows. The previous :focus:not(:focus-visible) approach avoids mouse-focus rings while preserving keyboard focus; alternatively add explicit :focus-visible styling for these controls.
	button:focus,
	.monaco-button:focus {
		outline: none !important;
		box-shadow: none !important;
	}
  • Files reviewed: 2/2 changed files
  • Comments generated: 3

justschen and others added 3 commits April 19, 2026 14:15
…toolInvocationParts/chatToolConfirmationCarouselPart.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…media/chatToolConfirmationCarousel.css

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…toolInvocationParts/chatToolConfirmationCarouselPart.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@justschen justschen closed this Apr 19, 2026
auto-merge was automatically disabled April 19, 2026 22:35

Pull request was closed

@justschen justschen reopened this Apr 19, 2026
@ulugbekna ulugbekna merged commit fd55f11 into main Apr 20, 2026
44 of 49 checks passed
@ulugbekna ulugbekna deleted the justin/slurpuff branch April 20, 2026 10:31
@vs-code-engineering vs-code-engineering Bot added this to the 1.118.0 milestone Apr 20, 2026
@justschen justschen added the ~release-cherry-pick Trigger: cherry-pick this PR to the latest release branch label Apr 20, 2026
@vs-code-engineering vs-code-engineering Bot added release-cherry-pick Automated cherry-pick between release and main branches and removed ~release-cherry-pick Trigger: cherry-pick this PR to the latest release branch labels Apr 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-cherry-pick Automated cherry-pick between release and main branches

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Testing: Confirmation carousel focus outline hidden

4 participants