-
Notifications
You must be signed in to change notification settings - Fork 37.6k
feat: add skip all button and improve navigation styles in chat questions #290577
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 pull request adds a "Skip All" button to the chat question carousel and enhances the visual styling of radio buttons and checkboxes with custom CSS styling.
Changes:
- Adds a "Skip All" button (when
allowSkipis true) that calls theignore()method to dismiss the carousel without providing answers - Implements custom CSS styling for radio buttons and checkboxes including appearance, checked states, focus, and hover effects
- Restructures the footer layout using a flexbox spacer to separate the Skip All button (left) from navigation buttons (right)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| chatQuestionCarousel.css | Adds comprehensive custom styling for radio/checkbox inputs (appearance, checked states, focus/hover effects) and updates footer layout with align-items and spacer classes |
| chatQuestionCarouselPart.ts | Adds Skip All button conditionally based on allowSkip property, creates spacer element for layout, updates disableAllButtons to handle the new button, and adds comment about hiding back button for single questions |
Comments suppressed due to low confidence (1)
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatQuestionCarouselPart.ts:108
- While the ignore() method has existing test coverage, there are no tests verifying that the Skip All button is rendered when allowSkip is true, or that clicking it correctly triggers the ignore() method. Consider adding tests to verify: 1) Skip All button is rendered when allowSkip is true, 2) Skip All button is not rendered when allowSkip is false, and 3) clicking the Skip All button calls ignore() and submits undefined.
if (this.carousel.allowSkip) {
this._skipAllButton = this._register(new Button(this._navigationButtons, { ...defaultButtonStyles, secondary: true }));
this._skipAllButton.label = localize('skipAll', 'Skip All');
this._skipAllButton.element.classList.add('chat-question-skip-all');
this._register(this._skipAllButton.onDidClick(() => this.ignore()));
}
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatQuestionCarouselPart.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatQuestionCarouselPart.ts
Outdated
Show resolved
Hide resolved
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
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
No description provided.