Move max requests setting from auto-approve menu to settings tab #6489
Conversation
- Remove max requests input field from AutoApproveMenu.tsx - Add max requests setting to AutoApproveSettings.tsx below execute setting - Update component props and types to handle allowedMaxRequests - Ensure setting persists via save button instead of auto-save - Fix linting issues by removing unused imports
dosubot
bot
added
size:M
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| input.value = input.value.replace(/[^0-9]/g, "") | ||
| const value = parseInt(input.value) | ||
| const parsedValue = !isNaN(value) && value > 0 ? value : undefined | ||
| setCachedStateField("allowedMaxRequests", parsedValue) |
There was a problem hiding this comment.
Critical: Missing immediate persistence behavior. The original implementation in AutoApproveMenu.tsx immediately persisted changes via vscode.postMessage. This new implementation only updates the cached state via setCachedStateField, which means the setting won't be saved until the user clicks the Save button. This creates inconsistent behavior compared to how this setting worked before. Should we maintain the immediate persistence behavior for consistency?
| const parsedValue = !isNaN(value) && value > 0 ? value : undefined | ||
| setCachedStateField("allowedMaxRequests", parsedValue) | ||
| }} | ||
| style={{ flex: 1, maxWidth: "200px" }} |
There was a problem hiding this comment.
Inconsistent styling approach: This section mixes Tailwind classes with inline styles. For consistency with the rest of the component, consider using Tailwind classes throughout or following the existing pattern used by other settings sections.
| )} | ||
|
|
||
| {/* Max Requests Setting */} | ||
| <div className="flex flex-col gap-3 mt-6"> |
There was a problem hiding this comment.
Missing visual grouping: Unlike other conditional settings sections (read-only, write, execute), this max requests setting doesn't have the visual grouping with left border that indicates it's part of the auto-approve feature set. This makes it appear disconnected from the other settings. Should this be grouped visually with the other auto-approve settings?
| <div>{t("settings:autoApprove.apiRequestLimit.title")}</div> | ||
| </div> | ||
| <div className="flex items-center gap-2"> | ||
| <VSCodeTextField |
There was a problem hiding this comment.
Consider adding a data-testid attribute for testing consistency with other form elements in the component.
hannesrudolph
added
the
Issue/PR - Triage
|
Closing this for now, will make this move most likely after merging #6484 |
This PR moves the "max requests" setting from the auto-approve dropdown menu in the main chat UI to the auto-approve tab in the settings panel.
Changes Made
AutoApproveMenu.tsx(auto-approve dropdown in chat)AutoApproveSettings.tsxbelow the execute settingallowedMaxRequestsBenefits
Testing
The functionality remains exactly the same - only the location and persistence method have changed.
Important
Move
max requestssetting fromAutoApproveMenu.tsxtoAutoApproveSettings.tsxand change persistence to save-button-based.max requestssetting fromAutoApproveMenu.tsxtoAutoApproveSettings.tsx.SettingsView.tsx.allowedMaxRequestsfromAutoApproveMenu.tsx.allowedMaxRequestsinput field toAutoApproveSettings.tsx.SettingsView.tsxto handleallowedMaxRequestswith save button.AutoApproveMenu.tsx.This description was created by
for dd31868. You can customize this summary. It will automatically update as commits are pushed.