fix: prevent endless rAF loop in layout when widget is hidden#312183
fix: prevent endless rAF loop in layout when widget is hidden#312183joshspicer merged 2 commits intomainfrom
Conversation
The layout methods in mcpListWidget, pluginListWidget, and aiCustomizationListWidget would defer to requestAnimationFrame when searchAndButtonContainer.offsetHeight was 0, assuming the browser hadn't reflowed yet. When the widget is created while permanently hidden (e.g. in component explorer or when the view isn't visible), offsetHeight stays 0 forever, causing an infinite rAF loop. Add a _layoutDeferred flag so the deferral happens at most once. If offsetHeight is still 0 after the retry, proceed with zero heights instead of the next real layout call when the widgetlooping becomes visible will compute correct dimensions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
084ff82 to
6059ed4
Compare
There was a problem hiding this comment.
Pull request overview
Fixes a UI performance/behavior issue in the AI Customization-related list widgets where layout() could get stuck in an endless requestAnimationFrame retry loop when the widget is created while permanently hidden (so offsetHeight remains 0).
Changes:
- Add a
_layoutDeferredguard flag tolayout()in three widgets. - Update the inline comments to explain the single-retry intent and the “permanently hidden” scenario.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/browser/aiCustomization/pluginListWidget.ts | Adds _layoutDeferred gating around the rAF-based layout deferral. |
| src/vs/workbench/contrib/chat/browser/aiCustomization/mcpListWidget.ts | Adds _layoutDeferred gating around the rAF-based layout deferral. |
| src/vs/workbench/contrib/chat/browser/aiCustomization/aiCustomizationListWidget.ts | Adds _layoutDeferred gating around the rAF-based layout deferral. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 3
| if (searchBarHeight === 0 && !this._layoutDeferred) { | ||
| this._layoutDeferred = true; | ||
| DOM.getWindow(this.element).requestAnimationFrame(() => { | ||
| this._layoutDeferred = false; |
There was a problem hiding this comment.
The _layoutDeferred flag is reset to false inside the rAF callback before calling layout() again. If offsetHeight stays 0 (e.g. widget remains hidden), the next layout() call will see !this._layoutDeferred and schedule another rAF, so the endless loop still happens. Keep the flag set across the retried layout() call and only clear it after the second attempt has proceeded without deferring.
| this._layoutDeferred = false; |
| if (searchBarHeight === 0 && !this._layoutDeferred) { | ||
| this._layoutDeferred = true; | ||
| DOM.getWindow(this.element).requestAnimationFrame(() => { | ||
| this._layoutDeferred = false; |
There was a problem hiding this comment.
The rAF retry resets _layoutDeferred to false before calling layout() again. If searchAndButtonContainer.offsetHeight remains 0, the next layout() invocation will defer again, so this does not actually cap retries to one and can still spin forever. Consider leaving _layoutDeferred set for the retried call and clearing it only after that call has proceeded without scheduling another rAF.
| this._layoutDeferred = false; |
| this._layoutDeferred = false; | ||
| this.layout(height, width); |
There was a problem hiding this comment.
_layoutDeferred is cleared inside the rAF callback before calling layout(height, width) again. If the widget remains hidden and offsetHeight stays 0, the retried layout() call will see !this._layoutDeferred and schedule another rAF, so this can still loop indefinitely. Keep _layoutDeferred set across the retried call (and clear it only after the second attempt continues) so the deferral truly happens at most once.
| this._layoutDeferred = false; | |
| this.layout(height, width); | |
| try { | |
| this.layout(height, width); | |
| } finally { | |
| this._layoutDeferred = false; | |
| } |
Screenshot ChangesBase: Changed (34) |
Keep _layoutDeferred set while the deferred layout call runs so a second offsetHeight === 0 measurement cannot schedule another requestAnimationFrame. This makes the retry truly one-shot in all three AI customization list widgets. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The
layout()methods inmcpListWidget,pluginListWidget, andaiCustomizationListWidgetdefer torequestAnimationFramewhensearchAndButtonContainer.offsetHeightis 0, assuming the browser hasn't reflowed yet after adisplay:none→ visible transition.When the widget is created while permanently hidden (e.g. in component explorer, or when the MCP servers view is created but not visible),
offsetHeightstays 0 forever, causing an infinite rAF loop.Fix
Add a
_layoutDeferredflag so the deferral happens at most once. IfoffsetHeightis still 0 after the single retry, we proceed with zero heights instead of looping — the next reallayout()call when the widget becomes visible will compute correct dimensions.