From b440a51675deb1bd19bb62c6b260696c93d625d7 Mon Sep 17 00:00:00 2001 From: Keith Simmons Date: Tue, 8 Mar 2022 18:42:31 -0800 Subject: [PATCH 1/4] Make theme selector eagerly display the selected theme --- crates/outline/src/outline.rs | 2 +- crates/theme_selector/src/theme_selector.rs | 80 +++++++++++++++++---- crates/workspace/src/workspace.rs | 14 ++-- 3 files changed, 78 insertions(+), 18 deletions(-) diff --git a/crates/outline/src/outline.rs b/crates/outline/src/outline.rs index 3607f1af87b4b95fb0222ecaad0c35d4a49c2010..dc8ba002a38e9490e2e8980a42a24883277b92d3 100644 --- a/crates/outline/src/outline.rs +++ b/crates/outline/src/outline.rs @@ -144,7 +144,7 @@ impl OutlineView { let view = cx.add_view(|cx| OutlineView::new(outline, editor, settings, cx)); cx.subscribe(&view, Self::on_event).detach(); view - }) + }); } } } diff --git a/crates/theme_selector/src/theme_selector.rs b/crates/theme_selector/src/theme_selector.rs index 9ac35339abe5b1951e85899fbf4e240059092a15..8ea6b577d4978cb4355dda1e4905c26568d6798b 100644 --- a/crates/theme_selector/src/theme_selector.rs +++ b/crates/theme_selector/src/theme_selector.rs @@ -10,7 +10,7 @@ use gpui::{ use parking_lot::Mutex; use postage::watch; use std::{cmp, sync::Arc}; -use theme::ThemeRegistry; +use theme::{ThemeRegistry, Theme}; use workspace::{ menu::{Confirm, SelectNext, SelectPrev}, AppState, Settings, Workspace, @@ -31,6 +31,7 @@ pub struct ThemeSelector { query_editor: ViewHandle, list_state: UniformListState, selected_index: usize, + original_theme: Arc, } action!(Toggle, ThemeSelectorParams); @@ -72,6 +73,8 @@ impl ThemeSelector { cx.subscribe(&query_editor, Self::on_query_editor_event) .detach(); + let original_theme = settings.borrow().theme.clone(); + let mut this = Self { settings, settings_tx, @@ -79,14 +82,19 @@ impl ThemeSelector { query_editor, matches: Vec::new(), list_state: Default::default(), - selected_index: 0, + selected_index: 0, // Default index for now + original_theme, }; this.update_matches(cx); + + // Set selected index to current theme + this.select_if_matching(this.original_theme.name.clone()); + this } fn toggle(workspace: &mut Workspace, action: &Toggle, cx: &mut ViewContext) { - workspace.toggle_modal(cx, |cx, _| { + let possible_closed_theme_selector = workspace.toggle_modal(cx, |cx, _| { let selector = cx.add_view(|cx| { Self::new( action.0.settings_tx.clone(), @@ -98,6 +106,14 @@ impl ThemeSelector { cx.subscribe(&selector, Self::on_event).detach(); selector }); + + if let Some(closed_theme_selector) = possible_closed_theme_selector { + let theme_selector = closed_theme_selector.read(cx); + + theme_selector.settings_tx.lock().borrow_mut().theme = + theme_selector.original_theme.clone(); + cx.refresh_windows(); + } } fn reload(_: &mut Workspace, action: &Reload, cx: &mut ViewContext) { @@ -116,15 +132,8 @@ impl ThemeSelector { } fn confirm(&mut self, _: &Confirm, cx: &mut ViewContext) { - if let Some(mat) = self.matches.get(self.selected_index) { - match self.themes.get(&mat.string) { - Ok(theme) => { - self.settings_tx.lock().borrow_mut().theme = theme; - cx.refresh_windows(); - cx.emit(Event::Dismissed); - } - Err(error) => log::error!("error loading theme {}: {}", mat.string, error), - } + if self.show_selected_theme(cx) { + cx.emit(Event::Dismissed); } } @@ -134,6 +143,8 @@ impl ThemeSelector { } self.list_state .scroll_to(ScrollTarget::Show(self.selected_index)); + + self.show_selected_theme(cx); cx.notify(); } @@ -143,9 +154,38 @@ impl ThemeSelector { } self.list_state .scroll_to(ScrollTarget::Show(self.selected_index)); + + self.show_selected_theme(cx); cx.notify(); } + fn show_selected_theme(&mut self, cx: &mut MutableAppContext) -> bool { + if let Some(mat) = self.matches.get(self.selected_index) { + match self.themes.get(&mat.string) { + Ok(theme) => { + self.settings_tx.lock().borrow_mut().theme = theme; + cx.refresh_windows(); + return true; + } + Err(error) => { + log::error!("error loading theme {}: {}", mat.string, error) + }, + } + } + + return false; + } + + fn select_if_matching(&mut self, theme_name: String) { + self.selected_index = self.matches.iter() + .position(|mat| mat.string == theme_name) + .unwrap_or(self.selected_index); + } + + fn selected_theme_name(&self) -> Option { + self.matches.get(self.selected_index).map(|mat| mat.string.clone()) + } + fn update_matches(&mut self, cx: &mut ViewContext) { let background = cx.background().clone(); let candidates = self @@ -181,6 +221,11 @@ impl ThemeSelector { background, )) }; + + self.selected_index = self.selected_index + .min(self.matches.len() - 1) + .max(0); + cx.notify(); } @@ -204,7 +249,16 @@ impl ThemeSelector { cx: &mut ViewContext, ) { match event { - editor::Event::Edited => self.update_matches(cx), + editor::Event::Edited => { + let previous_selection = self.selected_theme_name(); + self.update_matches(cx); + if let Some(previous_selection) = previous_selection { + self.select_if_matching(previous_selection); + } + + self.show_selected_theme(cx); + + }, editor::Event::Blurred => cx.emit(Event::Dismissed), _ => {} } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index dee138b0aeec0ca2243c5146f08057f377e1921c..4e057fa6bc47086c507f8e79d7e1168da71f6f17 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -754,20 +754,26 @@ impl Workspace { }) } - pub fn toggle_modal(&mut self, cx: &mut ViewContext, add_view: F) + // Returns the model that was toggled closed if it was open + pub fn toggle_modal(&mut self, cx: &mut ViewContext, add_view: F) -> Option> where V: 'static + View, F: FnOnce(&mut ViewContext, &mut Self) -> ViewHandle, { - if self.modal.as_ref().map_or(false, |modal| modal.is::()) { - self.modal.take(); + cx.notify(); + // Whatever modal was visible is getting clobbered. If its the same type as V, then return + // it. Otherwise, create a new modal and set it as active. + let already_open_modal = self.modal.take() + .and_then(|modal| modal.downcast::()); + if let Some(already_open_modal) = already_open_modal { cx.focus_self(); + Some(already_open_modal) } else { let modal = add_view(cx, self); cx.focus(&modal); self.modal = Some(modal.into()); + None } - cx.notify(); } pub fn modal(&self) -> Option<&AnyViewHandle> { From 99e34db0ecca8e753c03a0672aa9ca5806eacc66 Mon Sep 17 00:00:00 2001 From: Keith Simmons Date: Wed, 9 Mar 2022 10:34:52 -0800 Subject: [PATCH 2/4] ensure that we set original theme when dismissing theme selector and fix some minor edge cases --- crates/theme_selector/src/theme_selector.rs | 68 ++++++++++----------- 1 file changed, 32 insertions(+), 36 deletions(-) diff --git a/crates/theme_selector/src/theme_selector.rs b/crates/theme_selector/src/theme_selector.rs index 8ea6b577d4978cb4355dda1e4905c26568d6798b..1f334a860906d63825ed64984237063b806e2db5 100644 --- a/crates/theme_selector/src/theme_selector.rs +++ b/crates/theme_selector/src/theme_selector.rs @@ -10,7 +10,7 @@ use gpui::{ use parking_lot::Mutex; use postage::watch; use std::{cmp, sync::Arc}; -use theme::{ThemeRegistry, Theme}; +use theme::{Theme, ThemeRegistry}; use workspace::{ menu::{Confirm, SelectNext, SelectPrev}, AppState, Settings, Workspace, @@ -32,6 +32,7 @@ pub struct ThemeSelector { list_state: UniformListState, selected_index: usize, original_theme: Arc, + selection_completed: bool, } action!(Toggle, ThemeSelectorParams); @@ -83,18 +84,19 @@ impl ThemeSelector { matches: Vec::new(), list_state: Default::default(), selected_index: 0, // Default index for now - original_theme, + original_theme: original_theme.clone(), + selection_completed: false, }; this.update_matches(cx); // Set selected index to current theme - this.select_if_matching(this.original_theme.name.clone()); + this.select_if_matching(&original_theme.name); this } fn toggle(workspace: &mut Workspace, action: &Toggle, cx: &mut ViewContext) { - let possible_closed_theme_selector = workspace.toggle_modal(cx, |cx, _| { + workspace.toggle_modal(cx, |cx, _| { let selector = cx.add_view(|cx| { Self::new( action.0.settings_tx.clone(), @@ -106,14 +108,6 @@ impl ThemeSelector { cx.subscribe(&selector, Self::on_event).detach(); selector }); - - if let Some(closed_theme_selector) = possible_closed_theme_selector { - let theme_selector = closed_theme_selector.read(cx); - - theme_selector.settings_tx.lock().borrow_mut().theme = - theme_selector.original_theme.clone(); - cx.refresh_windows(); - } } fn reload(_: &mut Workspace, action: &Reload, cx: &mut ViewContext) { @@ -121,8 +115,8 @@ impl ThemeSelector { action.0.themes.clear(); match action.0.themes.get(¤t_theme_name) { Ok(theme) => { - cx.refresh_windows(); action.0.settings_tx.lock().borrow_mut().theme = theme; + cx.refresh_windows(); log::info!("reloaded theme {}", current_theme_name); } Err(error) => { @@ -132,9 +126,8 @@ impl ThemeSelector { } fn confirm(&mut self, _: &Confirm, cx: &mut ViewContext) { - if self.show_selected_theme(cx) { - cx.emit(Event::Dismissed); - } + self.selection_completed = true; + cx.emit(Event::Dismissed); } fn select_prev(&mut self, _: &SelectPrev, cx: &mut ViewContext) { @@ -159,31 +152,34 @@ impl ThemeSelector { cx.notify(); } - fn show_selected_theme(&mut self, cx: &mut MutableAppContext) -> bool { + fn show_selected_theme(&mut self, cx: &mut MutableAppContext) { if let Some(mat) = self.matches.get(self.selected_index) { match self.themes.get(&mat.string) { Ok(theme) => { - self.settings_tx.lock().borrow_mut().theme = theme; - cx.refresh_windows(); - return true; + self.set_theme(theme, cx); } Err(error) => { log::error!("error loading theme {}: {}", mat.string, error) - }, + } } } - - return false; } - fn select_if_matching(&mut self, theme_name: String) { - self.selected_index = self.matches.iter() + fn select_if_matching(&mut self, theme_name: &str) { + self.selected_index = self + .matches + .iter() .position(|mat| mat.string == theme_name) .unwrap_or(self.selected_index); } - fn selected_theme_name(&self) -> Option { - self.matches.get(self.selected_index).map(|mat| mat.string.clone()) + fn current_theme(&self) -> Arc { + self.settings_tx.lock().borrow().theme.clone() + } + + fn set_theme(&self, theme: Arc, cx: &mut MutableAppContext) { + self.settings_tx.lock().borrow_mut().theme = theme; + cx.refresh_windows(); } fn update_matches(&mut self, cx: &mut ViewContext) { @@ -223,8 +219,7 @@ impl ThemeSelector { }; self.selected_index = self.selected_index - .min(self.matches.len() - 1) - .max(0); + .min(self.matches.len().saturating_sub(1)); cx.notify(); } @@ -250,15 +245,10 @@ impl ThemeSelector { ) { match event { editor::Event::Edited => { - let previous_selection = self.selected_theme_name(); self.update_matches(cx); - if let Some(previous_selection) = previous_selection { - self.select_if_matching(previous_selection); - } - + self.select_if_matching(&self.current_theme().name); self.show_selected_theme(cx); - - }, + } editor::Event::Blurred => cx.emit(Event::Dismissed), _ => {} } @@ -330,6 +320,12 @@ impl ThemeSelector { impl Entity for ThemeSelector { type Event = Event; + + fn release(&mut self, cx: &mut MutableAppContext) { + if !self.selection_completed { + self.set_theme(self.original_theme.clone(), cx); + } + } } impl View for ThemeSelector { From 853acccbc27b6a5d5b09e843dbb57a7bef45abfd Mon Sep 17 00:00:00 2001 From: Keith Simmons Date: Wed, 9 Mar 2022 10:40:30 -0800 Subject: [PATCH 3/4] Make theme selector match other selector styling --- crates/file_finder/src/file_finder.rs | 2 +- crates/theme_selector/src/theme_selector.rs | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 552c7f41315c78e1b800a4eba63a2cc9a3560dcd..0a8513332ccbf1b24ff10386e6cc24895668f363 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -76,7 +76,7 @@ impl View for FileFinder { Container::new( Flex::new(Axis::Vertical) .with_child( - Container::new(ChildView::new(&self.query_editor).boxed()) + ChildView::new(&self.query_editor).contained() .with_style(settings.theme.selector.input_editor.container) .boxed(), ) diff --git a/crates/theme_selector/src/theme_selector.rs b/crates/theme_selector/src/theme_selector.rs index 1f334a860906d63825ed64984237063b806e2db5..11fbe3593535f0f3dd319f8af0f684a4e6871ba4 100644 --- a/crates/theme_selector/src/theme_selector.rs +++ b/crates/theme_selector/src/theme_selector.rs @@ -340,7 +340,11 @@ impl View for ThemeSelector { ConstrainedBox::new( Container::new( Flex::new(Axis::Vertical) - .with_child(ChildView::new(&self.query_editor).boxed()) + .with_child( + ChildView::new(&self.query_editor).contained() + .with_style(settings.theme.selector.input_editor.container) + .boxed(), + ) .with_child(Flexible::new(1.0, false, self.render_matches(cx)).boxed()) .boxed(), ) From 5aad1ff788d8f4e54640e132e1520a66e35d06ec Mon Sep 17 00:00:00 2001 From: Keith Simmons Date: Wed, 9 Mar 2022 10:42:27 -0800 Subject: [PATCH 4/4] formatting fixes --- crates/file_finder/src/file_finder.rs | 3 ++- crates/theme_selector/src/theme_selector.rs | 6 ++++-- crates/workspace/src/workspace.rs | 9 ++++++--- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 0a8513332ccbf1b24ff10386e6cc24895668f363..9e1a6d479c46e705ee5ed36c8689a80fe3f1f4cb 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -76,7 +76,8 @@ impl View for FileFinder { Container::new( Flex::new(Axis::Vertical) .with_child( - ChildView::new(&self.query_editor).contained() + ChildView::new(&self.query_editor) + .contained() .with_style(settings.theme.selector.input_editor.container) .boxed(), ) diff --git a/crates/theme_selector/src/theme_selector.rs b/crates/theme_selector/src/theme_selector.rs index 11fbe3593535f0f3dd319f8af0f684a4e6871ba4..8483fa4e8a25ae2d6c53b8872b45d04ae0b4b4e3 100644 --- a/crates/theme_selector/src/theme_selector.rs +++ b/crates/theme_selector/src/theme_selector.rs @@ -218,7 +218,8 @@ impl ThemeSelector { )) }; - self.selected_index = self.selected_index + self.selected_index = self + .selected_index .min(self.matches.len().saturating_sub(1)); cx.notify(); @@ -341,7 +342,8 @@ impl View for ThemeSelector { Container::new( Flex::new(Axis::Vertical) .with_child( - ChildView::new(&self.query_editor).contained() + ChildView::new(&self.query_editor) + .contained() .with_style(settings.theme.selector.input_editor.container) .boxed(), ) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 4e057fa6bc47086c507f8e79d7e1168da71f6f17..f06a244c0580fe9bf31ad2a387e9622322308daa 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -755,7 +755,11 @@ impl Workspace { } // Returns the model that was toggled closed if it was open - pub fn toggle_modal(&mut self, cx: &mut ViewContext, add_view: F) -> Option> + pub fn toggle_modal( + &mut self, + cx: &mut ViewContext, + add_view: F, + ) -> Option> where V: 'static + View, F: FnOnce(&mut ViewContext, &mut Self) -> ViewHandle, @@ -763,8 +767,7 @@ impl Workspace { cx.notify(); // Whatever modal was visible is getting clobbered. If its the same type as V, then return // it. Otherwise, create a new modal and set it as active. - let already_open_modal = self.modal.take() - .and_then(|modal| modal.downcast::()); + let already_open_modal = self.modal.take().and_then(|modal| modal.downcast::()); if let Some(already_open_modal) = already_open_modal { cx.focus_self(); Some(already_open_modal)