Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates chat tips to better match UX feedback and to make tip state (dismissed/last-shown) follow the user profile across workspaces, while also adding confetti feedback for tip actions and adjusting when in-session tips are shown/hidden.
Changes:
- Updated tip widget styling (border, icon visibility) to better match editor widget/welcome view styles.
- Moved dismissed tip IDs + last-shown tip ID storage from workspace scope to profile scope.
- Added confetti triggering for tip actions and auto-hiding behavior when a second request is sent in a session.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatTipContent.css |
Adjusts tip border styling and makes the lightbulb icon visible in the welcome/getting-started tip; adds positioning/overflow for effects. |
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatTipContentPart.ts |
Adds command execution listener to trigger confetti for tip actions, gated by setting + reduced-motion accessibility. |
src/vs/workbench/contrib/chat/browser/chatTipService.ts |
Persists tip dismissal/rotation state per profile and introduces in-session auto-hide behavior on subsequent requests. |
Comments suppressed due to low confidence (2)
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatTipContentPart.ts:126
_commandService.onDidExecuteCommandlistener can remain active even after the tip is hidden/removed from the DOM (e.g.chatListRendereronly removesdomNodeononDidHidebut doesn’t dispose theChatTipContentPart). This can trigger confetti (and retain listeners) after the tip is no longer visible. Consider clearing_commandListenerand/or disposing the content part when it transitions to hidden (after firingonDidHide).
this._commandListener.value = this._commandService.onDidExecuteCommand(e => {
if (commandSet.has(e.commandId) && this._configurationService.getValue<boolean>('chat.confettiOnThumbsUp') && !this._accessibilityService.isMotionReduced()) {
triggerConfetti(this.domNode);
}
});
} else {
this._commandListener.clear();
}
src/vs/workbench/contrib/chat/browser/chatTipService.ts:597
- New behavior: when a second request arrives, the existing tip is cleared and
onDidDismissTipis fired. There are existing unit tests forChatTipService; please add coverage asserting (1)onDidDismissTipfires when a different requestId arrives after a tip was shown and (2) dismissed/last-tip IDs are stored inStorageScope.PROFILE(not WORKSPACE), since that’s the core behavioral change for #294682/#294700.
// A new request arrived while we already showed a tip, hide the old one
if (this._hasShownRequestTip && this._tipRequestId && this._tipRequestId !== requestId) {
this._shownTip = undefined;
this._tipRequestId = undefined;
this._onDidDismissTip.fire();
return undefined;
}
…chatTipContentPart.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@meganrogge I've opened a new pull request, #294705, to work on those changes. Once the pull request is ready, I'll request review from you. |
…294705) * Initial plan * Update dismissTip documentation to reflect profile-scoped storage Changed "in this workspace" to "in this profile" in the dismissTip() JSDoc comment to accurately reflect that dismissed tips are persisted with StorageScope.PROFILE, not StorageScope.WORKSPACE. Co-authored-by: meganrogge <29464607+meganrogge@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: meganrogge <29464607+meganrogge@users.noreply.github.com>
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @rzhao271Matched files:
|
fixes #294682