Skip to content

fix: adjust indentation for collapsed rows in chat models widget#316357

Open
AndyXie0718 wants to merge 5 commits into
microsoft:mainfrom
AndyXie0718:modelswidget-provider-indentation-fix
Open

fix: adjust indentation for collapsed rows in chat models widget#316357
AndyXie0718 wants to merge 5 commits into
microsoft:mainfrom
AndyXie0718:modelswidget-provider-indentation-fix

Conversation

@AndyXie0718
Copy link
Copy Markdown

Summary

Fix inconsistent indentation of vendor/group headers in Chat Models widget when expanding/collapsing.

Previously, collapsed groups sometimes caused wrong rows to shift, due to leftover status-icon display states from table row recycling. Now, expanded groups show a small indent (via visible status-icon placeholder) while collapsed groups have no extra indent (status-icon hidden), consistent with the intended visual hierarchy.

Changes

  • In ModelNameColumnRenderer, explicitly control .status-icon display based on entry.collapsed:
    • collapsed === falsedisplay: '' (restore default, icon occupies space → indent)
    • collapsed === truedisplay: 'none' (icon hidden → no indent)

Affected files

  • src/vs/workbench/contrib/chat/browser/chatManagement/chatModelsWidget.ts

Copilot AI review requested due to automatic review settings May 14, 2026 03:48
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

Fixes inconsistent indentation for vendor/group header rows in the Chat Models widget when expanding/collapsing, addressing visual glitches caused by recycled table row templates retaining prior DOM state.

Changes:

  • Toggle the .status-icon element’s display for vendor and group rows based on entry.collapsed to control indentation consistently.
Comments suppressed due to low confidence (1)

src/vs/workbench/contrib/chat/browser/chatManagement/chatModelsWidget.ts:415

  • Same as vendor rows: because table cells are recycled, statusIcon may still have Codicon classes from a prior status entry. Before setting display for group rows, reset the icon element back to its base class/state so expanded groups don’t render a stale status glyph.
	override renderGroupElement(entry: ILanguageModelGroupEntry, index: number, templateData: IModelNameColumnTemplateData): void {
		templateData.statusIcon.style.display = entry.collapsed ? 'none' : '';
		templateData.nameLabel.set(entry.label, undefined);

@AndyXie0718
Copy link
Copy Markdown
Author

@lramos15

@lramos15
Copy link
Copy Markdown
Member

Do you have a photo of what the problem is?

@AndyXie0718
Copy link
Copy Markdown
Author

AndyXie0718 commented May 14, 2026

Do you have a photo of what the problem is?

Sure, when entering the Chat: Manage Language Models interface, the model provider "Copilot" have indentations and "OAI Compatible" haven't, which is strange in the first place.
image

When the row "OAI Compatible" is collasped, the indentations in row "Copilot" vanish, and "OAI Compatible" remains the same which I think is inappropriate because they are from different groups, and row "Copilot" wrongly delete the indentations when group "OAI Compatibale" collapsed.

image

@AndyXie0718
Copy link
Copy Markdown
Author

@lramos15 These problems could be reproduced on win11 23H2 12th Gen Intel(R) Core(TM) i5-12500H and win10 intel i5-1035g4

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@AndyXie0718 AndyXie0718 force-pushed the modelswidget-provider-indentation-fix branch from df50237 to 2069732 Compare May 15, 2026 12:40
@AndyXie0718
Copy link
Copy Markdown
Author

AndyXie0718 commented May 15, 2026

@lramos15 Here is how I figured out what's happened:
image
we can see that now the string "Copilot" has an indent, which comes from the element "statusIcon"; when it is displayed by default, ('display': 'block')
image
when we change the display atrribute into 'none', of course the indent vanished.
image
And the problem is that group title "Copilot" wrongly react (hide its indent) when group "OAI Compatible" collapsed, my solution is to check the attribute "entry.collapsed" and change the attribute "display" before rendering any vender or group element, with the core fixing sentence: templateData.statusIcon.style.display = entry.collapsed ? 'none' : 'block'

@lramos15
Copy link
Copy Markdown
Member

I'm not even sure we need top level status icons. It doesn't make much sense to me

@AndyXie0718
Copy link
Copy Markdown
Author

Perhaps you are right about removing top-level status icons, so in the latest commit I've hidden them with display: 'none' so they no longer cause indentation issues. I'll consider refactoring when I have more time.

@AndyXie0718
Copy link
Copy Markdown
Author

Can you offer some advice on this @sandy081?

@sandy081 sandy081 assigned justschen and unassigned sandy081 May 19, 2026
@AndyXie0718
Copy link
Copy Markdown
Author

Maybe this issue is bigger than we thought, after updating to 1.121, there are similar issues on the deprecation-warning-icon!
When we first enter the Chat: Manage Language Models, things all looks fine.
image
But when I fold and unfold the second group OAI Compatible, a deprecation-warning-icon appeared on the OAI Compatible row, which is rediculous because none of the models in the OAI Compatible group are deprecated.
image
When I unfold OAI Compatible again, the deprecation-warning-icon went to the first group Copilot row.
image

@AndyXie0718
Copy link
Copy Markdown
Author

I pushed a new commit to fix the new issue. Table template instances are reused across row types in ModelNameColumnRenderer, so codicon classes and display state set by a model row can leak into subsequent vendor/group header rows. This commit resets modelStatusIcon.className to 'model-status-icon' and display to 'none' in renderVendorElement and renderGroupElement, following the same cleanup pattern used in renderModelElement.

@AndyXie0718
Copy link
Copy Markdown
Author

@justschen sorry to bother you but I believe this issue still exists in the newest 1.122 version
image

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.

6 participants