Skip to content

Conversation

@brunobergher
Copy link
Collaborator

@brunobergher brunobergher commented Sep 16, 2025

As it says on the tin. We heard user feedback and are acting on it.

Description

  1. Adds an "Enabled" switch to the auto-approve dropdown, toggling the global auto-approve setting, without changing the toggle selection
  2. Selecting "None" doesn't affect this (aka you can have it enabled and with nothing selected, usually when clearing your selection and picking only a couple)
  3. The UI reacts to this state accordingly

In doing this, I saw that the existing code made it a requirement to have at least one toggle checked in order to enable/disable auto-approve, basically coupling the global setting and the specific toggle selection. I found that counter-intuitive and something which made #2 above impossible, so I lifted that requirement (and updated tests accordingly).

Test Procedure

  1. Open the auto-approve dropdown
  2. Play around with the switch and action toggles
  3. Play around with select all/none
  4. Make sure Roo respects the auto-approve selection

Screenshots / Videos

Off, Closed:
Screenshot 2025-09-16 at 13 25 26

Off, Open:
Screenshot 2025-09-16 at 13 25 32

On, No Selection, Open:
Screenshot 2025-09-16 at 13 25 41

On, Partial Selection, Open:
Screenshot 2025-09-16 at 13 25 48

On, Full Selection, Open:
Screenshot 2025-09-16 at 13 25 54

Off, Partial Selection, Open:
Screenshot 2025-09-16 at 13 26 03

Documentation Updates

Unsure, this might need doc changes in terms of overall behavior.
Most likely for screenshots.


Important

Adds a global "Enabled" switch to AutoApproveDropdown.tsx for pausing auto-approve without losing toggle states, with UI and logic updates.

  • Behavior:
    • Adds "Enabled" switch in AutoApproveDropdown.tsx to toggle global auto-approve without changing toggle selections.
    • Allows "None" selection with auto-approve enabled.
    • UI updates to reflect new state management.
  • Logic:
    • Decouples global auto-approve state from individual toggle selections in useAutoApprovalState.ts.
    • Updates tests in useAutoApprovalState.spec.ts to reflect new logic.
  • Translations:
    • Updates translation files to include new labels for auto-approve states.

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

@brunobergher brunobergher marked this pull request as ready for review September 16, 2025 12:28
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Sep 16, 2025
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Sep 16, 2025
Copy link

@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! I've reviewed the changes and this is a well-implemented feature that effectively addresses user feedback. The code is clean and the logic changes make sense.

Review Findings:

Important Suggestions (Should Consider):

  1. Missing i18n for "Enabled" label - The new "Enabled" label in AutoApproveDropdown.tsx (line 294) is hardcoded in English. This should use the translation system like other UI text for consistency with internationalization.

  2. Potential UX confusion with disabled state - When the main toggle is off, all action buttons are disabled but still clickable (they just don't do anything). Users might wonder why clicking doesn't work rather than understanding they need to enable the main toggle first. Consider adding a tooltip or visual indicator.

  3. Consider extracting the toggle logic - The handleAutoApprovalToggle function is duplicated between AutoApproveDropdown.tsx and AutoApproveSettings.tsx. Would it make sense to extract this into a shared hook to avoid duplication?

Minor Improvements (Nice to Have):

  1. Simplified hook implementation - The useAutoApprovalState hook now simply returns the autoApprovalEnabled value directly without considering toggles. The hasEnabledOptions is still calculated but seems unused in the new implementation. Could we simplify this further or remove unused code?

  2. Test coverage for new UI behavior - While the hook tests were updated correctly, consider adding UI tests for the new enabled switch interaction and the disabled state of buttons when the toggle is off.

  3. Accessibility consideration - The label element with custom click handling might cause confusion. Consider using a div with proper ARIA attributes instead.

Overall, great work on implementing this user-requested feature!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 16, 2025
@mrubens mrubens merged commit 3d45d4a into main Sep 16, 2025
28 of 30 checks passed
@mrubens mrubens deleted the feat/bring-back-auto-toggle branch September 16, 2025 12:59
@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap Sep 16, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Sep 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants