-
Notifications
You must be signed in to change notification settings - Fork 2.9k
feat: enhance ask_followup_question to support multiple questions #11139
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
base: main
Are you sure you want to change the base?
Conversation
- Separate navigation logic from submission logic in MultiQuestionHandler - Add dedicated handleFinish function to send combined responses - Prevent immediate sending when navigating between questions - Add comprehensive tests to verify the UX fix - All existing functionality preserved, tests passing
Reviewed 5c5811c (Indonesian translation fix). The Vietnamese word in the Indonesian locale is now corrected. One previously flagged issue remains unresolved.
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
| <SearchableSetting | ||
| settingId="ui-show-questions-one-by-one" | ||
| section="ui" | ||
| label={t("settings:ui.showQuestionsOneByOne.label")}> | ||
| <div className="flex flex-col gap-1"> | ||
| <VSCodeCheckbox | ||
| checked={showQuestionsOneByOne} | ||
| onChange={(e: any) => handleShowQuestionsOneByOneChange(e.target.checked)} | ||
| data-testid="show-questions-one-by-one-checkbox"> | ||
| <span className="font-medium">{t("settings:ui.showQuestionsOneByOne.label")}</span> | ||
| </VSCodeCheckbox> | ||
| <div className="text-vscode-descriptionForeground text-sm ml-5 mt-1"> | ||
| {t("settings:ui.showQuestionsOneByOne.description")} | ||
| </div> | ||
| </div> | ||
| </SearchableSetting> |
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.
Missing translation keys. The settings:ui.showQuestionsOneByOne.label and settings:ui.showQuestionsOneByOne.description keys are referenced here but not defined in webview-ui/src/i18n/locales/en/settings.json. The UI section in that file only has collapseThinking and requireCtrlEnterToSend. This will cause the raw translation keys to be displayed in the UI instead of human-readable text.
Fix it with Roo Code or mention @roomote and request a fix.
| | "enterBehavior" | ||
| | "includeCurrentTime" | ||
| | "includeCurrentCost" | ||
| | "showQuestionsOneByOne" |
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.
showQuestionsOneByOne is added to ExtensionState here, but ClineProvider.ts never includes it in the state objects returned by getState() or getStateToPostToWebview(). Every other UI setting in this Pick type (e.g., enterBehavior, includeCurrentCost, taskHeaderHighlightEnabled) is explicitly included in both methods. Without the same wiring, the setting value stored via contextProxy.setValue is never sent back to the webview -- it will always default to false on initial load and after webview reloads, and the ExtensionStateContext check at line 343 ((newState as any).showQuestionsOneByOne) will never trigger. You need to add showQuestionsOneByOne to the destructured values from getState() (~line 2082), and include showQuestionsOneByOne: showQuestionsOneByOne ?? false in both state return objects (near lines 2228 and 2470).
Fix it with Roo Code or mention @roomote and request a fix.
| <SearchableSetting | ||
| settingId="ui-task-header-highlight" | ||
| section="ui" | ||
| label={t("settings:ui.taskHeaderHighlight.label")}> | ||
| <div className="flex flex-col gap-1"> | ||
| <VSCodeCheckbox | ||
| checked={taskHeaderHighlightEnabled} | ||
| onChange={(e: any) => handleTaskHeaderHighlightChange(e.target.checked)} | ||
| data-testid="task-header-highlight-checkbox"> | ||
| <span className="font-medium">{t("settings:ui.taskHeaderHighlight.label")}</span> | ||
| </VSCodeCheckbox> | ||
| <div className="text-vscode-descriptionForeground text-sm ml-5 mt-1"> | ||
| {t("settings:ui.taskHeaderHighlight.description")} | ||
| </div> | ||
| </div> | ||
| </SearchableSetting> |
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.
The translation keys settings:ui.taskHeaderHighlight.label and settings:ui.taskHeaderHighlight.description referenced here don't exist in any locale's settings.json. The old taskCompleteColor keys were removed during the merge with main (which reverted the highlight feature in #11349), but the replacement taskHeaderHighlight keys were never added. Users will see raw key strings like settings:ui.taskHeaderHighlight.label in the settings UI. Either add the translation keys to all locale files, or remove this setting block entirely since the highlight feature it controls was reverted on main and TaskHeader.tsx no longer reads taskHeaderHighlightEnabled.
Fix it with Roo Code or mention @roomote and request a fix.
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.
@roomote fix it
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.
Fixaroo
See task
Removed the orphaned taskHeaderHighlight setting block, handler, and prop from UISettings since the feature was reverted on main and the translation keys were never added. All lint, type checks, and tests pass.
| enterBehavior: enterBehavior ?? "send", | ||
| includeCurrentTime: includeCurrentTime ?? true, | ||
| includeCurrentCost: includeCurrentCost ?? true, | ||
| showQuestionsOneByOne: showQuestionsOneByOne ?? false, |
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.
taskHeaderHighlightEnabled was removed from this save payload during the merge resolution (the diff shows -taskHeaderHighlightEnabled,), but the setting toggle was kept in UISettings. This means the checkbox renders in the UI but toggling it and clicking Save has no effect -- the value is never sent to the extension host. If the setting should be preserved, add taskHeaderHighlightEnabled: taskHeaderHighlightEnabled ?? false, here alongside the other UI settings. If the intent is to remove it (since the highlight was reverted on main), remove the toggle from UISettings.tsx as well.
Fix it with Roo Code or mention @roomote and request a fix.
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.
@roomote fix
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.
Fixaroo
See task
Removed the orphaned taskHeaderHighlightEnabled setting that leaked back in during the merge resolution (the feature was reverted on main). Cleaned up all 7 affected files. All local checks passed (lint, type-check, tests).
Co-authored-by: roomote[bot] <219738659+roomote[bot]@users.noreply.github.com>
This PR enhances the
ask_followup_questiontool to support multiple questions and optional multiple-choice responses.Key changes:
ask_followup_questionschema to accept aquestionsarray.Questionobjects withtextandoptions.Important
Enhance
ask_followup_questionto support multiple questions and multiple-choice responses, with updated schema, UI, and tests.ask_followup_questionnow supports multiple questions and multiple-choice options.ask_followup_questionschema inask_followup_question.tsto acceptquestionsarray.MultiQuestionHandlerinChatRow.tsxto handle multiple questions.showQuestionsOneByOneinUISettings.tsxto toggle question display mode.AskFollowupQuestionTool.tsto show first question while accumulating others.MultiQuestionHandlerinMultiQuestionHandler.spec.tsx.askFollowupQuestionTool.spec.tsto cover new functionality.showQuestionsOneByOnesetting toExtensionStateContext.tsxandSettingsView.tsx.chat.jsonfor new question handling.This description was created by
for d3028c3. You can customize this summary. It will automatically update as commits are pushed.