Tidy up

crates/command_palette2/src/command_palette.rs

@@ -4,7 +4,7 @@ use fuzzy::{StringMatch, StringMatchCandidate};
 use gpui::{
     actions, div, Action, AnyElement, AnyWindowHandle, AppContext, BorrowWindow, Component, Div,
     Element, EventEmitter, FocusHandle, Keystroke, ParentElement, Render, StatelessInteractive,
-    Styled, View, ViewContext, VisualContext, WeakView,
+    Styled, View, ViewContext, VisualContext, WeakView, WindowContext,
 };
 use picker::{Picker, PickerDelegate};
 use std::cmp::{self, Reverse};
@@ -14,7 +14,7 @@ use util::{
     channel::{parse_zed_link, ReleaseChannel, RELEASE_CHANNEL},
     ResultExt,
 };
-use workspace::{ModalEvent, Workspace};
+use workspace::{Modal, ModalEvent, Workspace};
 use zed_actions::OpenZedURL;
 
 actions!(Toggle);
@@ -24,7 +24,7 @@ pub fn init(cx: &mut AppContext) {
 
     cx.observe_new_views(
         |workspace: &mut Workspace, _: &mut ViewContext<Workspace>| {
-            workspace.modal_layer().register_modal(Toggle, |_, cx| {
+            workspace.modal_layer().register_modal(Toggle, |cx| {
                 let Some(previous_focus_handle) = cx.focused() else {
                     return None;
                 };
@@ -73,7 +73,13 @@ impl CommandPalette {
         Self { picker }
     }
 }
+
 impl EventEmitter<ModalEvent> for CommandPalette {}
+impl Modal for CommandPalette {
+    fn focus(&self, cx: &mut WindowContext) {
+        self.picker.update(cx, |picker, cx| picker.focus(cx));
+    }
+}
 
 impl Render for CommandPalette {
     type Element = Div<Self>;
@@ -147,7 +153,6 @@ impl PickerDelegate for CommandPaletteDelegate {
     type ListItem = Div<Picker<Self>>;
 
     fn match_count(&self) -> usize {
-        dbg!(self.matches.len());
         self.matches.len()
     }
 
@@ -254,7 +259,6 @@ impl PickerDelegate for CommandPaletteDelegate {
             picker
                 .update(&mut cx, |picker, _| {
                     let delegate = &mut picker.delegate;
-                    dbg!(&matches);
                     delegate.commands = commands;
                     delegate.matches = matches;
                     if delegate.matches.is_empty() {
@@ -269,6 +273,7 @@ impl PickerDelegate for CommandPaletteDelegate {
     }
 
     fn dismissed(&mut self, cx: &mut ViewContext<Picker<Self>>) {
+        cx.focus(&self.previous_focus_handle);
         self.command_palette
             .update(cx, |_, cx| cx.emit(ModalEvent::Dismissed))
             .log_err();

crates/go_to_line2/src/go_to_line.rs

@@ -8,17 +8,19 @@ use text::{Bias, Point};
 use theme::ActiveTheme;
 use ui::{h_stack, modal, v_stack, Label, LabelColor};
 use util::paths::FILE_ROW_COLUMN_DELIMITER;
-use workspace::{ModalEvent, Workspace};
+use workspace::{Modal, ModalEvent, Workspace};
 
 actions!(Toggle);
 
 pub fn init(cx: &mut AppContext) {
     cx.observe_new_views(
-        |workspace: &mut Workspace, _: &mut ViewContext<Workspace>| {
+        |workspace: &mut Workspace, cx: &mut ViewContext<Workspace>| {
-            workspace
+            let handle = cx.view().downgrade();
-                .modal_layer()
+
-                .register_modal(Toggle, |workspace, cx| {
+            workspace.modal_layer().register_modal(Toggle, move |cx| {
+                let workspace = handle.upgrade()?;
                 let editor = workspace
+                    .read(cx)
                     .active_item(cx)
                     .and_then(|active_item| active_item.downcast::<Editor>())?;
 
@@ -44,14 +46,15 @@ pub enum Event {
 impl EventEmitter<Event> for GoToLine {}
 
 impl EventEmitter<ModalEvent> for GoToLine {}
+impl Modal for GoToLine {
+    fn focus(&self, cx: &mut WindowContext) {
+        self.line_editor.update(cx, |editor, cx| editor.focus(cx))
+    }
+}
 
 impl GoToLine {
     pub fn new(active_editor: View<Editor>, cx: &mut ViewContext<Self>) -> Self {
-        let line_editor = cx.build_view(|cx| {
+        let line_editor = cx.build_view(|cx| Editor::single_line(cx));
-            let editor = Editor::single_line(cx);
-            editor.focus(cx);
-            editor
-        });
         let line_editor_change = cx.subscribe(&line_editor, Self::on_line_editor_event);
 
         let editor = active_editor.read(cx);

crates/gpui2/src/elements/uniform_list.rs

@@ -9,6 +9,9 @@ use smallvec::SmallVec;
 use std::{cmp, ops::Range, sync::Arc};
 use taffy::style::Overflow;
 
+/// uniform_list provides lazy rendering for a set of items that are of uniform height.
+/// When rendered into a container with overflow-y: hidden and a fixed (or max) height,
+/// uniform_list will only render the visibile subset of items.
 pub fn uniform_list<Id, V, C>(
     id: Id,
     item_count: usize,
@@ -20,9 +23,12 @@ where
     C: Component<V>,
 {
     let id = id.into();
+    let mut style = StyleRefinement::default();
+    style.overflow.y = Some(Overflow::Hidden);
+
     UniformList {
         id: id.clone(),
-        style: Default::default(),
+        style,
         item_count,
         render_items: Box::new(move |view, visible_range, cx| {
             f(view, visible_range, cx)
@@ -123,6 +129,7 @@ impl<V: 'static> Element<V> for UniformList<V> {
         let max_items = self.item_count;
         let item_size = element_state.item_size;
         let rem_size = cx.rem_size();
+
         cx.request_measured_layout(
             self.computed_style(),
             rem_size,
@@ -132,15 +139,12 @@ impl<V: 'static> Element<V> for UniformList<V> {
                     .width
                     .unwrap_or(match available_space.width {
                         AvailableSpace::Definite(x) => x,
-                        AvailableSpace::MinContent => item_size.width,
+                        AvailableSpace::MinContent | AvailableSpace::MaxContent => item_size.width,
-                        AvailableSpace::MaxContent => item_size.width,
                     });
                 let height = match available_space.height {
                     AvailableSpace::Definite(x) => desired_height.min(x),
-                    AvailableSpace::MinContent => desired_height,
+                    AvailableSpace::MinContent | AvailableSpace::MaxContent => desired_height,
-                    AvailableSpace::MaxContent => desired_height,
                 };
-                dbg!(known_dimensions, available_space, size(width, height));
                 size(width, height)
             },
         )
@@ -171,7 +175,6 @@ impl<V: 'static> Element<V> for UniformList<V> {
                 let item_height = self
                     .measure_first_item(view_state, Some(padded_bounds.size.width), cx)
                     .height;
-                dbg!(item_height, padded_bounds);
                 if let Some(scroll_handle) = self.scroll_handle.clone() {
                     scroll_handle.0.lock().replace(ScrollHandleState {
                         item_height,
@@ -184,7 +187,6 @@ impl<V: 'static> Element<V> for UniformList<V> {
                 } else {
                     0
                 };
-                dbg!(visible_item_count);
                 let scroll_offset = element_state
                     .interactive
                     .scroll_offset()
@@ -289,3 +291,11 @@ impl<V: 'static> Component<V> for UniformList<V> {
         AnyElement::new(self)
     }
 }
+
+#[cfg(test)]
+mod test {
+    use crate::{self as gpui, TestAppContext};
+
+    #[gpui::test]
+    fn test_uniform_list(cx: &mut TestAppContext) {}
+}

crates/gpui2/src/window.rs

@@ -146,6 +146,11 @@ impl FocusHandle {
         }
     }
 
+    /// Moves the focus to the element associated with this handle.
+    pub fn focus(&self, cx: &mut WindowContext) {
+        cx.focus(self)
+    }
+
     /// Obtains whether the element associated with this handle is currently focused.
     pub fn is_focused(&self, cx: &WindowContext) -> bool {
         self.id.is_focused(cx)

crates/picker2/src/picker2.rs

@@ -59,6 +59,7 @@ impl<D: PickerDelegate> Picker<D> {
             let ix = cmp::min(index + 1, count - 1);
             self.delegate.set_selected_index(ix, cx);
             self.scroll_handle.scroll_to_item(ix);
+            cx.notify();
         }
     }
 
@@ -69,6 +70,7 @@ impl<D: PickerDelegate> Picker<D> {
             let ix = index.saturating_sub(1);
             self.delegate.set_selected_index(ix, cx);
             self.scroll_handle.scroll_to_item(ix);
+            cx.notify();
         }
     }
 
@@ -77,6 +79,7 @@ impl<D: PickerDelegate> Picker<D> {
         if count > 0 {
             self.delegate.set_selected_index(0, cx);
             self.scroll_handle.scroll_to_item(0);
+            cx.notify();
         }
     }
 
@@ -85,6 +88,7 @@ impl<D: PickerDelegate> Picker<D> {
         if count > 0 {
             self.delegate.set_selected_index(count - 1, cx);
             self.scroll_handle.scroll_to_item(count - 1);
+            cx.notify();
         }
     }
 
@@ -163,7 +167,11 @@ impl<D: PickerDelegate> Render for Picker<D> {
                     .bg(cx.theme().colors().element_background),
             )
             .child(
-                v_stack().py_0p5().px_1().grow().max_h_96().child(
+                v_stack()
+                    .py_0p5()
+                    .px_1()
+                    .grow()
+                    .child(
                         uniform_list("candidates", self.delegate.match_count(), {
                             move |this: &mut Self, visible_range, cx| {
                                 let selected_ix = this.delegate.selected_index();
@@ -172,9 +180,10 @@ impl<D: PickerDelegate> Render for Picker<D> {
                                     .collect()
                             }
                         })
-                    .track_scroll(self.scroll_handle.clone())
+                        .track_scroll(self.scroll_handle.clone()),
-                    .size_full(),
+                    )
-                ),
+                    .max_h_72()
+                    .overflow_hidden(),
             )
     }
 }

crates/workspace2/src/modal_layer.rs

@@ -1,14 +1,21 @@
 use crate::Workspace;
 use gpui::{
-    div, px, AnyView, Component, Div, EventEmitter, ParentElement, Render, StatefulInteractivity,
+    div, px, AnyView, Component, Div, EventEmitter, FocusHandle, ParentElement, Render,
-    StatelessInteractive, Styled, Subscription, View, ViewContext,
+    StatefulInteractivity, StatelessInteractive, Styled, Subscription, View, ViewContext,
+    WindowContext,
 };
 use std::{any::TypeId, sync::Arc};
 use ui::v_stack;
 
+pub struct ActiveModal {
+    modal: AnyView,
+    subscription: Subscription,
+    previous_focus_handle: Option<FocusHandle>,
+    focus_handle: FocusHandle,
+}
+
 pub struct ModalLayer {
-    open_modal: Option<AnyView>,
+    active_modal: Option<ActiveModal>,
-    subscription: Option<Subscription>,
     registered_modals: Vec<(
         TypeId,
         Box<
@@ -19,6 +26,10 @@ pub struct ModalLayer {
     )>,
 }
 
+pub trait Modal: Render + EventEmitter<ModalEvent> {
+    fn focus(&self, cx: &mut WindowContext);
+}
+
 pub enum ModalEvent {
     Dismissed,
 }
@@ -26,16 +37,15 @@ pub enum ModalEvent {
 impl ModalLayer {
     pub fn new() -> Self {
         Self {
-            open_modal: None,
+            active_modal: None,
-            subscription: None,
             registered_modals: Vec::new(),
         }
     }
 
     pub fn register_modal<A: 'static, V, B>(&mut self, action: A, build_view: B)
     where
-        V: EventEmitter<ModalEvent> + Render,
+        V: Modal,
-        B: Fn(&mut Workspace, &mut ViewContext<Workspace>) -> Option<View<V>> + 'static,
+        B: Fn(&mut WindowContext) -> Option<View<V>> + 'static,
     {
         let build_view = Arc::new(build_view);
 
@@ -45,29 +55,56 @@ impl ModalLayer {
                 let build_view = build_view.clone();
 
                 div.on_action(move |workspace, event: &A, cx| {
-                    let Some(new_modal) = (build_view)(workspace, cx) else {
+                    let previous_focus = cx.focused();
+                    if let Some(active_modal) = &workspace.modal_layer().active_modal {
+                        if active_modal.modal.clone().downcast::<V>().is_ok() {
+                            workspace.modal_layer().hide_modal(cx);
+                            return;
+                        }
+                    }
+                    let Some(new_modal) = (build_view)(cx) else {
                         return;
                     };
-                    workspace.modal_layer().show_modal(new_modal, cx);
+                    workspace
+                        .modal_layer()
+                        .show_modal(previous_focus, new_modal, cx);
                 })
             }),
         ));
     }
 
-    pub fn show_modal<V>(&mut self, new_modal: View<V>, cx: &mut ViewContext<Workspace>)
+    pub fn show_modal<V>(
-    where
+        &mut self,
+        previous_focus: Option<FocusHandle>,
+        new_modal: View<V>,
+        cx: &mut ViewContext<Workspace>,
+    ) where
         V: EventEmitter<ModalEvent> + Render,
     {
-        self.subscription = Some(cx.subscribe(&new_modal, |this, modal, e, cx| match e {
+        self.active_modal = Some(ActiveModal {
+            modal: new_modal.clone().into(),
+            subscription: cx.subscribe(&new_modal, |this, modal, e, cx| match e {
                 ModalEvent::Dismissed => this.modal_layer().hide_modal(cx),
-        }));
+            }),
-        self.open_modal = Some(new_modal.into());
+            previous_focus_handle: previous_focus,
+            focus_handle: cx.focus_handle(),
+        });
         cx.notify();
     }
 
     pub fn hide_modal(&mut self, cx: &mut ViewContext<Workspace>) {
-        self.open_modal.take();
+        dbg!("hiding...");
-        self.subscription.take();
+        if let Some(active_modal) = self.active_modal.take() {
+            dbg!("something");
+            if let Some(previous_focus) = active_modal.previous_focus_handle {
+                dbg!("oohthing");
+                if active_modal.focus_handle.contains_focused(cx) {
+                    dbg!("aahthing");
+                    previous_focus.focus(cx);
+                }
+            }
+        }
+
         cx.notify();
     }
 
@@ -81,7 +118,7 @@ impl ModalLayer {
             parent = (action)(parent);
         }
 
-        parent.when_some(self.open_modal.as_ref(), |parent, open_modal| {
+        parent.when_some(self.active_modal.as_ref(), |parent, open_modal| {
             let container1 = div()
                 .absolute()
                 .flex()
@@ -92,10 +129,13 @@ impl ModalLayer {
                 .left_0()
                 .z_index(400);
 
-            // transparent layer
+            let container2 = v_stack()
-            let container2 = v_stack().h(px(0.0)).relative().top_20();
+                .h(px(0.0))
+                .relative()
+                .top_20()
+                .track_focus(&open_modal.focus_handle);
 
-            parent.child(container1.child(container2.child(open_modal.clone())))
+            parent.child(container1.child(container2.child(open_modal.modal.clone())))
         })
     }
 }