Skip to content

Save Settings text inputs on blur#53036

Merged
ChristopherBiscardi merged 3 commits intozed-industries:mainfrom
ChristopherBiscardi:settings-ui-blurred-save
Apr 7, 2026
Merged

Save Settings text inputs on blur#53036
ChristopherBiscardi merged 3 commits intozed-industries:mainfrom
ChristopherBiscardi:settings-ui-blurred-save

Conversation

@ChristopherBiscardi
Copy link
Copy Markdown
Contributor

Previously, when writing in a Settings input field and tabbing between text inputs, the value the user wrote would be lost. This was particularly an issue when searching for settings meant that a text value was at the end of the settings page, and a tab out of the input would result in cycling to the top and losing a view of the input.

Settings inputs that don't have confirm buttons should confirm when blurring focus, since a user has written something into the text box.

This is complicated by the fact that there is another piece of code here that fires events during render, which is an anti-pattern since it means that events relating to inputs will fire out-of-order (such as: on_blur:confirm -> render:set_state back to original value -> render value from Settings file). However this code's behavior is meant to prevent the override of a user's active field if the settings file is changed by another process. It is unclear to me that this is a good behavior, since a user can lose that data without realizing it, but it was intentionally implemented.

The test for this render logic currently uses the user's written data in the input field for comparison, which isn't actually indicating whether the initial value has changed. So in this PR we store the original initial value, and compare future initial values against it, updating the initial value if necessary.

Previous Behavior

Screen.Recording.2026-04-02.at.6.13.28.PM.mov

This PR

Screen.Recording.2026-04-02.at.6.01.14.PM.mov

Self-Review Checklist:

  • I've reviewed my own diff for quality, security, and reliability
  • Unsafe blocks (if any) have justifying comments
  • The content is consistent with the UI/UX checklist
  • Tests cover the new/changed behavior
  • Performance impact has been considered and is acceptable

Release Notes:

  • Settings text inputs no longer lose user data when blurring focus

ChristopherBiscardi and others added 3 commits April 2, 2026 17:44
Co-authored-by: Anthony Eid <[email protected]>
Previously, when writing in a Settings input field and tabbing between text inputs, the value the user wrote would be lost. This was particularly an issue when searching for settings meant that a text value was at the *end* of the settings page, and a tab out of the input would result in cycling to the top and losing a view of the input.

Settings inputs that don't have confirm buttons should confirm when blurring focus, since a user has written something into the text box.

This is complicated by the fact that there is another piece of code here that fires events during render, which is an anti-pattern since it means that events relating to inputs will fire out-of-order (such as: on_blur:confirm -> render:set_state back to original value -> render value from Settings file). However this code's behavior is meant to prevent the override of a user's active field if the settings file is changed by another process. It is unclear to me that this is a good behavior, since a user can lose that data without realizing it, but it was intentionally implemented.

The test for this render logic currently uses the user's written data in the input field for comparison, which isn't actually indicating whether the initial value has changed. So in this PR we store the *original* initial value, and compare future initial values against it, updating the initial value if necessary.
@cla-bot cla-bot Bot added the cla-signed The user has signed the Contributor License Agreement label Apr 3, 2026
@zed-community-bot zed-community-bot Bot added the staff Pull requests authored by a current member of Zed staff label Apr 3, 2026
Copy link
Copy Markdown
Contributor

@Anthony-Eid Anthony-Eid left a comment

Choose a reason for hiding this comment

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

Great work!

@ChristopherBiscardi ChristopherBiscardi merged commit 0de9b55 into zed-industries:main Apr 7, 2026
74 of 79 checks passed
MasoudAlali pushed a commit to MasoudAlali/zed-ide that referenced this pull request Apr 7, 2026
piper-of-dawn pushed a commit to piper-of-dawn/zed that referenced this pull request Apr 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The user has signed the Contributor License Agreement staff Pull requests authored by a current member of Zed staff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants