Ben Kunkle created
Closes #41533
Both of the issues in the release notes that are fixed in this PR, were
caused by incorrect usage of the `window.use_state` API.
The first issue was caused by calling `window.use_state` in a render
helper, resulting in the element ID used to share state being the same
across different pages, resulting in the state being re-used when it
should have been re-created. The fix for this was to move the
`window.state` (and rendering logic) into a `impl RenderOnce` component,
so that the IDs are resolved during the render, avoiding the state
conflicts.
The second issue is caused by using a `move` closure in the
`window.use_state` call, resulting in stale closure values when the
window state is re-used.
Release Notes:
- settings_ui: Fixed an issue where some dropdown menus would show
options from a different dropdown when clicked
- settings_ui: Fixed an issue where attempting to change a setting in a
dropdown back to it's original value after changing it would do nothingcrates/settings_ui/src/components.rs | 2
crates/settings_ui/src/components/dropdown.rs | 108 +++++++++++++++++++++
crates/settings_ui/src/settings_ui.rs | 66 ++---------
3 files changed, 125 insertions(+), 51 deletions(-)
@@ -1,8 +1,10 @@
+mod dropdown;
mod font_picker;
mod icon_theme_picker;
mod input_field;
mod theme_picker;
+pub use dropdown::*;
pub use font_picker::font_picker;
pub use icon_theme_picker::icon_theme_picker;
pub use input_field::*;
@@ -0,0 +1,108 @@
+use std::rc::Rc;
+
+use gpui::{App, ElementId, IntoElement, RenderOnce};
+use heck::ToTitleCase as _;
+use ui::{
+ ButtonSize, ContextMenu, DropdownMenu, DropdownStyle, FluentBuilder as _, IconPosition, px,
+};
+
+#[derive(IntoElement)]
+pub struct EnumVariantDropdown<T>
+where
+ T: strum::VariantArray + strum::VariantNames + Copy + PartialEq + Send + Sync + 'static,
+{
+ id: ElementId,
+ current_value: T,
+ variants: &'static [T],
+ labels: &'static [&'static str],
+ should_do_title_case: bool,
+ tab_index: Option<isize>,
+ on_change: Rc<dyn Fn(T, &mut App) + 'static>,
+}
+
+impl<T> EnumVariantDropdown<T>
+where
+ T: strum::VariantArray + strum::VariantNames + Copy + PartialEq + Send + Sync + 'static,
+{
+ pub fn new(
+ id: impl Into<ElementId>,
+ current_value: T,
+ variants: &'static [T],
+ labels: &'static [&'static str],
+ on_change: impl Fn(T, &mut App) + 'static,
+ ) -> Self {
+ Self {
+ id: id.into(),
+ current_value,
+ variants,
+ labels,
+ should_do_title_case: true,
+ tab_index: None,
+ on_change: Rc::new(on_change),
+ }
+ }
+
+ pub fn title_case(mut self, title_case: bool) -> Self {
+ self.should_do_title_case = title_case;
+ self
+ }
+
+ pub fn tab_index(mut self, tab_index: isize) -> Self {
+ self.tab_index = Some(tab_index);
+ self
+ }
+}
+
+impl<T> RenderOnce for EnumVariantDropdown<T>
+where
+ T: strum::VariantArray + strum::VariantNames + Copy + PartialEq + Send + Sync + 'static,
+{
+ fn render(self, window: &mut ui::Window, cx: &mut ui::App) -> impl gpui::IntoElement {
+ let current_value_label = self.labels[self
+ .variants
+ .iter()
+ .position(|v| *v == self.current_value)
+ .unwrap()];
+
+ let context_menu = window.use_keyed_state(current_value_label, cx, |window, cx| {
+ ContextMenu::new(window, cx, move |mut menu, _, _| {
+ for (&value, &label) in std::iter::zip(self.variants, self.labels) {
+ let on_change = self.on_change.clone();
+ let current_value = self.current_value;
+ menu = menu.toggleable_entry(
+ if self.should_do_title_case {
+ label.to_title_case()
+ } else {
+ label.to_string()
+ },
+ value == current_value,
+ IconPosition::End,
+ None,
+ move |_, cx| {
+ on_change(value, cx);
+ },
+ );
+ }
+ menu
+ })
+ });
+
+ DropdownMenu::new(
+ self.id,
+ if self.should_do_title_case {
+ current_value_label.to_title_case()
+ } else {
+ current_value_label.to_string()
+ },
+ context_menu,
+ )
+ .when_some(self.tab_index, |elem, tab_index| elem.tab_index(tab_index))
+ .trigger_size(ButtonSize::Medium)
+ .style(DropdownStyle::Outlined)
+ .offset(gpui::Point {
+ x: px(0.0),
+ y: px(2.0),
+ })
+ .into_any_element()
+ }
+}
@@ -11,7 +11,6 @@ use gpui::{
Subscription, Task, TitlebarOptions, UniformListScrollHandle, Window, WindowBounds,
WindowHandle, WindowOptions, actions, div, list, point, prelude::*, px, uniform_list,
};
-use heck::ToTitleCase as _;
use project::{Project, WorktreeId};
use release_channel::ReleaseChannel;
use schemars::JsonSchema;
@@ -38,7 +37,9 @@ use util::{ResultExt as _, paths::PathStyle, rel_path::RelPath};
use workspace::{AppState, OpenOptions, OpenVisible, Workspace, client_side_decorations};
use zed_actions::{OpenSettings, OpenSettingsAt};
-use crate::components::{SettingsInputField, font_picker, icon_theme_picker, theme_picker};
+use crate::components::{
+ EnumVariantDropdown, SettingsInputField, font_picker, icon_theme_picker, theme_picker,
+};
const NAVBAR_CONTAINER_TAB_INDEX: isize = 0;
const NAVBAR_GROUP_TAB_INDEX: isize = 1;
@@ -3428,7 +3429,7 @@ fn render_dropdown<T>(
field: SettingField<T>,
file: SettingsUiFile,
metadata: Option<&SettingsFieldMetadata>,
- window: &mut Window,
+ _window: &mut Window,
cx: &mut App,
) -> AnyElement
where
@@ -3444,56 +3445,19 @@ where
SettingsStore::global(cx).get_value_from_file(file.to_settings(), field.pick);
let current_value = current_value.copied().unwrap_or(variants()[0]);
- let current_value_label =
- labels()[variants().iter().position(|v| *v == current_value).unwrap()];
-
- DropdownMenu::new(
- "dropdown",
- if should_do_titlecase {
- current_value_label.to_title_case()
- } else {
- current_value_label.to_string()
- },
- window.use_state(cx, |window, cx| {
- ContextMenu::new(window, cx, move |mut menu, _, _| {
- for (&value, &label) in std::iter::zip(variants(), labels()) {
- let file = file.clone();
- menu = menu.toggleable_entry(
- if should_do_titlecase {
- label.to_title_case()
- } else {
- label.to_string()
- },
- value == current_value,
- IconPosition::End,
- None,
- move |_, cx| {
- if value == current_value {
- return;
- }
- update_settings_file(
- file.clone(),
- field.json_path,
- cx,
- move |settings, _cx| {
- (field.write)(settings, Some(value));
- },
- )
- .log_err(); // todo(settings_ui) don't log err
- },
- );
- }
- menu
+ EnumVariantDropdown::new("dropdown", current_value, variants(), labels(), {
+ move |value, cx| {
+ if value == current_value {
+ return;
+ }
+ update_settings_file(file.clone(), field.json_path, cx, move |settings, _cx| {
+ (field.write)(settings, Some(value));
})
- }),
- )
- .tab_index(0)
- .trigger_size(ButtonSize::Medium)
- .style(DropdownStyle::Outlined)
- .offset(gpui::Point {
- x: px(0.0),
- y: px(2.0),
+ .log_err(); // todo(settings_ui) don't log err
+ }
})
+ .tab_index(0)
+ .title_case(should_do_titlecase)
.into_any_element()
}