From 16466ea9366782614fea266dfd1aae38e2a5a9be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Soares?= <37777652+Dnreikronos@users.noreply.github.com> Date: Thu, 23 Apr 2026 08:36:03 -0300 Subject: [PATCH] theme_selector: Revert theme when modal is dismissed by clicking outside (#52773) Context Related to #52118 Found this while looking into the theme selector filtering bug: clicking outside the theme selector (or icon theme selector) keeps the previewed theme applied instead of reverting. `hide_modal()` doesn't go through the Picker's `dismissed()` callback, so the in-memory theme override sticks around without being written to `settings.json`. This PR only fixes the click-outside behavior. The filtering issue from #52118 is separate. ## Demo **Before (bug):** https://github.com/user-attachments/assets/f8be49c4-7e1d-483c-bf23-346022ac83aa **After (fix):** https://github.com/user-attachments/assets/29cc1740-c869-42d1-8aee-fe46a091a949 ## How to review Two files, symmetrical changes. Look at: - `on_before_dismiss` in `theme_selector.rs` and `icon_theme_selector.rs` - The extracted `revert_theme()` method on both delegates, now shared between `dismissed()` and `on_before_dismiss` ## Self-Review Checklist - [x] I've reviewed my own diff for quality, security, and reliability - [ ] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [ ] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed theme selector keeping the previewed theme when clicking outside instead of reverting to the original. --- .../theme_selector/src/icon_theme_selector.rs | 25 ++++++++++++---- crates/theme_selector/src/theme_selector.rs | 29 ++++++++++++++----- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/crates/theme_selector/src/icon_theme_selector.rs b/crates/theme_selector/src/icon_theme_selector.rs index be8e5e01d0a..725881f8cc1 100644 --- a/crates/theme_selector/src/icon_theme_selector.rs +++ b/crates/theme_selector/src/icon_theme_selector.rs @@ -26,7 +26,18 @@ impl Focusable for IconThemeSelector { } } -impl ModalView for IconThemeSelector {} +impl ModalView for IconThemeSelector { + fn on_before_dismiss( + &mut self, + _window: &mut Window, + cx: &mut Context, + ) -> workspace::DismissDecision { + self.picker.update(cx, |picker, cx| { + picker.delegate.revert_theme(cx); + }); + workspace::DismissDecision::Dismiss(true) + } +} impl IconThemeSelector { pub fn new( @@ -132,6 +143,13 @@ impl IconThemeSelectorDelegate { .unwrap_or(self.selected_index); } + fn revert_theme(&mut self, cx: &mut App) { + if !self.selection_completed { + Self::set_icon_theme(self.original_theme.clone(), cx); + self.selection_completed = true; + } + } + fn set_icon_theme(name: IconThemeName, cx: &mut App) { SettingsStore::update_global(cx, |store, _| { let mut theme_settings = store.get::(None).clone(); @@ -185,10 +203,7 @@ impl PickerDelegate for IconThemeSelectorDelegate { } fn dismissed(&mut self, _: &mut Window, cx: &mut Context>) { - if !self.selection_completed { - Self::set_icon_theme(self.original_theme.clone(), cx); - self.selection_completed = true; - } + self.revert_theme(cx); self.selector .update(cx, |_, cx| cx.emit(DismissEvent)) diff --git a/crates/theme_selector/src/theme_selector.rs b/crates/theme_selector/src/theme_selector.rs index 8a0e835fa86..c60218003df 100644 --- a/crates/theme_selector/src/theme_selector.rs +++ b/crates/theme_selector/src/theme_selector.rs @@ -79,7 +79,18 @@ fn toggle_icon_theme_selector( }); } -impl ModalView for ThemeSelector {} +impl ModalView for ThemeSelector { + fn on_before_dismiss( + &mut self, + _window: &mut Window, + cx: &mut Context, + ) -> workspace::DismissDecision { + self.picker.update(cx, |picker, cx| { + picker.delegate.revert_theme(cx); + }); + workspace::DismissDecision::Dismiss(true) + } +} struct ThemeSelector { picker: Entity>, @@ -215,6 +226,15 @@ impl ThemeSelectorDelegate { } } + fn revert_theme(&mut self, cx: &mut App) { + if !self.selection_completed { + SettingsStore::update_global(cx, |store, _| { + store.override_global(self.original_theme_settings.clone()); + }); + self.selection_completed = true; + } + } + fn set_theme(&mut self, new_theme: Arc, cx: &mut App) { // Update the global (in-memory) theme settings. SettingsStore::update_global(cx, |store, _| { @@ -371,12 +391,7 @@ impl PickerDelegate for ThemeSelectorDelegate { } fn dismissed(&mut self, _: &mut Window, cx: &mut Context>) { - if !self.selection_completed { - SettingsStore::update_global(cx, |store, _| { - store.override_global(self.original_theme_settings.clone()); - }); - self.selection_completed = true; - } + self.revert_theme(cx); self.selector .update(cx, |_, cx| cx.emit(DismissEvent))