From 970c7b89877fb12ca571fa1514e75bf7f29f417a Mon Sep 17 00:00:00 2001 From: Julia Date: Fri, 8 Dec 2023 12:02:31 -0500 Subject: [PATCH 1/3] zed2: Properly position terminal context menu & hide on dismiss --- crates/terminal_view2/src/terminal_view.rs | 42 ++++++++++++++-------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/crates/terminal_view2/src/terminal_view.rs b/crates/terminal_view2/src/terminal_view.rs index 5a81b494b3..c4f1512e8c 100644 --- a/crates/terminal_view2/src/terminal_view.rs +++ b/crates/terminal_view2/src/terminal_view.rs @@ -9,9 +9,10 @@ pub mod terminal_panel; // use crate::terminal_element::TerminalElement; use editor::{scroll::autoscroll::Autoscroll, Editor}; use gpui::{ - div, Action, AnyElement, AppContext, Div, EventEmitter, FocusEvent, FocusHandle, Focusable, - FocusableElement, FocusableView, KeyContext, KeyDownEvent, Keystroke, Model, MouseButton, - MouseDownEvent, Pixels, Render, Subscription, Task, View, VisualContext, WeakView, + div, overlay, Action, AnyElement, AppContext, DismissEvent, Div, EventEmitter, FocusEvent, + FocusHandle, Focusable, FocusableElement, FocusableView, KeyContext, KeyDownEvent, Keystroke, + Model, MouseButton, MouseDownEvent, Pixels, Render, Subscription, Task, View, VisualContext, + WeakView, }; use language::Bias; use persistence::TERMINAL_DB; @@ -81,7 +82,7 @@ pub struct TerminalView { has_new_content: bool, //Currently using iTerm bell, show bell emoji in tab until input is received has_bell: bool, - context_menu: Option>, + context_menu: Option<(View, gpui::Point, Subscription)>, blink_state: bool, blinking_on: bool, blinking_paused: bool, @@ -312,14 +313,24 @@ impl TerminalView { position: gpui::Point, cx: &mut ViewContext, ) { - self.context_menu = Some(ContextMenu::build(cx, |menu, cx| { + let context_menu = ContextMenu::build(cx, |menu, cx| { menu.action("Clear", Box::new(Clear)) .action("Close", Box::new(CloseActiveItem { save_intent: None })) - })); - // todo!(context menus) - // self.context_menu - // .show(position, AnchorCorner::TopLeft, menu_entries, cx); - // cx.notify(); + }); + + cx.focus_view(&context_menu); + let subscription = + cx.subscribe(&context_menu, |this, _, _: &DismissEvent, cx| { + if this.context_menu.as_ref().is_some_and(|context_menu| { + context_menu.0.focus_handle(cx).contains_focused(cx) + }) { + cx.focus_self(); + } + this.context_menu.take(); + cx.notify(); + }); + + self.context_menu = Some((context_menu, position, subscription)); } fn show_character_palette(&mut self, _: &ShowCharacterPalette, cx: &mut ViewContext) { @@ -658,11 +669,12 @@ impl Render for TerminalView { }), ), ) - .children( - self.context_menu - .clone() - .map(|context_menu| div().z_index(1).absolute().child(context_menu)), - ) + .children(self.context_menu.as_ref().map(|(menu, positon, _)| { + overlay() + .position(*positon) + .anchor(gpui::AnchorCorner::TopLeft) + .child(menu.clone()) + })) .track_focus(&self.focus_handle) .on_focus_in(cx.listener(Self::focus_in)) .on_focus_out(cx.listener(Self::focus_out)) From 79e6dedb7a98bcc5b7f97aed91f5953579a4d6dd Mon Sep 17 00:00:00 2001 From: Julia Date: Fri, 8 Dec 2023 13:49:42 -0500 Subject: [PATCH 2/3] Track focus shenanigans with context menu Co-Authored-By: Max Brunsfeld --- crates/terminal_view2/src/terminal_element.rs | 16 ++---------- crates/terminal_view2/src/terminal_view.rs | 25 +++++++++---------- crates/ui2/src/components/context_menu.rs | 6 +---- 3 files changed, 15 insertions(+), 32 deletions(-) diff --git a/crates/terminal_view2/src/terminal_element.rs b/crates/terminal_view2/src/terminal_element.rs index d61ba5988e..907e8cd9c1 100644 --- a/crates/terminal_view2/src/terminal_element.rs +++ b/crates/terminal_view2/src/terminal_element.rs @@ -5,7 +5,7 @@ use gpui::{ FontStyle, FontWeight, HighlightStyle, Hsla, InteractiveElement, InteractiveElementState, IntoElement, LayoutId, Model, ModelContext, ModifiersChangedEvent, MouseButton, Pixels, PlatformInputHandler, Point, Rgba, ShapedLine, Size, StatefulInteractiveElement, Styled, - TextRun, TextStyle, TextSystem, UnderlineStyle, View, WhiteSpace, WindowContext, + TextRun, TextStyle, TextSystem, UnderlineStyle, WhiteSpace, WindowContext, }; use itertools::Itertools; use language::CursorShape; @@ -27,8 +27,6 @@ use ui::Tooltip; use std::mem; use std::{fmt::Debug, ops::RangeInclusive}; -use crate::TerminalView; - ///The information generated during layout that is necessary for painting pub struct LayoutState { cells: Vec, @@ -149,7 +147,6 @@ impl LayoutRect { ///We need to keep a reference to the view for mouse events, do we need it for any other terminal stuff, or can we move that to connection? pub struct TerminalElement { terminal: Model, - terminal_view: View, focus: FocusHandle, focused: bool, cursor_visible: bool, @@ -168,7 +165,6 @@ impl StatefulInteractiveElement for TerminalElement {} impl TerminalElement { pub fn new( terminal: Model, - terminal_view: View, focus: FocusHandle, focused: bool, cursor_visible: bool, @@ -176,7 +172,6 @@ impl TerminalElement { ) -> TerminalElement { TerminalElement { terminal, - terminal_view, focused, focus: focus.clone(), cursor_visible, @@ -774,18 +769,11 @@ impl Element for TerminalElement { (layout_id, interactive_state) } - fn paint( - mut self, - bounds: Bounds, - state: &mut Self::State, - cx: &mut WindowContext<'_>, - ) { + fn paint(self, bounds: Bounds, state: &mut Self::State, cx: &mut WindowContext<'_>) { let mut layout = self.compute_layout(bounds, cx); let theme = cx.theme(); - let dispatch_context = self.terminal_view.read(cx).dispatch_context(cx); - self.interactivity().key_context = Some(dispatch_context); cx.paint_quad( bounds, Default::default(), diff --git a/crates/terminal_view2/src/terminal_view.rs b/crates/terminal_view2/src/terminal_view.rs index c4f1512e8c..d41e535d50 100644 --- a/crates/terminal_view2/src/terminal_view.rs +++ b/crates/terminal_view2/src/terminal_view.rs @@ -632,7 +632,6 @@ impl Render for TerminalView { fn render(&mut self, cx: &mut ViewContext) -> Self::Element { let terminal_handle = self.terminal.clone(); - let this_view = cx.view().clone(); let self_id = cx.entity_id(); let focused = self.focus_handle.is_focused(cx); @@ -640,22 +639,25 @@ impl Render for TerminalView { div() .size_full() .relative() + .track_focus(&self.focus_handle) + .key_context(self.dispatch_context(cx)) + .on_action(cx.listener(TerminalView::send_text)) + .on_action(cx.listener(TerminalView::send_keystroke)) + .on_action(cx.listener(TerminalView::copy)) + .on_action(cx.listener(TerminalView::paste)) + .on_action(cx.listener(TerminalView::clear)) + .on_action(cx.listener(TerminalView::show_character_palette)) + .on_action(cx.listener(TerminalView::select_all)) + .on_focus_in(cx.listener(Self::focus_in)) + .on_focus_out(cx.listener(Self::focus_out)) + .on_key_down(cx.listener(Self::key_down)) .child( div() .z_index(0) .absolute() .size_full() - .on_key_down(cx.listener(Self::key_down)) - .on_action(cx.listener(TerminalView::send_text)) - .on_action(cx.listener(TerminalView::send_keystroke)) - .on_action(cx.listener(TerminalView::copy)) - .on_action(cx.listener(TerminalView::paste)) - .on_action(cx.listener(TerminalView::clear)) - .on_action(cx.listener(TerminalView::show_character_palette)) - .on_action(cx.listener(TerminalView::select_all)) .child(TerminalElement::new( terminal_handle, - this_view, self.focus_handle.clone(), focused, self.should_show_cursor(focused, cx), @@ -675,9 +677,6 @@ impl Render for TerminalView { .anchor(gpui::AnchorCorner::TopLeft) .child(menu.clone()) })) - .track_focus(&self.focus_handle) - .on_focus_in(cx.listener(Self::focus_in)) - .on_focus_out(cx.listener(Self::focus_out)) } } diff --git a/crates/ui2/src/components/context_menu.rs b/crates/ui2/src/components/context_menu.rs index 0d6a632db5..c2dc0abe1a 100644 --- a/crates/ui2/src/components/context_menu.rs +++ b/crates/ui2/src/components/context_menu.rs @@ -239,7 +239,6 @@ impl Render for ContextMenu { action, } => { let handler = handler.clone(); - let dismiss = cx.listener(|_, _, cx| cx.emit(DismissEvent)); let label_element = if let Some(icon) = icon { h_stack() @@ -263,10 +262,7 @@ impl Render for ContextMenu { })), ) .selected(Some(ix) == self.selected_index) - .on_click(move |event, cx| { - handler(cx); - dismiss(event, cx) - }) + .on_click(move |_, cx| handler(cx)) .into_any_element() } }, From f0cc54a0b528766d3f24e5e6a39d517ea3ec324d Mon Sep 17 00:00:00 2001 From: Julia Date: Fri, 8 Dec 2023 14:26:02 -0500 Subject: [PATCH 3/3] Comment the weirdness Co-Authored-By: Max Brunsfeld --- crates/terminal_view2/src/terminal_view.rs | 37 ++++++++++------------ 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/crates/terminal_view2/src/terminal_view.rs b/crates/terminal_view2/src/terminal_view.rs index d41e535d50..83c7779d2e 100644 --- a/crates/terminal_view2/src/terminal_view.rs +++ b/crates/terminal_view2/src/terminal_view.rs @@ -11,8 +11,8 @@ use editor::{scroll::autoscroll::Autoscroll, Editor}; use gpui::{ div, overlay, Action, AnyElement, AppContext, DismissEvent, Div, EventEmitter, FocusEvent, FocusHandle, Focusable, FocusableElement, FocusableView, KeyContext, KeyDownEvent, Keystroke, - Model, MouseButton, MouseDownEvent, Pixels, Render, Subscription, Task, View, VisualContext, - WeakView, + Model, MouseButton, MouseDownEvent, Pixels, Render, Styled, Subscription, Task, View, + VisualContext, WeakView, }; use language::Bias; use persistence::TERMINAL_DB; @@ -651,25 +651,22 @@ impl Render for TerminalView { .on_focus_in(cx.listener(Self::focus_in)) .on_focus_out(cx.listener(Self::focus_out)) .on_key_down(cx.listener(Self::key_down)) + .on_mouse_down( + MouseButton::Right, + cx.listener(|this, event: &MouseDownEvent, cx| { + this.deploy_context_menu(event.position, cx); + cx.notify(); + }), + ) .child( - div() - .z_index(0) - .absolute() - .size_full() - .child(TerminalElement::new( - terminal_handle, - self.focus_handle.clone(), - focused, - self.should_show_cursor(focused, cx), - self.can_navigate_to_selected_word, - )) - .on_mouse_down( - MouseButton::Right, - cx.listener(|this, event: &MouseDownEvent, cx| { - this.deploy_context_menu(event.position, cx); - cx.notify(); - }), - ), + // TODO: Oddly this wrapper div is needed for TerminalElement to not steal events from the context menu + div().size_full().child(TerminalElement::new( + terminal_handle, + self.focus_handle.clone(), + focused, + self.should_show_cursor(focused, cx), + self.can_navigate_to_selected_word, + )), ) .children(self.context_menu.as_ref().map(|(menu, positon, _)| { overlay()