-
Notifications
You must be signed in to change notification settings - Fork 37.9k
Improve context widget UX based on feedback #291344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this 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 improves the context widget UX based on user feedback. The main changes are: moving the widget from the title control to the chat input area, changing interaction from click-based to hover-based (with click for sticky behavior), and visual improvements including fixing the double border issue and simplifying the widget appearance.
Changes:
- Widget relocated from chat view title control to chat input part with absolute positioning for automatic repositioning on resize
- Interaction model changed to show details on hover (delayed) or on click/keyboard activation (sticky with focus)
- Visual simplification: token label removed from widget itself, now shown only in hover details; padding and margins reduced throughout for more compact appearance
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| chatViewTitleControl.css | Removed context usage widget styles and moved margin-left to actions toolbar |
| chatContextUsageWidget.css | Simplified styles by removing token label, animations, and transitions; centered icon display |
| chatContextUsageDetails.css | Reduced padding/margins for more compact appearance; added CSS to prevent double border on focus |
| chat.css | Added absolute positioning for context usage widget container in chat input |
| chatViewTitleControl.ts | Removed context usage widget from title control |
| chatContextUsageWidget.ts | Refactored to use hover service's delayed hover; added click and keyboard handlers for sticky hover; removed token label display logic |
| chatContextUsageDetails.ts | Added token count display to details view; moved formatTokenCount utility here |
| chatInputPart.ts | Added context usage widget to input part with proper lifecycle management |
Comments suppressed due to low confidence (1)
src/vs/workbench/contrib/chat/browser/widgetHosts/viewPane/chatContextUsageWidget.ts:180
- Memory leak: Similar to the issue in resolveHoverOptions, a new ChatContextUsageDetails instance is created here but never disposed. The instance is passed to the hover service, which will dispose its own internal structures but does not know that the content is a Disposable. If the hover is not shown (returns undefined on line 178), the details instance is disposed, but if the hover is shown successfully, the details instance is never disposed, leading to a memory leak.
const details = this.instantiationService.createInstance(ChatContextUsageDetails);
details.update(this.currentData);
const hover = this.hoverService.showInstantHover(
{
content: details.domNode,
target: this.domNode,
appearance: { showPointer: true, compact: true },
persistence: { hideOnHover: false, sticky: true },
trapFocus: true,
},
true
);
if (!hover) {
details.dispose();
}
src/vs/workbench/contrib/chat/browser/widgetHosts/viewPane/chatContextUsageWidget.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/chat/browser/widgetHosts/viewPane/chatContextUsageWidget.ts
Show resolved
Hide resolved
…tContextUsageWidget.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Fix #290378
Fix #291063
Fix #291051
Fix #291050