From 0b5952e1bdd58c01a6ae0c4c7f39400d58e1da8f Mon Sep 17 00:00:00 2001 From: K Simmons Date: Tue, 13 Sep 2022 15:17:27 -0700 Subject: [PATCH] Fix incorrect rendering of toolbar in right anchored dock Make dock keybinding activate the dock if it wasn't hidden, and hide it if it was already active Make clicking the expanded dock wash, hide the dock Fix some issues with programmatically activating other panes, not hiding the dock Tweak dock anchor menu text Swap dock hide button for thin variant Fix dock sidebar interactions Add clicked state to search button and fix presenter issue sending clicked events when mouse not overlapping MouseRegion Co-Authored-By: Mikayla Maki --- assets/keymaps/default.json | 9 +-- crates/gpui/src/presenter.rs | 22 +++++++- crates/terminal/src/modal.rs | 72 ------------------------ crates/terminal/src/terminal.rs | 4 -- crates/theme/src/theme.rs | 6 ++ crates/workspace/src/dock.rs | 93 +++++++++++++++++++++++++------ crates/workspace/src/pane.rs | 21 ++++--- crates/workspace/src/workspace.rs | 29 ++++------ styles/src/styleTree/search.ts | 5 ++ styles/src/styleTree/workspace.ts | 4 +- 10 files changed, 134 insertions(+), 131 deletions(-) delete mode 100644 crates/terminal/src/modal.rs diff --git a/assets/keymaps/default.json b/assets/keymaps/default.json index b11a7900880ba44cc3772ab88135670ebbaf29d7..8995654523937ef4a75e0fb8eebf6bc64258a881 100644 --- a/assets/keymaps/default.json +++ b/assets/keymaps/default.json @@ -310,7 +310,7 @@ "cmd-shift-m": "diagnostics::Deploy", "cmd-shift-e": "project_panel::ToggleFocus", "cmd-alt-s": "workspace::SaveAll", - "shift-escape": "workspace::ToggleDock" + "shift-escape": "workspace::ActivateOrHideDock" } }, // Bindings from Sublime Text @@ -426,12 +426,5 @@ "cmd-v": "terminal::Paste", "cmd-k": "terminal::Clear" } - }, - { - "context": "ModalTerminal", - "bindings": { - "ctrl-cmd-space": "terminal::ShowCharacterPalette", - "shift-escape": "terminal::DeployModal" - } } ] \ No newline at end of file diff --git a/crates/gpui/src/presenter.rs b/crates/gpui/src/presenter.rs index dde8de831a60330935d5046a7d18123260c881ad..cba7d343782c09db731266f844181a53294a947b 100644 --- a/crates/gpui/src/presenter.rs +++ b/crates/gpui/src/presenter.rs @@ -218,6 +218,7 @@ impl Presenter { ) -> bool { if let Some(root_view_id) = cx.root_view_id(self.window_id) { let mut events_to_send = Vec::new(); + let mut invalidated_views: HashSet = Default::default(); // 1. Allocate the correct set of GPUI events generated from the platform events // -> These are usually small: [Mouse Down] or [Mouse up, Click] or [Mouse Moved, Mouse Dragged?] @@ -242,6 +243,12 @@ impl Presenter { } }) .collect(); + + // Clicked status is used when rendering views via the RenderContext. + // So when it changes, these views need to be rerendered + for clicked_region_id in self.clicked_region_ids.iter() { + invalidated_views.insert(clicked_region_id.view_id()); + } self.clicked_button = Some(e.button); } @@ -339,7 +346,6 @@ impl Presenter { self.mouse_position = position; } - let mut invalidated_views: HashSet = Default::default(); let mut any_event_handled = false; // 2. Process the raw mouse events into region events for mut region_event in events_to_send { @@ -394,12 +400,19 @@ impl Presenter { // Clear clicked regions and clicked button let clicked_region_ids = std::mem::replace(&mut self.clicked_region_ids, Default::default()); + // Clicked status is used when rendering views via the RenderContext. + // So when it changes, these views need to be rerendered + for clicked_region_id in clicked_region_ids.iter() { + invalidated_views.insert(clicked_region_id.view_id()); + } self.clicked_button = None; // Find regions which still overlap with the mouse since the last MouseDown happened for (mouse_region, _) in self.mouse_regions.iter().rev() { if clicked_region_ids.contains(&mouse_region.id()) { - valid_regions.push(mouse_region.clone()); + if mouse_region.bounds.contains_point(self.mouse_position) { + valid_regions.push(mouse_region.clone()); + } } } } @@ -439,13 +452,16 @@ impl Presenter { if let MouseRegionEvent::Hover(e) = &mut region_event { e.started = hovered_region_ids.contains(&valid_region.id()) } - // Handle Down events if the MouseRegion has a Click handler. This makes the api more intuitive as you would + // Handle Down events if the MouseRegion has a Click or Drag handler. This makes the api more intuitive as you would // not expect a MouseRegion to be transparent to Down events if it also has a Click handler. // This behavior can be overridden by adding a Down handler that calls cx.propogate_event if let MouseRegionEvent::Down(e) = ®ion_event { if valid_region .handlers .contains_handler(MouseRegionEvent::click_disc(), Some(e.button)) + || valid_region + .handlers + .contains_handler(MouseRegionEvent::drag_disc(), Some(e.button)) { event_cx.handled = true; } diff --git a/crates/terminal/src/modal.rs b/crates/terminal/src/modal.rs deleted file mode 100644 index 309a11d41f39ec0bad22c020678133f9cbb86c40..0000000000000000000000000000000000000000 --- a/crates/terminal/src/modal.rs +++ /dev/null @@ -1,72 +0,0 @@ -use gpui::{ModelHandle, ViewContext}; -use workspace::Workspace; - -use crate::{terminal_container_view::DeployModal, Event, Terminal}; - -pub fn deploy_modal(_workspace: &mut Workspace, _: &DeployModal, _cx: &mut ViewContext) { - // let window = cx.window_id(); - - // // Pull the terminal connection out of the global if it has been stored - // let possible_terminal = Dock::remove::(window, cx); - - // if let Some(terminal_handle) = possible_terminal { - // workspace.toggle_modal(cx, |_, cx| { - // // Create a view from the stored connection if the terminal modal is not already shown - // cx.add_view(|cx| TerminalContainer::from_terminal(terminal_handle.clone(), true, cx)) - // }); - // // Toggle Modal will dismiss the terminal modal if it is currently shown, so we must - // // store the terminal back in the global - // Dock::insert_or_replace::(window, terminal_handle, cx); - // } else { - // // No connection was stored, create a new terminal - // if let Some(closed_terminal_handle) = workspace.toggle_modal(cx, |workspace, cx| { - // // No terminal modal visible, construct a new one. - // let wd_strategy = cx - // .global::() - // .terminal_overrides - // .working_directory - // .clone() - // .unwrap_or(WorkingDirectory::CurrentProjectDirectory); - - // let working_directory = get_working_directory(workspace, cx, wd_strategy); - - // let this = cx.add_view(|cx| TerminalContainer::new(working_directory, true, cx)); - - // if let TerminalContainerContent::Connected(connected) = &this.read(cx).content { - // let terminal_handle = connected.read(cx).handle(); - // cx.subscribe(&terminal_handle, on_event).detach(); - // // Set the global immediately if terminal construction was successful, - // // in case the user opens the command palette - // Dock::insert_or_replace::(window, terminal_handle, cx); - // } - - // this - // }) { - // // Terminal modal was dismissed and the terminal view is connected, store the terminal - // if let TerminalContainerContent::Connected(connected) = - // &closed_terminal_handle.read(cx).content - // { - // let terminal_handle = connected.read(cx).handle(); - // // Set the global immediately if terminal construction was successful, - // // in case the user opens the command palette - // Dock::insert_or_replace::(window, terminal_handle, cx); - // } - // } - // } -} - -pub fn on_event( - _workspace: &mut Workspace, - _: ModelHandle, - _event: &Event, - _cx: &mut ViewContext, -) { - // Dismiss the modal if the terminal quit - // if let Event::CloseTerminal = event { - // Dock::remove::(cx.window_id(), cx); - - // if workspace.modal::().is_some() { - // workspace.dismiss_modal(cx) - // } - // } -} diff --git a/crates/terminal/src/terminal.rs b/crates/terminal/src/terminal.rs index d6c22ee6bce68013123028d17fe1e176f9229c66..24c51d0ccac4e320c91d4aa671a37352e590b299 100644 --- a/crates/terminal/src/terminal.rs +++ b/crates/terminal/src/terminal.rs @@ -1,5 +1,4 @@ pub mod mappings; -pub mod modal; pub mod terminal_container_view; pub mod terminal_element; pub mod terminal_view; @@ -32,7 +31,6 @@ use futures::{ use mappings::mouse::{ alt_scroll, mouse_button_report, mouse_moved_report, mouse_point, mouse_side, scroll_report, }; -use modal::deploy_modal; use procinfo::LocalProcessInfo; use settings::{AlternateScroll, Settings, Shell, TerminalBlink}; @@ -63,8 +61,6 @@ use crate::mappings::{ ///Initialize and register all of our action handlers pub fn init(cx: &mut MutableAppContext) { - cx.add_action(deploy_modal); - terminal_view::init(cx); terminal_container_view::init(cx); } diff --git a/crates/theme/src/theme.rs b/crates/theme/src/theme.rs index 259b9a690ceaef18ab2bbc8fc153ed09cada893e..657e3c59150ddedf15cdc704be5ac94d9a342b2d 100644 --- a/crates/theme/src/theme.rs +++ b/crates/theme/src/theme.rs @@ -571,6 +571,7 @@ pub struct CodeActions { pub struct Interactive { pub default: T, pub hover: Option, + pub clicked: Option, pub active: Option, pub disabled: Option, } @@ -579,6 +580,8 @@ impl Interactive { pub fn style_for(&self, state: MouseState, active: bool) -> &T { if active { self.active.as_ref().unwrap_or(&self.default) + } else if state.clicked == Some(gpui::MouseButton::Left) && self.clicked.is_some() { + self.clicked.as_ref().unwrap() } else if state.hovered { self.hover.as_ref().unwrap_or(&self.default) } else { @@ -601,6 +604,7 @@ impl<'de, T: DeserializeOwned> Deserialize<'de> for Interactive { #[serde(flatten)] default: Value, hover: Option, + clicked: Option, active: Option, disabled: Option, } @@ -627,6 +631,7 @@ impl<'de, T: DeserializeOwned> Deserialize<'de> for Interactive { }; let hover = deserialize_state(json.hover)?; + let clicked = deserialize_state(json.clicked)?; let active = deserialize_state(json.active)?; let disabled = deserialize_state(json.disabled)?; let default = serde_json::from_value(json.default).map_err(serde::de::Error::custom)?; @@ -634,6 +639,7 @@ impl<'de, T: DeserializeOwned> Deserialize<'de> for Interactive { Ok(Interactive { default, hover, + clicked, active, disabled, }) diff --git a/crates/workspace/src/dock.rs b/crates/workspace/src/dock.rs index b0ac47cc90b73d1876472c9b591d2337a1314a7c..a059687193965dda2483d591de775445c0417a66 100644 --- a/crates/workspace/src/dock.rs +++ b/crates/workspace/src/dock.rs @@ -17,15 +17,16 @@ pub struct MoveDock(pub DockAnchor); #[derive(PartialEq, Clone)] pub struct AddDefaultItemToDock; -actions!(workspace, [ToggleDock]); +actions!(workspace, [ToggleDock, ActivateOrHideDock]); impl_internal_actions!(workspace, [MoveDock, AddDefaultItemToDock]); pub fn init(cx: &mut MutableAppContext) { cx.add_action(Dock::toggle); + cx.add_action(Dock::activate_or_hide_dock); cx.add_action(Dock::move_dock); } -#[derive(Copy, Clone)] +#[derive(Copy, Clone, PartialEq, Eq)] pub enum DockPosition { Shown(DockAnchor), Hidden(DockAnchor), @@ -72,6 +73,13 @@ impl DockPosition { DockPosition::Hidden(_) => self, } } + + fn show(self) -> Self { + match self { + DockPosition::Hidden(anchor) => DockPosition::Shown(anchor), + DockPosition::Shown(_) => self, + } + } } pub type DefaultItemFactory = @@ -88,6 +96,9 @@ impl Dock { pub fn new(cx: &mut ViewContext, default_item_factory: DefaultItemFactory) -> Self { let anchor = cx.global::().default_dock_anchor; let pane = cx.add_view(|cx| Pane::new(Some(anchor), cx)); + pane.update(cx, |pane, cx| { + pane.set_active(false, cx); + }); let pane_id = pane.id(); cx.subscribe(&pane, move |workspace, _, event, cx| { workspace.handle_pane_event(pane_id, event, cx); @@ -119,6 +130,10 @@ impl Dock { new_position: DockPosition, cx: &mut ViewContext, ) { + if workspace.dock.position == new_position { + return; + } + workspace.dock.position = new_position; // Tell the pane about the new anchor position workspace.dock.pane.update(cx, |pane, cx| { @@ -139,7 +154,6 @@ impl Dock { let item_to_add = (workspace.dock.default_item_factory)(workspace, cx); Pane::add_item(workspace, &pane, item_to_add, true, true, None, cx); } - cx.focus(pane); } else if let Some(last_active_center_pane) = workspace.last_active_center_pane.clone() { cx.focus(last_active_center_pane); } @@ -151,10 +165,40 @@ impl Dock { Self::set_dock_position(workspace, workspace.dock.position.hide(), cx); } + pub fn show(workspace: &mut Workspace, cx: &mut ViewContext) { + Self::set_dock_position(workspace, workspace.dock.position.show(), cx); + } + + pub fn hide_on_sidebar_shown( + workspace: &mut Workspace, + sidebar_side: SidebarSide, + cx: &mut ViewContext, + ) { + if (sidebar_side == SidebarSide::Right && workspace.dock.is_anchored_at(DockAnchor::Right)) + || workspace.dock.is_anchored_at(DockAnchor::Expanded) + { + Self::hide(workspace, cx); + } + } + fn toggle(workspace: &mut Workspace, _: &ToggleDock, cx: &mut ViewContext) { Self::set_dock_position(workspace, workspace.dock.position.toggle(), cx); } + fn activate_or_hide_dock( + workspace: &mut Workspace, + _: &ActivateOrHideDock, + cx: &mut ViewContext, + ) { + let dock_pane = workspace.dock_pane().clone(); + if dock_pane.read(cx).is_active() { + Self::hide(workspace, cx); + } else { + Self::show(workspace, cx); + cx.focus(dock_pane); + } + } + fn move_dock( workspace: &mut Workspace, &MoveDock(new_anchor): &MoveDock, @@ -178,18 +222,19 @@ impl Dock { .map(|anchor| match anchor { DockAnchor::Bottom | DockAnchor::Right => { let mut panel_style = style.panel.clone(); - let resize_side = if anchor == DockAnchor::Bottom { + let (resize_side, initial_size) = if anchor == DockAnchor::Bottom { panel_style.margin = Margin { top: panel_style.margin.top, ..Default::default() }; - Side::Top + + (Side::Top, style.initial_size_bottom) } else { panel_style.margin = Margin { left: panel_style.margin.left, ..Default::default() }; - Side::Left + (Side::Left, style.initial_size_right) }; enum DockResizeHandle {} @@ -200,7 +245,10 @@ impl Dock { resize_side as usize, resize_side, 4., - self.panel_sizes.get(&anchor).copied().unwrap_or(200.), + self.panel_sizes + .get(&anchor) + .copied() + .unwrap_or(initial_size), cx, ); @@ -216,18 +264,29 @@ impl Dock { resizable.flex(style.flex, false).boxed() } - DockAnchor::Expanded => Container::new( - MouseEventHandler::::new(0, cx, |_state, _cx| { - Container::new(ChildView::new(self.pane.clone()).boxed()) + DockAnchor::Expanded => { + enum ExpandedDockWash {} + enum ExpandedDockPane {} + Container::new( + MouseEventHandler::::new(0, cx, |_state, cx| { + MouseEventHandler::::new(0, cx, |_state, _cx| { + ChildView::new(self.pane.clone()).boxed() + }) + .capture_all() + .contained() .with_style(style.maximized) .boxed() - }) - .capture_all() - .with_cursor_style(CursorStyle::Arrow) - .boxed(), - ) - .with_background_color(style.wash_color) - .boxed(), + }) + .capture_all() + .on_down(MouseButton::Left, |_, cx| { + cx.dispatch_action(ToggleDock); + }) + .with_cursor_style(CursorStyle::Arrow) + .boxed(), + ) + .with_background_color(style.wash_color) + .boxed() + } }) } } diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index a4937cbd970f46f2bf5ca30f3dc7f803c455312e..3d3fb24726a4183a61bb47909ab42a47ed6c11c8 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -279,6 +279,10 @@ impl Pane { } } + pub fn is_active(&self) -> bool { + self.is_active + } + pub fn set_active(&mut self, is_active: bool, cx: &mut ViewContext) { self.is_active = is_active; cx.notify(); @@ -1010,9 +1014,9 @@ impl Pane { action.position, AnchorCorner::TopRight, vec![ - ContextMenuItem::item("Move Dock Right", MoveDock(DockAnchor::Right)), - ContextMenuItem::item("Move Dock Bottom", MoveDock(DockAnchor::Bottom)), - ContextMenuItem::item("Move Dock Maximized", MoveDock(DockAnchor::Expanded)), + ContextMenuItem::item("Anchor Dock Right", MoveDock(DockAnchor::Right)), + ContextMenuItem::item("Anchor Dock Bottom", MoveDock(DockAnchor::Bottom)), + ContextMenuItem::item("Expand Dock", MoveDock(DockAnchor::Expanded)), ], cx, ); @@ -1407,9 +1411,12 @@ impl View for Pane { ) // Add the close dock button if this pane is a dock .with_children(self.docked.map(|_| { - tab_bar_button(3, "icons/x_mark_12.svg", cx, |_| { - ToggleDock - }) + tab_bar_button( + 3, + "icons/x_mark_thin_8.svg", + cx, + |_| ToggleDock, + ) })) .contained() .with_style(theme.workspace.tab_bar.container) @@ -1426,7 +1433,7 @@ impl View for Pane { .flex(1., false) .named("tab bar") }) - .with_child(ChildView::new(&self.toolbar).boxed()) + .with_child(ChildView::new(&self.toolbar).expanded().boxed()) .with_child(ChildView::new(active_item).flex(1., true).boxed()) .boxed() } else { diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 0f3920eae08608276a9f08d21a93551be25213a0..86a61141ad94b5580a9e018c5af884a60ca652e9 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -957,7 +957,6 @@ impl Workspace { .detach(); let center_pane = cx.add_view(|cx| Pane::new(None, cx)); - dbg!(¢er_pane); let pane_id = center_pane.id(); cx.subscribe(¢er_pane, move |this, _, event, cx| { this.handle_pane_event(pane_id, event, cx) @@ -993,7 +992,6 @@ impl Workspace { let dock = Dock::new(cx, dock_default_factory); let dock_pane = dock.pane().clone(); - dbg!(&dock_pane); let left_sidebar = cx.add_view(|_| Sidebar::new(SidebarSide::Left)); let right_sidebar = cx.add_view(|_| Sidebar::new(SidebarSide::Right)); @@ -1490,9 +1488,9 @@ impl Workspace { sidebar.set_open(open, cx); open }); - if open && sidebar_side == SidebarSide::Right && self.dock.is_anchored_at(DockAnchor::Right) - { - Dock::hide(self, cx); + + if open { + Dock::hide_on_sidebar_shown(self, sidebar_side, cx); } cx.focus_self(); @@ -1516,13 +1514,7 @@ impl Workspace { }); if let Some(active_item) = active_item { - // If there is an active item, that means the sidebar was opened, - // which means we need to check if the dock is open and close it - if action.sidebar_side == SidebarSide::Right - && self.dock.is_anchored_at(DockAnchor::Right) - { - Dock::hide(self, cx); - } + Dock::hide_on_sidebar_shown(self, action.sidebar_side, cx); if active_item.is_focused(cx) { cx.focus_self(); @@ -1551,11 +1543,7 @@ impl Workspace { sidebar.active_item().cloned() }); if let Some(active_item) = active_item { - // If there is an active item, that means the sidebar was opened, - // which means we need to check if the dock is open and close it - if sidebar_side == SidebarSide::Right && self.dock.is_anchored_at(DockAnchor::Right) { - Dock::hide(self, cx); - } + Dock::hide_on_sidebar_shown(self, sidebar_side, cx); if active_item.is_focused(cx) { cx.focus_self(); @@ -1726,8 +1714,13 @@ impl Workspace { }); self.active_item_path_changed(cx); - if &pane != self.dock.pane() { + if &pane == self.dock_pane() { + Dock::show(self, cx); + } else { self.last_active_center_pane = Some(pane.clone()); + if self.dock.is_anchored_at(DockAnchor::Expanded) { + Dock::hide(self, cx); + } } cx.notify(); } diff --git a/styles/src/styleTree/search.ts b/styles/src/styleTree/search.ts index 3e9c30d8d9a14b9e2bcbe4e319dc0274a13bd70d..f3555256430ba181312b03333ecb665636416cd1 100644 --- a/styles/src/styleTree/search.ts +++ b/styles/src/styleTree/search.ts @@ -46,6 +46,11 @@ export default function search(theme: Theme) { background: backgroundColor(theme, "on500", "active"), border: border(theme, "muted"), }, + clicked: { + ...text(theme, "mono", "active"), + background: backgroundColor(theme, "on300", "active"), + border: border(theme, "secondary"), + }, hover: { ...text(theme, "mono", "active"), background: backgroundColor(theme, "on500", "hovered"), diff --git a/styles/src/styleTree/workspace.ts b/styles/src/styleTree/workspace.ts index a32334be634b57ceaf555e0bc25df0cbd236d35e..4b279c0360927d85b7bdf7a79bb4b69aaf275232 100644 --- a/styles/src/styleTree/workspace.ts +++ b/styles/src/styleTree/workspace.ts @@ -160,8 +160,8 @@ export default function workspace(theme: Theme) { margin: { right: 10, bottom: 10 }, }, dock: { - initialSizeRight: 240, - initialSizeBottom: 360, + initialSizeRight: 640, + initialSizeBottom: 480, wash_color: withOpacity(theme.backgroundColor[500].base, 0.5), flex: 0.5, panel: {