improve question carousel submit button flow#298360
Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts the Chat Question Carousel navigation so the > button remains purely navigational and can’t become an implicit “Submit” action on the last question (addressing #292404).
Changes:
- Added a dedicated
Submitbutton that is only displayed/enabled on the last question. - Disabled the
>(“Next”) navigation button on the last question while keeping its icon/label stable. - Updated carousel CSS and added/updated tests to validate the new behavior.
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/chatQuestionCarouselPart.ts | Adds a dedicated submit button and updates navigation/hover behavior for last-question handling. |
| src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatQuestionCarousel.css | Styles the new submit button using primary button theming. |
| src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatQuestionCarouselPart.test.ts | Updates navigation tests and adds coverage for presence/visibility of the submit button. |
| const nextButton = widget.domNode.querySelector('.chat-question-nav-next') as HTMLButtonElement; | ||
| assert.ok(nextButton, 'Next button should exist'); | ||
| assert.strictEqual(nextButton.getAttribute('aria-label'), 'Submit', 'Next button should have Submit aria-label on last question'); | ||
| assert.strictEqual(nextButton.getAttribute('aria-label'), 'Next', 'Next button should preserve Next aria-label on last question'); |
There was a problem hiding this comment.
This assertion hard-codes the exact aria-label value as Next, but the implementation intentionally appends a keybinding when available via getLabelWithKeybinding(...). To avoid a brittle test (e.g., if a keybinding for the action ID is added later), assert that the label starts with/contains Next rather than strict equality.
| assert.strictEqual(nextButton.getAttribute('aria-label'), 'Next', 'Next button should preserve Next aria-label on last question'); | |
| const nextAriaLabel = nextButton.getAttribute('aria-label'); | |
| assert.ok(nextAriaLabel && nextAriaLabel.startsWith('Next'), 'Next button aria-label should start with "Next" on last question'); |
| background: var(--vscode-button-background) !important; | ||
| color: var(--vscode-button-foreground) !important; | ||
| width: auto; | ||
| height: 22px; |
There was a problem hiding this comment.
Does this react to chat font size?
|
@copilot open a new pull request to apply changes based on the comments in this thread |
Fixes #292404
Kept
>as navigation only and stopped it from becomingSubmiton the last page.Submitbutton shown only on the last question.>on the last question.Result: users can no longer accidentally submit by repeatedly clicking
>; submission is explicit.submit.mov