Skip to content

Conversation

@chrarnoldus
Copy link
Contributor

@chrarnoldus chrarnoldus commented Aug 11, 2025

Kilo Code issue: Kilo-Org/kilocode#1880

Description

Relevant properties were not set on AutoApproveSettings, causes the value to be saved but not shown.

Test Procedure

Open settings, edit allowedMaxRequests and allowedMaxCost and verify the values stay on-screen when the inputs lose focus.

Get in Touch

Christiaan in shared Slack


Important

Fixes display issue for allowedMaxRequests and allowedMaxCost in settings UI by ensuring proper value assignment in SettingsView.tsx.

  • Behavior:
    • Fixes issue where allowedMaxRequests and allowedMaxCost values were not displayed in the settings UI.
    • Ensures these values are set to undefined if not provided, preventing display issues.
  • Code Changes:
    • Updates SettingsView in SettingsView.tsx to include allowedMaxRequests and allowedMaxCost with fallback to undefined.

This description was created by Ellipsis for 9340758. You can customize this summary. It will automatically update as commits are pushed.

@chrarnoldus chrarnoldus requested review from cte, jr and mrubens as code owners August 11, 2025 07:30
@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. bug Something isn't working labels Aug 11, 2025
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Aug 11, 2025
Copy link
Contributor

@roomote roomote bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution! This is a clean and focused fix that correctly addresses the display issue.

The change properly converts values to using the nullish coalescing operator, which aligns with how the FormattedTextField component expects to handle empty values. The fix is minimal, targeted, and follows the existing patterns in the codebase.

LGTM! 🎯

followupAutoApproveTimeoutMs={followupAutoApproveTimeoutMs}
allowedCommands={allowedCommands}
allowedMaxRequests={allowedMaxRequests ?? undefined}
allowedMaxCost={allowedMaxCost ?? undefined}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good fix! Using here ensures that values are properly converted to , which the FormattedTextField component expects for displaying empty fields. This is consistent with how the component's formatters are designed to work.

chrarnoldus added a commit to Kilo-Org/kilocode that referenced this pull request Aug 11, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Aug 13, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Aug 13, 2025
Copy link
Member

@daniel-lxs daniel-lxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @chrarnoldus

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Aug 14, 2025
@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to PR [Needs Review] in Roo Code Roadmap Aug 14, 2025
@mrubens
Copy link
Collaborator

mrubens commented Aug 14, 2025

Thank you!

@mrubens mrubens merged commit a203a90 into RooCodeInc:main Aug 14, 2025
31 checks passed
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Aug 14, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Aug 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer PR - Needs Review size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants