Skip to content

sort default question options to the top#305696

Merged
meganrogge merged 7 commits intomainfrom
merogge/sort-options
Mar 27, 2026
Merged

sort default question options to the top#305696
meganrogge merged 7 commits intomainfrom
merogge/sort-options

Conversation

@meganrogge
Copy link
Copy Markdown
Collaborator

fixes #293080

Copilot AI review requested due to automatic review settings March 27, 2026 15:34
@meganrogge meganrogge self-assigned this Mar 27, 2026
@meganrogge meganrogge added this to the 1.114.0 milestone Mar 27, 2026
@meganrogge meganrogge enabled auto-merge (squash) March 27, 2026 15:34
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

This PR addresses chat question carousel UX by ensuring predefined default option(s) are moved to the top of the option list, so the initially selected/checked answers are always the first visible choices (per #293080).

Changes:

  • Re-sorts singleSelect options so the default option is rendered first.
  • Re-sorts multiSelect options so all default options are rendered first.
  • Updates unit tests to reflect the new visual ordering/selection state.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatQuestionCarouselPart.ts Re-sorts option rendering for single- and multi-select questions to move default option(s) to the top.
src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatQuestionCarouselPart.test.ts Adjusts assertions to expect default option(s) to appear first and be selected/checked.
Comments suppressed due to low confidence (2)

src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatQuestionCarouselPart.ts:1212

  • Same indexing issue as single-select: checkboxes are created in the sorted options order, but getCurrentAnswer() later uses question.options?.[index] (original order) when collecting checked values. After this re-sort, submitted selectedValues will no longer correspond to what the user checked. Fix by keeping the rendered/sorted options array (or a parallel value list) and using it when translating checkbox indices back to option values.
		// Re-sort options so default options appear first
		const options = defaultOptionIds.length > 0
			? [...originalOptions].sort((a, b) => {
				const aIsDefault = defaultOptionIds.includes(a.id);
				const bIsDefault = defaultOptionIds.includes(b.id);
				return aIsDefault === bIsDefault ? 0 : aIsDefault ? -1 : 1;
			})
			: originalOptions;

src/vs/workbench/contrib/chat/test/browser/widget/chatContentParts/chatQuestionCarouselPart.test.ts:395

  • This test checks that default options appear checked after re-sorting, but it doesn’t validate that the collected selectedValues on submit match the checked options (especially important now that UI order no longer matches question.options order). Consider submitting and asserting submittedAnswers.get('q1') contains values for 'a' and 'c' only.
			// Default options 'a' and 'c' are re-sorted to appear first
			const listItems = widget.domNode.querySelectorAll('.chat-question-list-item') as NodeListOf<HTMLElement>;
			assert.strictEqual(listItems[0].classList.contains('checked'), true, 'First default option should be checked');
			assert.strictEqual(listItems[1].classList.contains('checked'), true, 'Second default option should be checked (re-sorted from third)');
			assert.strictEqual(listItems[2].classList.contains('checked'), false, 'Non-default option should not be checked');

…chatQuestionCarouselPart.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

bpasero
bpasero previously approved these changes Mar 27, 2026
…chatQuestionCarouselPart.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@meganrogge meganrogge marked this pull request as draft March 27, 2026 15:54
auto-merge was automatically disabled March 27, 2026 15:54

Pull request was converted to draft

@meganrogge meganrogge marked this pull request as ready for review March 27, 2026 15:59
@meganrogge meganrogge enabled auto-merge (squash) March 27, 2026 16:11
@meganrogge meganrogge merged commit 00515ed into main Mar 27, 2026
18 checks passed
@meganrogge meganrogge deleted the merogge/sort-options branch March 27, 2026 16:54
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.

Chat: Sometimes a question has a predefined selection for multiple options

4 participants