Skip to content

Conversation

@Mnehmos
Copy link
Contributor

@Mnehmos Mnehmos commented Jun 11, 2025

Here's the filled-out PR template for this fix:

PR Template - Auto-Approve Checkbox State Synchronization Fix

Related GitHub Issue

Closes: #2579

Description

This PR implements a minimal fix for the auto-approve checkbox state synchronization issue in the chat input menu. The main problem was that the checkbox didn't accurately reflect whether any individual auto-approve actions were enabled.

Key Implementation Details:

  • Added hasAnyAutoApprovedAction computed value that tracks if any individual toggle is enabled
  • Updated the main checkbox's checked prop to use this computed value instead of the global autoApprovalEnabled state
  • Added master toggle functionality: clicking the main checkbox now enables/disables ALL individual auto-approve settings
  • Removed unused autoApprovalEnabled variable to resolve ESLint warnings
  • Maintained all existing functionality while fixing the visual state inconsistency

Design Choice: Used a minimal approach that only modifies the existing component without introducing new hooks, state management, or performance optimizations, keeping the change scope focused and reducing risk.

Test Procedure

Manual Testing Steps:

  1. Open Roo Code and navigate to the chat interface
  2. Click on the auto-approve menu to expand it
  3. Test the main checkbox behavior:
    • When no individual toggles are enabled → main checkbox should be unchecked
    • Enable one or more individual toggles → main checkbox should become checked
    • Click main checkbox when unchecked → should enable ALL individual toggles
    • Click main checkbox when checked → should disable ALL individual toggles
  4. Verify visual state consistency matches actual toggle states

Automated Testing:

  • All existing tests pass (pnpm test - 418 passing, 0 failing)
  • ESLint passes with no warnings (pnpm lint)
  • TypeScript compilation successful

Type of Change

  • 🐛 Bug Fix: Non-breaking change that fixes an issue.
  • New Feature: Non-breaking change that adds functionality.
  • 💥 Breaking Change: Fix or feature that would cause existing functionality to not work as expected.
  • ♻️ Refactor: Code change that neither fixes a bug nor adds a feature.
  • 💅 Style: Changes that do not affect the meaning of the code (white-space, formatting, etc.).
  • 📚 Documentation: Updates to documentation files.
  • ⚙️ Build/CI: Changes to the build process or CI configuration.
  • 🧹 Chore: Other changes that don't modify src or test files.

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (Redesign Auto-approve; don't allow confusing checkbox status with labels #2579).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Code Quality:
    • My code adheres to the project's style guidelines.
    • There are no new linting errors or warnings (pnpm lint passes).
    • All debug code (e.g., console.log) has been removed.
  • Testing:
    • New and/or updated tests have been added to cover my changes.
    • All tests pass locally (pnpm test - 418 passing).
    • The application builds successfully with my changes.
  • Branch Hygiene: My branch is up-to-date (rebased) with the main branch.
  • Documentation Impact: I have considered if my changes require documentation updates.
  • Changeset: A changeset has been created using npm run changeset if this PR includes user-facing changes or dependency updates.
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

Before: The main auto-approve checkbox would show inconsistent state - it might be checked even when no individual auto-approve actions were enabled, causing user confusion.
image
image

After: The checkbox now accurately reflects the state:

  • ✅ Unchecked when no individual toggles are enabled
  • ✅ Checked when one or more individual toggles are enabled
  • ✅ Clicking it toggles ALL individual settings as a master control

image
https://youtu.be/5VbPbGeszSk

Documentation Updates

  • No documentation updates are required.

This is a bug fix that doesn't change the intended behavior or add new features, so existing documentation remains accurate.

Additional Notes

  • This fix maintains backward compatibility
  • No breaking changes to existing auto-approve functionality
  • The change is isolated to a single component (AutoApproveMenu.tsx)
  • Performance impact is minimal (adds one computed boolean)
  • Previous complex implementation was intentionally replaced with this simpler, more maintainable solution

Get in Touch

Mnehmos


Important

Fixes checkbox state synchronization in AutoApproveMenu.tsx by using a computed value and adding master toggle functionality.

  • Behavior:
    • hasAnyAutoApprovedAction computed value added to track if any auto-approve toggles are enabled in AutoApproveMenu.tsx.
    • Main checkbox checked prop updated to use hasAnyAutoApprovedAction.
    • Main checkbox now toggles all individual auto-approve settings.
  • Code Cleanup:
    • Removed unused autoApprovalEnabled variable from AutoApproveMenu.tsx.

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

Fixes RooCodeInc#2579

- Add hasAnyAutoApprovedAction to track if any individual auto-approve setting is enabled
- Update main checkbox to reflect actual state of individual toggles
- Add master toggle functionality to enable/disable all auto-approve options
- Remove unused autoApprovalEnabled variable to fix ESLint warning
@Mnehmos Mnehmos requested review from cte, jr and mrubens as code owners June 11, 2025 19:13
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working labels Jun 11, 2025
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jun 11, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jun 11, 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 Jun 12, 2025
@Mnehmos Mnehmos closed this Jun 12, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Prelim Review] to Done in Roo Code Roadmap Jun 12, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 12, 2025
@Mnehmos Mnehmos deleted the fix/2579-auto-approve-checkbox-state branch June 12, 2025 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working PR - Needs Preliminary Review size:S This PR changes 10-29 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Redesign Auto-approve; don't allow confusing checkbox status with labels

2 participants