settings_ui: Fix dropdowns after #41036 (#41920) (cherry-pick) (#41930)

Cherry-pick of #41920

----
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 nothing

Co-authored-by: Ben Kunkle <[email protected]>

Author: ConradIrwin

crates/settings_ui/src/components.rs

@@ -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::*;

crates/settings_ui/src/components/dropdown.rs

@@ -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()
+    }
+}

crates/settings_ui/src/settings_ui.rs

@@ -11,7 +11,6 @@ use gpui::{
     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;
@@ -3325,7 +3326,7 @@ fn render_dropdown<T>(
     field: SettingField<T>,
     file: SettingsUiFile,
     metadata: Option<&SettingsFieldMetadata>,
-    window: &mut Window,
+    _window: &mut Window,
     cx: &mut App,
 ) -> AnyElement
 where
@@ -3341,56 +3342,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()
 }