Skip to content

Fix toolbar responsive overflow oscillation in chat input#303780

Draft
roblourens wants to merge 1 commit intomainfrom
roblou/fix-toolbar-overflow-oscillation
Draft

Fix toolbar responsive overflow oscillation in chat input#303780
roblourens wants to merge 1 commit intomainfrom
roblou/fix-toolbar-overflow-oscillation

Conversation

@roblourens
Copy link
Member

Fixes #296060

Problem

The model dropdown in the chat input toolbar flickers between being visible and hidden in the overflow ("...") menu. Resizing the chat input by even 1px would temporarily fix it, but the oscillation would return.

The root cause is in ToolBar.updateActions(). The responsive overflow logic used a mutually exclusive if/else structure — in a single call it would either hide items into the overflow menu OR show items from the overflow menu, but never both. This created an oscillation cycle:

  1. Hide path: All 4 toolbar items (attach, mode picker, model picker, tools) totaling ~424px exceed the container width. The tools button gets hidden into overflow. The overflow menu is added.
  2. CSS flex-shrink activates: With overflow present, the model picker (now nth-last-child(2)) gets flex-shrink: 1 via CSS and shrinks from ~291px to ~278px. Total drops to ~409px.
  3. Show path (next call): actionBarWidth(false) uses min-widths and reports ~153px ≤ container. The show-check computes 409 - 24 + 22 + 4 = 411 which fits. Tools button is restored, overflow menu removed.
  4. CSS flex-shrink deactivates: Model picker loses flex-shrink, expands back to 291px. Total goes back to 424px.
  5. Next call: 424 > container → back to step 1. Infinite oscillation.

Fix

Core fix — sequential show then hide: Changed updateActions() from if (overflow) { hide } else { show } to running show first, then hide. After the show loop restores items and removes the overflow menu, the hide check runs in the same call. If CSS flex-state changes cause the total to exceed the container again, items are immediately re-hidden, converging to a stable state without oscillation.

Supporting fixes:

  • has-overflow immediate toggle: Toggle the has-overflow CSS class on the action bar DOM node immediately when adding/removing the overflow menu (not just at the end of updateActions). This ensures CSS flex-shrink targets the correct item (nth-last-child(2) vs :last-child) during width checks within the same call.
  • freedOverflowWidth in show-check: When showing the last hidden item would also remove the overflow menu (no secondary actions), account for the freed space in the show-check calculation. Without this, items that would fit after overflow menu removal stayed hidden (hysteresis).
  • actionBarWidth(false) padding fix: The min-width calculation for the last primary item incorrectly included ACTION_PADDING (4px gap). The last primary item has no trailing gap, so this overestimated by 4px.
  • containerWidth <= 0 guard: Skip overflow logic when the element hasn't been laid out yet (not in DOM or display:none). The ResizeObserver will call again once the element gets a real size.
  • relayout() public API: New method for external callers to force re-evaluation of overflow when action view items change their rendered size without the toolbar being notified (e.g. label text changes).
  • chatInputPart relayout calls: Call relayout() when the model selection changes (picker label changes size) and when hideChevrons observable changes (chevrons collapse/expand changes item widths), since neither triggers the toolbar's ResizeObserver.

Tests

Added 6 unit tests in toolbar.test.ts covering the responsive overflow behavior:

  1. relayout hides items that no longer fit — items are hidden when container shrinks
  2. relayout restores items when container grows — items return when container grows
  3. relayout restores last hidden item by accounting for overflow menu removal — the freedOverflowWidth calculation correctly allows restoration
  4. relayout does nothing when container width is zero — the <= 0 guard works
  5. relayout re-evaluates after item size changes — items that grow beyond container are hidden
  6. kind=last does not over-hide — the ACTION_PADDING fix prevents incorrect hiding of items that fit

Enhanced chatInput.fixture.ts with model picker rendering and narrow-width variants for visual testing.

Validation

  • All 18,413 unit tests pass (0 failures)
  • TypeScript compilation clean (no errors)
  • Hygiene checks pass (pre-commit hook)
  • Manual testing confirmed no flickering at any width, model picker correctly shrinks when tools button is in overflow, and all items restore when container grows

(Written by Copilot)

The model dropdown in the chat input toolbar would flicker between
being visible and hidden in the overflow menu. This was caused by the
show/hide logic in updateActions() using a mutually exclusive if/else
structure that couldn't converge when CSS flex-shrink changed the
effective widths between show and hide states.

Changes:
- Restructure updateActions() to run show then hide sequentially
  instead of exclusively, so the system converges in a single call
- Extract hideOverflowingActions() to avoid code duplication
- Toggle has-overflow CSS class immediately when adding/removing the
  overflow menu so flex-shrink targets the correct item within the
  same updateActions() call
- Account for freed overflow menu space in the show-check calculation
- Fix actionBarWidth(false) to not include ACTION_PADDING for the
  last primary item (no trailing gap)
- Add containerWidth <= 0 guard to skip overflow logic before layout
- Add relayout() public API for external callers
- Call relayout() from chatInputPart when model selection changes or
  hideChevrons observable changes, since these resize toolbar children
  without triggering the toolbar's ResizeObserver

(Written by Copilot)
Copilot AI review requested due to automatic review settings March 21, 2026 21:08
@vs-code-engineering vs-code-engineering bot added this to the 1.113.0 milestone Mar 21, 2026
@roblourens roblourens marked this pull request as draft March 21, 2026 21:10
Copy link
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 fixes a responsive overflow oscillation (flicker) in the chat input toolbar by adjusting ToolBar.updateActions() to converge in a single pass and by adding explicit relayout triggers when toolbar items change size without a container resize.

Changes:

  • Reworked ToolBar.updateActions() responsive overflow logic to run “show then hide”, added guards/width fixes, and introduced a relayout() API for externally-triggered recalculation.
  • Updated ChatInputPart to call relayout() when model label sizing or chevron visibility changes affect toolbar item widths.
  • Added unit tests for responsive overflow behavior and enhanced the chat input fixture with narrow variants and model-picker rendering setup.

Reviewed changes

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

File Description
src/vs/base/browser/ui/toolbar/toolbar.ts Fixes overflow oscillation by sequencing show/hide, improves width calculations, adds relayout() and a zero-width guard.
src/vs/workbench/contrib/chat/browser/widget/input/chatInputPart.ts Forces toolbar overflow re-evaluation after model/chevron-driven width changes.
src/vs/base/test/browser/ui/toolbar/toolbar.test.ts Adds unit tests validating the responsive overflow convergence and width edge cases.
src/vs/workbench/test/browser/componentFixtures/chatInput.fixture.ts Enhances fixture to render model picker state and adds narrow-width fixture variants.

Comment on lines +256 to +259
inputPart.layout(width);
await new Promise(r => setTimeout(r, 100));
inputPart.layout(500);
inputPart.layout(width);

Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

Now that width is configurable via fixtureOptions, the later if (editingSession) { ... inputPart.layout(500); } still hard-codes 500px. This makes the fixture behavior inconsistent and would break narrow variants if combined with editingSession. Consider using the width variable consistently for all layout(...) calls in this helper.

Copilot uses AI. Check for mistakes.

// 3 items [22, 77, 110]. Actual CSS total = (22+4)+(77+4)+110 = 217.
// Container = 220, so everything fits.
// Old actionBarWidth(false): (22+4)+(77+4)+26 = 133 (wrong, too high - waitthis is less...).
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

Typo in the explanatory comment: "waitthis" should be split into "wait this" (or rephrase the sentence).

Suggested change
// Old actionBarWidth(false): (22+4)+(77+4)+26 = 133 (wrong, too high - waitthis is less...).
// Old actionBarWidth(false): (22+4)+(77+4)+26 = 133 (wrong, too high - wait, this is less...).

Copilot uses AI. Check for mistakes.
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.

Chat input toolbar issues

2 participants