Skip to content
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.

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

fix: Cancel button in Chat settings dialog not working properly #1897

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

hexart
Copy link
Contributor

@hexart hexart commented Feb 14, 2025

Issue

Cancel button behavior:
Previously, clicking the "Cancel" button would incorrectly save the settings changes, behaving the same as the "Confirm" button. This was not the expected behavior.
Console warning:

[Warning] Warning: Missing `Description` or `aria-describedby={undefined}` for {DialogContent}. (chunk-WOIZ637Z.js, line 340)

Changes Made

  • Fixed the Cancel button behavior to revert to the last saved settings instead of saving the changes
  • Added setChatSettingsValue to maintain local state
  • Ensured consistent behavior when closing the dialog through different methods (Cancel button, X button, clicking outside)
  • add DialogDescription to resolve accessibility warning

Technical Details

  1. Imported required Recoil state management tools
  2. Added proper handling of chatSettingsValue
  3. Updated handleClose function to reset to last saved values (not default values)
  4. Ensured handleConfirm synchronizes local state when saving settings

Testing procedure

  1. Modify settings and save, verify new values are correctly saved
  2. Modify any settings without saving, close dialog through different methods (Cancel, X, click outside), verify it reverts to last saved values
  3. Reset settings to default values, click cancel, verify it reverts to last saved values
  4. Reset settings to default values again and save, verify all revert to default values

1. Added reset to the last saved value when clicking cancel or X button
2. Added local state update when settings confirmed
3. Fixed bug where settings were not restored correctly after cancellation
4. Fixed chat settings value sync between frontend and backend
- Changed Cancel button to use handleClose instead of direct setChatSettingsOpen
- Unified all dialog closing behaviors (Cancel button, X button, clicking outside) to properly reset form values

This ensures consistent behavior when closing the dialog in any way, preventing issues with unsaved reset values.
@hexart hexart marked this pull request as ready for review February 14, 2025 17:51
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. frontend Pertains to the frontend. labels Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend Pertains to the frontend. size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant