Skip to content

fix: resolve save button activation in prompts settings (#5780) #6026

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

fix: resolve save button activation in prompts settings (#5780) #6026

wants to merge 3 commits into from

Conversation

@MuriloFP
Copy link
Contributor

@MuriloFP MuriloFP commented Jul 21, 2025 •
edited by ellipsis-dev bot
Loading

Related GitHub Issue

Closes: #5780

Roo Code Task Context (Optional)

No Roo Code task context for this PR

Description

This PR fixes the issue where the Save button in the Prompts settings doesn't activate immediately when editing text. The user had to click outside the input area for the button to become active.

The issue had two parts:

  1. Event handling: The onChange handler was using the logical OR operator (||) which treats empty strings as falsy values, causing incorrect fallback behavior.
  2. Change detection: The PromptsSettings component wasn't properly triggering change detection in the parent SettingsView component for all prompt types.

Key changes:

  • Replaced || with ?? (nullish coalescing) in onChange handlers across PromptsSettings.tsx and ModesView.tsx
  • Updated the CONDENSE prompt handling to also call setCustomSupportPrompts to ensure proper change detection
  • Applied the same fix to the reset handler for consistency

Test Procedure

Automated Testing:

  • Ran all tests for settings components: All 86 tests passed
  • Ran all tests for modes components: All 8 tests passed
  • No regressions introduced

Manual Testing:

  1. Open the Settings view in VSCode
  2. Navigate to the "Prompts" tab
  3. Start typing in any prompt text area
  4. Verify that the Save button activates immediately upon typing (no longer grayed out)
  5. Clear the text field completely
  6. Verify the Save button still responds correctly to empty input
  7. Test with different prompt types (ENHANCE, CONDENSE, etc.)
  8. Verify the reset button also triggers proper change detection

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • 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.
  • Testing: New and/or updated tests have been added to cover my changes (if applicable).
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

No UI changes in this PR (The fix is behavioral - the Save button now activates immediately when typing, but there are no visual changes to the UI itself)

Documentation Updates

  • No documentation updates are required.

Additional Notes

This fix was initially identified by @daniel-lxs in the issue comments. The solution ensures:

  1. Empty strings are properly handled in onChange events
  2. Change detection is triggered for all prompt types
  3. Consistent behavior across all text areas in the application

Get in Touch

Discord username not provided

Important

Fixes Save button activation in Prompts settings by replacing || with ?? in event handlers to handle empty strings correctly.

  • Behavior:
    • Fixes Save button activation in PromptsSettings.tsx and ModesView.tsx by replacing || with ?? in onChange handlers to handle empty strings correctly.
    • Ensures setCustomSupportPrompts is called for CONDENSE prompt type to trigger change detection.
    • Applies the same fix to the reset handler for consistency.
  • Testing:
    • All 86 settings component tests and 8 modes component tests passed.
    • Manual testing confirms immediate Save button activation upon typing and correct response to empty input.

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

@MuriloFP
fix: resolve save button activation in prompts settings (RooCodeInc#5780
f913f9c 
)

- Replace logical OR (||) with nullish coalescing (??) in onChange handlers
- Fixes issue where empty strings were treated as falsy, preventing proper event handling
- Applied fix to PromptsSettings.tsx and 5 similar instances in ModesView.tsx
- Ensures Save button activates immediately when editing prompt text
@MuriloFP MuriloFP requested review from cte, jr and mrubens as code owners July 21, 2025 17:04
@MuriloFP MuriloFP mentioned this pull request Jul 21, 2025
Closed
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working labels Jul 21, 2025
@MuriloFP MuriloFP marked this pull request as draft July 21, 2025 17:08
@MuriloFP
fix: ensure change detection triggers for all prompt types
7eae039 
- Update CONDENSE prompt handling to also call setCustomSupportPrompts
- This ensures the parent SettingsView component detects changes
- Fixes issue where Save button remained disabled when editing prompts
@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Jul 21, 2025
@MuriloFP
fix: remove immediate trim in onChange to ensure proper state updates
358a76b 
- Move trimming logic into updateSupportPrompt function
- This ensures the onChange event triggers immediately on any input
- Empty strings are converted to undefined only when storing
@daniel-lxs daniel-lxs moved this from Triage to PR [Draft / In Progress] in Roo Code Roadmap Jul 22, 2025
@hannesrudolph hannesrudolph added PR - Draft / In Progress and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Jul 22, 2025
@hannesrudolph hannesrudolph moved this from PR [Draft / In Progress] to PR [Needs Prelim Review] in Roo Code Roadmap Sep 23, 2025
@hannesrudolph hannesrudolph marked this pull request as ready for review September 23, 2025 01:49
Copilot AI review requested due to automatic review settings September 23, 2025 01:49
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Sep 23, 2025
Copilot AI reviewed Sep 23, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a bug where the Save button in the Prompts settings doesn't activate immediately when users start typing in text areas. The issue was caused by improper handling of empty strings in onChange events and missing change detection for certain prompt types.

Key Changes

  • Replaced logical OR (||) with nullish coalescing (??) operators in onChange handlers to properly handle empty strings
  • Added proper change detection for CONDENSE prompt type to ensure the Save button activates
  • Applied consistent value trimming and processing logic across all prompt handling

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
webview-ui/src/components/settings/PromptsSettings.tsx Fixed onChange event handling and added change detection for CONDENSE prompts
webview-ui/src/components/modes/ModesView.tsx Updated onChange handlers to use nullish coalescing operator

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

webview-ui/src/components/settings/PromptsSettings.tsx
Comment on lines +59 to +61
// Trim the value when storing, but keep empty strings
const trimmedValue = value?.trim()
const finalValue = trimmedValue === "" ? undefined : trimmedValue
Copy link

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

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

[nitpick] The logic for handling empty strings could be simplified. Consider using const finalValue = value?.trim() || undefined which achieves the same result more concisely.

Suggested change
// Trim the value when storing, but keep empty strings
const trimmedValue = value?.trim()
const finalValue = trimmedValue === "" ? undefined : trimmedValue
// Trim the value when storing, and treat empty strings as undefined
const finalValue = value?.trim() || undefined

Copilot uses AI. Check for mistakes.
webview-ui/src/components/settings/PromptsSettings.tsx
Comment on lines +64 to +67
setCustomCondensingPrompt(finalValue || supportPrompt.default.CONDENSE)
vscode.postMessage({
type: "updateCondensingPrompt",
text: value || supportPrompt.default.CONDENSE,
text: finalValue || supportPrompt.default.CONDENSE,
Copy link

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

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

[nitpick] The fallback to supportPrompt.default.CONDENSE is duplicated. Consider extracting this to a variable to avoid repetition: const promptValue = finalValue || supportPrompt.default.CONDENSE.

Copilot uses AI. Check for mistakes.
webview-ui/src/components/settings/PromptsSettings.tsx
Comment on lines +64 to +67
setCustomCondensingPrompt(finalValue || supportPrompt.default.CONDENSE)
vscode.postMessage({
type: "updateCondensingPrompt",
text: value || supportPrompt.default.CONDENSE,
text: finalValue || supportPrompt.default.CONDENSE,
Copy link

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

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

[nitpick] The fallback to supportPrompt.default.CONDENSE is duplicated. Consider extracting this to a variable to avoid repetition: const promptValue = finalValue || supportPrompt.default.CONDENSE.

Copilot uses AI. Check for mistakes.
daniel-lxs
daniel-lxs reviewed Sep 23, 2025
View reviewed changes
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.

Found issues that need attention.

webview-ui/src/components/settings/PromptsSettings.tsx

const updateSupportPrompt = (type: SupportPromptType, value: string | undefined) => {
// Trim the value when storing, but keep empty strings
const trimmedValue = value?.trim()
Copy link
Member

@daniel-lxs daniel-lxs Sep 23, 2025

Choose a reason for hiding this comment

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

Empty strings are converted to undefined; this can repopulate defaults while clearing. Keep '' during editing and only coerce to undefined on save.

webview-ui/src/components/settings/PromptsSettings.tsx

const updateSupportPrompt = (type: SupportPromptType, value: string | undefined) => {
// Trim the value when storing, but keep empty strings
const trimmedValue = value?.trim()
Copy link
Member

@daniel-lxs daniel-lxs Sep 23, 2025

Choose a reason for hiding this comment

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

Trimming on every keystroke can remove intentional whitespace; trim only on save.

webview-ui/src/components/settings/PromptsSettings.tsx
setCustomSupportPrompts(updatedPrompts)
} else {
const updatedPrompts = { ...customSupportPrompts, [type]: value }
const updatedPrompts = { ...customSupportPrompts, [type]: finalValue }
Copy link
Member

@daniel-lxs daniel-lxs Sep 23, 2025

Choose a reason for hiding this comment

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

Non-CONDENSE branch sets [type]: finalValue even when undefined; delete the key instead to fall back and keep change detection predictable.

webview-ui/src/components/settings/PromptsSettings.tsx

if (type === "CONDENSE") {
setCustomCondensingPrompt(value || supportPrompt.default.CONDENSE)
setCustomCondensingPrompt(finalValue || supportPrompt.default.CONDENSE)
Copy link
Member

@daniel-lxs daniel-lxs Sep 23, 2025

Choose a reason for hiding this comment

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

Use nullish coalescing (??) instead of || for fallback so empty strings don’t revert to defaults.

webview-ui/src/components/modes/ModesView.tsx
(e as unknown as CustomEvent)?.detail?.target?.value ||
(e as unknown as CustomEvent)?.detail?.target?.value ??
((e as any).target as HTMLTextAreaElement).value
setCustomInstructions(value || undefined)
Copy link
Member

@daniel-lxs daniel-lxs Sep 23, 2025

Choose a reason for hiding this comment

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

setCustomInstructions uses value || undefined; use value ?? undefined to preserve empty strings.

@daniel-lxs
Copy link
Member

daniel-lxs commented Sep 23, 2025

@roomote-agent Create a new PR based on this branch and incorporate the review comments left on this one, I'll close this PR but I won't delete the branch just yet so you can get the changes.

@daniel-lxs daniel-lxs closed this Sep 23, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Sep 23, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Prelim Review] to Done in Roo Code Roadmap Sep 23, 2025
@roomote
Copy link
Contributor

roomote bot commented Sep 23, 2025

I've seen your request @daniel-lxs. I'll create a new PR based on the branch 'fix/issue-5780-save-button-activation' and incorporate the review comments. Working on it now.

roomote bot pushed a commit that referenced this pull request Sep 23, 2025
@roomote
fix: improve save button activation in prompts settings
61aed25 
- Use nullish coalescing (??) instead of logical OR (||) to preserve empty strings
- Remove trimming during editing to preserve intentional whitespace
- Delete keys when undefined instead of setting them to undefined
- Properly handle empty strings vs undefined values for better change detection

Addresses review comments from PR #6026
@roomote roomote bot mentioned this pull request Sep 23, 2025
Merged
5 tasks
@roomote
Copy link
Contributor

roomote bot commented Sep 23, 2025

✅ Task completed successfully!

I've created a new PR #8267 based on your branch that incorporates all the review comments:
#8267

Changes made based on review feedback:

  • ✅ Replaced logical OR (||) with nullish coalescing (??) to preserve empty strings
  • ✅ Removed trimming during editing to preserve intentional whitespace
  • ✅ Delete keys when undefined instead of setting them to undefined for better change detection
  • ✅ Updated tests to match the new behavior

CI Status: All checks have passed ✅

  • Unit tests (Ubuntu & Windows): Passed
  • Integration tests: Passed
  • Type checking: Passed
  • Linting: Passed

The new PR is ready for review. Thanks for the initial work on this fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@daniel-lxs daniel-lxs daniel-lxs left review comments

Copilot code review Copilot Copilot left review comments

@mrubens mrubens Awaiting requested review from mrubens mrubens is a code owner

@cte cte Awaiting requested review from cte cte is a code owner

@jr jr Awaiting requested review from jr jr is a code owner

Assignees

@daniel-lxs daniel-lxs

Labels

bug Something isn't working PR - Needs Preliminary Review size:M This PR changes 30-99 lines, ignoring generated files.

Projects

Roo Code Roadmap
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] Prompts editor: Save button doesn’t activate on edit; requires two clicks to persist

3 participants

@MuriloFP @daniel-lxs @hannesrudolph