Minor improvements to dock visuals, rework dock keybindings, and fix panic on split when dock is active

K Simmons created

Change summary

assets/keymaps/default.json             |  15 ++
crates/terminal/src/terminal_element.rs |  51 +++++-----
crates/theme/src/theme.rs               |   2 
crates/workspace/src/dock.rs            | 133 ++++++++++++++++----------
crates/workspace/src/pane.rs            |  25 +++-
crates/workspace/src/workspace.rs       |  15 ++
styles/src/styleTree/tabBar.ts          |  19 +--
styles/src/styleTree/workspace.ts       |  11 +
8 files changed, 163 insertions(+), 108 deletions(-)

Detailed changes

assets/keymaps/default.json 🔗

@@ -309,8 +309,7 @@
             "cmd-shift-p": "command_palette::Toggle",
             "cmd-shift-m": "diagnostics::Deploy",
             "cmd-shift-e": "project_panel::ToggleFocus",
-            "cmd-alt-s": "workspace::SaveAll",
-            "shift-escape": "workspace::ActivateOrHideDock"
+            "cmd-alt-s": "workspace::SaveAll"
         }
     },
     // Bindings from Sublime Text
@@ -395,7 +394,17 @@
         "context": "Workspace",
         "bindings": {
             "cmd-shift-c": "contacts_panel::ToggleFocus",
-            "cmd-shift-b": "workspace::ToggleRightSidebar"
+            "cmd-shift-b": "workspace::ToggleRightSidebar",
+            "shift-escape": "dock::FocusDock",
+            "cmd-shift-k cmd-shift-right": "dock::AnchorDockRight",
+            "cmd-shift-k cmd-shift-down": "dock::AnchorDockBottom",
+            "cmd-shift-k cmd-shift-up": "dock::ExpandDock"
+        }
+    },
+    {
+        "context": "Dock",
+        "bindings": {
+            "shift-escape": "dock::HideDock"
         }
     },
     {

crates/terminal/src/terminal_element.rs 🔗

@@ -755,35 +755,34 @@ impl Element for TerminalElement {
         _paint: &mut Self::PaintState,
         cx: &mut gpui::EventContext,
     ) -> bool {
-        match event {
-            Event::KeyDown(KeyDownEvent { keystroke, .. }) => {
-                if !cx.is_parent_view_focused() {
-                    return false;
-                }
+        if let Event::KeyDown(KeyDownEvent { keystroke, .. }) = event {
+            if !cx.is_parent_view_focused() {
+                return false;
+            }
 
-                if let Some(view) = self.view.upgrade(cx.app) {
-                    view.update(cx.app, |view, cx| {
-                        view.clear_bel(cx);
-                        view.pause_cursor_blinking(cx);
-                    })
-                }
+            if let Some(view) = self.view.upgrade(cx.app) {
+                view.update(cx.app, |view, cx| {
+                    view.clear_bel(cx);
+                    view.pause_cursor_blinking(cx);
+                })
+            }
 
-                self.terminal
-                    .upgrade(cx.app)
-                    .map(|model_handle| {
-                        model_handle.update(cx.app, |term, cx| {
-                            term.try_keystroke(
-                                keystroke,
-                                cx.global::<Settings>()
-                                    .terminal_overrides
-                                    .option_as_meta
-                                    .unwrap_or(false),
-                            )
-                        })
+            self.terminal
+                .upgrade(cx.app)
+                .map(|model_handle| {
+                    model_handle.update(cx.app, |term, cx| {
+                        term.try_keystroke(
+                            keystroke,
+                            cx.global::<Settings>()
+                                .terminal_overrides
+                                .option_as_meta
+                                .unwrap_or(false),
+                        )
                     })
-                    .unwrap_or(false)
-            }
-            _ => false,
+                })
+                .unwrap_or(false)
+        } else {
+            false
         }
     }
 

crates/theme/src/theme.rs 🔗

@@ -80,6 +80,7 @@ pub struct TabBar {
     #[serde(flatten)]
     pub container: ContainerStyle,
     pub pane_button: Interactive<IconButton>,
+    pub pane_button_container: ContainerStyle,
     pub active_pane: TabStyles,
     pub inactive_pane: TabStyles,
     pub dragged_tab: Tab,
@@ -155,7 +156,6 @@ pub struct Dock {
     pub initial_size_right: f32,
     pub initial_size_bottom: f32,
     pub wash_color: Color,
-    pub flex: f32,
     pub panel: ContainerStyle,
     pub maximized: ContainerStyle,
 }

crates/workspace/src/dock.rs 🔗

@@ -1,8 +1,8 @@
 use collections::HashMap;
 use gpui::{
     actions,
-    elements::{ChildView, Container, Empty, Margin, MouseEventHandler, Side, Svg},
-    impl_internal_actions, CursorStyle, Element, ElementBox, Entity, MouseButton,
+    elements::{ChildView, Container, Empty, MouseEventHandler, Side, Svg},
+    impl_internal_actions, Border, CursorStyle, Element, ElementBox, Entity, MouseButton,
     MutableAppContext, RenderContext, View, ViewContext, ViewHandle, WeakViewHandle,
 };
 use serde::Deserialize;
@@ -17,13 +17,37 @@ pub struct MoveDock(pub DockAnchor);
 #[derive(PartialEq, Clone)]
 pub struct AddDefaultItemToDock;
 
-actions!(workspace, [ToggleDock, ActivateOrHideDock]);
-impl_internal_actions!(workspace, [MoveDock, AddDefaultItemToDock]);
+actions!(
+    dock,
+    [
+        FocusDock,
+        HideDock,
+        AnchorDockRight,
+        AnchorDockBottom,
+        ExpandDock
+    ]
+);
+impl_internal_actions!(dock, [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::focus_dock);
+    cx.add_action(Dock::hide_dock);
     cx.add_action(Dock::move_dock);
+    cx.add_action(
+        |workspace: &mut Workspace, _: &AnchorDockRight, cx: &mut ViewContext<Workspace>| {
+            Dock::move_dock(workspace, &MoveDock(DockAnchor::Right), cx)
+        },
+    );
+    cx.add_action(
+        |workspace: &mut Workspace, _: &AnchorDockBottom, cx: &mut ViewContext<Workspace>| {
+            Dock::move_dock(workspace, &MoveDock(DockAnchor::Bottom), cx)
+        },
+    );
+    cx.add_action(
+        |workspace: &mut Workspace, _: &ExpandDock, cx: &mut ViewContext<Workspace>| {
+            Dock::move_dock(workspace, &MoveDock(DockAnchor::Expanded), cx)
+        },
+    );
 }
 
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
@@ -60,13 +84,6 @@ impl DockPosition {
         }
     }
 
-    fn toggle(self) -> Self {
-        match self {
-            DockPosition::Shown(anchor) => DockPosition::Hidden(anchor),
-            DockPosition::Hidden(anchor) => DockPosition::Shown(anchor),
-        }
-    }
-
     fn hide(self) -> Self {
         match self {
             DockPosition::Shown(anchor) => DockPosition::Hidden(anchor),
@@ -130,10 +147,6 @@ impl Dock {
         new_position: DockPosition,
         cx: &mut ViewContext<Workspace>,
     ) {
-        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| {
@@ -184,22 +197,12 @@ impl Dock {
         }
     }
 
-    fn toggle(workspace: &mut Workspace, _: &ToggleDock, cx: &mut ViewContext<Workspace>) {
-        Self::set_dock_position(workspace, workspace.dock.position.toggle(), cx);
+    fn focus_dock(workspace: &mut Workspace, _: &FocusDock, cx: &mut ViewContext<Workspace>) {
+        Self::set_dock_position(workspace, workspace.dock.position.show(), cx);
     }
 
-    fn activate_or_hide_dock(
-        workspace: &mut Workspace,
-        _: &ActivateOrHideDock,
-        cx: &mut ViewContext<Workspace>,
-    ) {
-        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 hide_dock(workspace: &mut Workspace, _: &HideDock, cx: &mut ViewContext<Workspace>) {
+        Self::set_dock_position(workspace, workspace.dock.position.hide(), cx);
     }
 
     fn move_dock(
@@ -226,16 +229,22 @@ impl Dock {
                 DockAnchor::Bottom | DockAnchor::Right => {
                     let mut panel_style = style.panel.clone();
                     let (resize_side, initial_size) = if anchor == DockAnchor::Bottom {
-                        panel_style.margin = Margin {
-                            top: panel_style.margin.top,
-                            ..Default::default()
+                        panel_style.border = Border {
+                            top: true,
+                            bottom: false,
+                            left: false,
+                            right: false,
+                            ..panel_style.border
                         };
 
                         (Side::Top, style.initial_size_bottom)
                     } else {
-                        panel_style.margin = Margin {
-                            left: panel_style.margin.left,
-                            ..Default::default()
+                        panel_style.border = Border {
+                            top: false,
+                            bottom: false,
+                            left: true,
+                            right: false,
+                            ..panel_style.border
                         };
                         (Side::Left, style.initial_size_right)
                     };
@@ -265,7 +274,7 @@ impl Dock {
                         }
                     });
 
-                    resizable.flex(style.flex, false).boxed()
+                    resizable.flex(5., false).boxed()
                 }
                 DockAnchor::Expanded => {
                     enum ExpandedDockWash {}
@@ -282,7 +291,7 @@ impl Dock {
                         })
                         .capture_all()
                         .on_down(MouseButton::Left, |_, cx| {
-                            cx.dispatch_action(ToggleDock);
+                            cx.dispatch_action(HideDock);
                         })
                         .with_cursor_style(CursorStyle::Arrow)
                         .boxed(),
@@ -328,7 +337,7 @@ impl View for ToggleDockButton {
         let dock_position = workspace.unwrap().read(cx).dock.position;
 
         let theme = cx.global::<Settings>().theme.clone();
-        MouseEventHandler::<Self>::new(0, cx, {
+        let button = MouseEventHandler::<Self>::new(0, cx, {
             let theme = theme.clone();
             move |state, _| {
                 let style = theme
@@ -348,17 +357,33 @@ impl View for ToggleDockButton {
                     .boxed()
             }
         })
-        .with_cursor_style(CursorStyle::PointingHand)
-        .on_click(MouseButton::Left, |_, cx| {
-            cx.dispatch_action(ToggleDock);
-        })
-        .with_tooltip::<Self, _>(
-            0,
-            "Toggle Dock".to_string(),
-            Some(Box::new(ToggleDock)),
-            theme.tooltip.clone(),
-            cx,
-        )
+        .with_cursor_style(CursorStyle::PointingHand);
+
+        if dock_position.is_visible() {
+            button
+                .on_click(MouseButton::Left, |_, cx| {
+                    cx.dispatch_action(HideDock);
+                })
+                .with_tooltip::<Self, _>(
+                    0,
+                    "Hide Dock".into(),
+                    Some(Box::new(HideDock)),
+                    theme.tooltip.clone(),
+                    cx,
+                )
+        } else {
+            button
+                .on_click(MouseButton::Left, |_, cx| {
+                    cx.dispatch_action(FocusDock);
+                })
+                .with_tooltip::<Self, _>(
+                    0,
+                    "Focus Dock".into(),
+                    Some(Box::new(FocusDock)),
+                    theme.tooltip.clone(),
+                    cx,
+                )
+        }
         .boxed()
     }
 }
@@ -478,7 +503,7 @@ mod tests {
 
         cx.move_dock(DockAnchor::Right);
         cx.assert_dock_pane_active();
-        cx.toggle_dock();
+        cx.hide_dock();
         cx.move_dock(DockAnchor::Right);
         cx.assert_dock_pane_active();
     }
@@ -600,8 +625,8 @@ mod tests {
             self.cx.dispatch_action(self.window_id, MoveDock(anchor));
         }
 
-        pub fn toggle_dock(&self) {
-            self.cx.dispatch_action(self.window_id, ToggleDock);
+        pub fn hide_dock(&self) {
+            self.cx.dispatch_action(self.window_id, HideDock);
         }
 
         pub fn open_sidebar(&mut self, sidebar_side: SidebarSide) {

crates/workspace/src/pane.rs 🔗

@@ -1,6 +1,6 @@
 use super::{ItemHandle, SplitDirection};
 use crate::{
-    dock::{icon_for_dock_anchor, MoveDock, ToggleDock},
+    dock::{icon_for_dock_anchor, AnchorDockBottom, AnchorDockRight, ExpandDock, HideDock},
     toolbar::Toolbar,
     Item, NewFile, NewSearch, NewTerminal, WeakItemHandle, Workspace,
 };
@@ -1014,9 +1014,9 @@ impl Pane {
                 action.position,
                 AnchorCorner::TopRight,
                 vec![
-                    ContextMenuItem::item("Anchor Dock Right", MoveDock(DockAnchor::Right)),
-                    ContextMenuItem::item("Anchor Dock Bottom", MoveDock(DockAnchor::Bottom)),
-                    ContextMenuItem::item("Expand Dock", MoveDock(DockAnchor::Expanded)),
+                    ContextMenuItem::item("Anchor Dock Right", AnchorDockRight),
+                    ContextMenuItem::item("Anchor Dock Bottom", AnchorDockBottom),
+                    ContextMenuItem::item("Expand Dock", ExpandDock),
                 ],
                 cx,
             );
@@ -1095,6 +1095,7 @@ impl Pane {
                             Self::render_tab(
                                 &item,
                                 pane,
+                                ix == 0,
                                 detail,
                                 hovered,
                                 Self::tab_overlay_color(hovered, theme.as_ref(), cx),
@@ -1139,6 +1140,7 @@ impl Pane {
                                 Self::render_tab(
                                     &dragged_item.item,
                                     dragged_item.pane.clone(),
+                                    false,
                                     detail,
                                     false,
                                     None,
@@ -1220,6 +1222,7 @@ impl Pane {
     fn render_tab<V: View>(
         item: &Box<dyn ItemHandle>,
         pane: WeakViewHandle<Pane>,
+        first: bool,
         detail: Option<usize>,
         hovered: bool,
         overlay: Option<Color>,
@@ -1227,6 +1230,10 @@ impl Pane {
         cx: &mut RenderContext<V>,
     ) -> ElementBox {
         let title = item.tab_content(detail, &tab_style, cx);
+        let mut container = tab_style.container.clone();
+        if first {
+            container.border.left = false;
+        }
 
         let mut tab = Flex::row()
             .with_child(
@@ -1307,7 +1314,7 @@ impl Pane {
                 .boxed(),
             )
             .contained()
-            .with_style(tab_style.container);
+            .with_style(container);
 
         if let Some(overlay) = overlay {
             tab = tab.with_overlay_color(overlay);
@@ -1405,7 +1412,7 @@ impl View for Pane {
                                                             2,
                                                             "icons/split_12.svg",
                                                             cx,
-                                                            |position| DeployNewMenu { position },
+                                                            |position| DeploySplitMenu { position },
                                                         )
                                                     }),
                                             )
@@ -1415,11 +1422,13 @@ impl View for Pane {
                                                     3,
                                                     "icons/x_mark_thin_8.svg",
                                                     cx,
-                                                    |_| ToggleDock,
+                                                    |_| HideDock,
                                                 )
                                             }))
                                             .contained()
-                                            .with_style(theme.workspace.tab_bar.container)
+                                            .with_style(
+                                                theme.workspace.tab_bar.pane_button_container,
+                                            )
                                             .flex(1., false)
                                             .boxed(),
                                     )

crates/workspace/src/workspace.rs 🔗

@@ -34,7 +34,7 @@ use gpui::{
     RenderContext, Task, View, ViewContext, ViewHandle, WeakViewHandle,
 };
 use language::LanguageRegistry;
-use log::error;
+use log::{error, warn};
 pub use pane::*;
 pub use pane_group::*;
 use postage::prelude::Stream;
@@ -1784,6 +1784,11 @@ impl Workspace {
         direction: SplitDirection,
         cx: &mut ViewContext<Self>,
     ) -> Option<ViewHandle<Pane>> {
+        if &pane == self.dock_pane() {
+            warn!("Can't split dock pane.");
+            return None;
+        }
+
         pane.read(cx).active_item().map(|item| {
             let new_pane = self.add_pane(cx);
             if let Some(clone) = item.clone_on_split(cx.as_mut()) {
@@ -2680,6 +2685,14 @@ impl View for Workspace {
             cx.focus(&self.active_pane);
         }
     }
+
+    fn keymap_context(&self, _: &AppContext) -> gpui::keymap::Context {
+        let mut keymap = Self::default_keymap_context();
+        if self.active_pane() == self.dock_pane() {
+            keymap.set.insert("Dock".into());
+        }
+        keymap
+    }
 }
 
 pub trait WorkspaceHandle {

styles/src/styleTree/tabBar.ts 🔗

@@ -59,13 +59,7 @@ export default function tabBar(theme: Theme) {
   const draggedTab = {
     ...activePaneActiveTab,
     background: withOpacity(tab.background, 0.8),
-    border: {
-      ...tab.border,
-      top: false,
-      left: false,
-      right: false,
-      bottom: false,
-    },
+    border: undefined as any, // Remove border
     shadow: draggedShadow(theme),
   }
 
@@ -74,7 +68,6 @@ export default function tabBar(theme: Theme) {
     background: backgroundColor(theme, 300),
     dropTargetOverlayColor: withOpacity(theme.textColor.muted, 0.6),
     border: border(theme, "primary", {
-      left: true,
       bottom: true,
       overlay: true,
     }),
@@ -89,14 +82,18 @@ export default function tabBar(theme: Theme) {
     draggedTab,
     paneButton: {
       color: iconColor(theme, "secondary"),
-      border: {
-        ...tab.border,
-      },
       iconWidth: 12,
       buttonWidth: activePaneActiveTab.height,
       hover: {
         color: iconColor(theme, "active"),
       },
     },
+    paneButtonContainer: {
+      background: tab.background,
+      border: {
+        ...tab.border,
+        right: false,
+      }
+    }
   }
 }

styles/src/styleTree/workspace.ts 🔗

@@ -163,13 +163,16 @@ export default function workspace(theme: Theme) {
       initialSizeRight: 640,
       initialSizeBottom: 480,
       wash_color: withOpacity(theme.backgroundColor[500].base, 0.5),
-      flex: 0.5,
       panel: {
-        margin: 4,
+        border: {
+          ...border(theme, "secondary"),
+          width: 1
+        },
       },
       maximized: {
-        margin: 32,
-        border: border(theme, "secondary"),
+        cornerRadius: 10,
+        margin: 24,
+        border: border(theme, "secondary", { "overlay": true }),
         shadow: modalShadow(theme),
       }
     }