Enable and disable nav buttons based on pane's navigation stack

Also, make the `NavHistory` type private to the `workspace` crate.
Expose only the `ItemNavHistory` type, via a method on Pane called
`nav_history_for_item`.

crates/editor/src/editor.rs

@@ -4065,13 +4065,16 @@ impl Editor {
                 }
             }
 
-            nav_history.push(Some(NavigationData {
+            nav_history.push(
-                cursor_anchor: position,
+                Some(NavigationData {
-                cursor_position: point,
+                    cursor_anchor: position,
-                scroll_position: self.scroll_position,
+                    cursor_position: point,
-                scroll_top_anchor: self.scroll_top_anchor.clone(),
+                    scroll_position: self.scroll_position,
-                scroll_top_row,
+                    scroll_top_anchor: self.scroll_top_anchor.clone(),
-            }));
+                    scroll_top_row,
+                }),
+                cx,
+            );
         }
     }
 
@@ -4669,7 +4672,7 @@ impl Editor {
         definitions: Vec<LocationLink>,
         cx: &mut ViewContext<Workspace>,
     ) {
-        let nav_history = workspace.active_pane().read(cx).nav_history().clone();
+        let pane = workspace.active_pane().clone();
         for definition in definitions {
             let range = definition
                 .target
@@ -4681,13 +4684,13 @@ impl Editor {
                 // When selecting a definition in a different buffer, disable the nav history
                 // to avoid creating a history entry at the previous cursor location.
                 if editor_handle != target_editor_handle {
-                    nav_history.borrow_mut().disable();
+                    pane.update(cx, |pane, _| pane.disable_history());
                 }
                 target_editor.change_selections(Some(Autoscroll::Center), cx, |s| {
                     s.select_ranges([range]);
                 });
 
-                nav_history.borrow_mut().enable();
+                pane.update(cx, |pane, _| pane.enable_history());
             });
         }
     }
@@ -5641,8 +5644,8 @@ impl Editor {
         editor_handle.update(cx, |editor, cx| {
             editor.push_to_nav_history(editor.selections.newest_anchor().head(), None, cx);
         });
-        let nav_history = workspace.active_pane().read(cx).nav_history().clone();
+        let pane = workspace.active_pane().clone();
-        nav_history.borrow_mut().disable();
+        pane.update(cx, |pane, _| pane.disable_history());
 
         // We defer the pane interaction because we ourselves are a workspace item
         // and activating a new item causes the pane to call a method on us reentrantly,
@@ -5657,7 +5660,7 @@ impl Editor {
                 });
             }
 
-            nav_history.borrow_mut().enable();
+            pane.update(cx, |pane, _| pane.enable_history());
         });
     }
 
@@ -6241,7 +6244,7 @@ mod tests {
         assert_set_eq,
         test::{marked_text_by, marked_text_ranges, marked_text_ranges_by, sample_text},
     };
-    use workspace::{FollowableItem, ItemHandle};
+    use workspace::{FollowableItem, ItemHandle, NavigationEntry, Pane};
 
     #[gpui::test]
     fn test_edit_events(cx: &mut MutableAppContext) {
@@ -6589,12 +6592,20 @@ mod tests {
     fn test_navigation_history(cx: &mut gpui::MutableAppContext) {
         cx.set_global(Settings::test(cx));
         use workspace::Item;
-        let nav_history = Rc::new(RefCell::new(workspace::NavHistory::default()));
+        let pane = cx.add_view(Default::default(), |cx| Pane::new(cx));
         let buffer = MultiBuffer::build_simple(&sample_text(300, 5, 'a'), cx);
 
         cx.add_window(Default::default(), |cx| {
             let mut editor = build_editor(buffer.clone(), cx);
-            editor.nav_history = Some(ItemNavHistory::new(nav_history.clone(), &cx.handle()));
+            let handle = cx.handle();
+            editor.set_nav_history(Some(pane.read(cx).nav_history_for_item(&handle)));
+
+            fn pop_history(
+                editor: &mut Editor,
+                cx: &mut MutableAppContext,
+            ) -> Option<NavigationEntry> {
+                editor.nav_history.as_mut().unwrap().pop_backward(cx)
+            }
 
             // Move the cursor a small distance.
             // Nothing is added to the navigation history.
@@ -6604,21 +6615,21 @@ mod tests {
             editor.change_selections(None, cx, |s| {
                 s.select_display_ranges([DisplayPoint::new(3, 0)..DisplayPoint::new(3, 0)])
             });
-            assert!(nav_history.borrow_mut().pop_backward().is_none());
+            assert!(pop_history(&mut editor, cx).is_none());
 
             // Move the cursor a large distance.
             // The history can jump back to the previous position.
             editor.change_selections(None, cx, |s| {
                 s.select_display_ranges([DisplayPoint::new(13, 0)..DisplayPoint::new(13, 3)])
             });
-            let nav_entry = nav_history.borrow_mut().pop_backward().unwrap();
+            let nav_entry = pop_history(&mut editor, cx).unwrap();
             editor.navigate(nav_entry.data.unwrap(), cx);
             assert_eq!(nav_entry.item.id(), cx.view_id());
             assert_eq!(
                 editor.selections.display_ranges(cx),
                 &[DisplayPoint::new(3, 0)..DisplayPoint::new(3, 0)]
             );
-            assert!(nav_history.borrow_mut().pop_backward().is_none());
+            assert!(pop_history(&mut editor, cx).is_none());
 
             // Move the cursor a small distance via the mouse.
             // Nothing is added to the navigation history.
@@ -6628,7 +6639,7 @@ mod tests {
                 editor.selections.display_ranges(cx),
                 &[DisplayPoint::new(5, 0)..DisplayPoint::new(5, 0)]
             );
-            assert!(nav_history.borrow_mut().pop_backward().is_none());
+            assert!(pop_history(&mut editor, cx).is_none());
 
             // Move the cursor a large distance via the mouse.
             // The history can jump back to the previous position.
@@ -6638,14 +6649,14 @@ mod tests {
                 editor.selections.display_ranges(cx),
                 &[DisplayPoint::new(15, 0)..DisplayPoint::new(15, 0)]
             );
-            let nav_entry = nav_history.borrow_mut().pop_backward().unwrap();
+            let nav_entry = pop_history(&mut editor, cx).unwrap();
             editor.navigate(nav_entry.data.unwrap(), cx);
             assert_eq!(nav_entry.item.id(), cx.view_id());
             assert_eq!(
                 editor.selections.display_ranges(cx),
                 &[DisplayPoint::new(5, 0)..DisplayPoint::new(5, 0)]
             );
-            assert!(nav_history.borrow_mut().pop_backward().is_none());
+            assert!(pop_history(&mut editor, cx).is_none());
 
             // Set scroll position to check later
             editor.set_scroll_position(Vector2F::new(5.5, 5.5), cx);
@@ -6658,7 +6669,7 @@ mod tests {
             assert_ne!(editor.scroll_position, original_scroll_position);
             assert_ne!(editor.scroll_top_anchor, original_scroll_top_anchor);
 
-            let nav_entry = nav_history.borrow_mut().pop_backward().unwrap();
+            let nav_entry = pop_history(&mut editor, cx).unwrap();
             editor.navigate(nav_entry.data.unwrap(), cx);
             assert_eq!(editor.scroll_position, original_scroll_position);
             assert_eq!(editor.scroll_top_anchor, original_scroll_top_anchor);

crates/theme/src/theme.rs

@@ -510,28 +510,23 @@ pub struct Interactive<T> {
     pub default: T,
     pub hover: Option<T>,
     pub active: Option<T>,
-    pub active_hover: Option<T>,
+    pub disabled: Option<T>,
 }
 
 impl<T> Interactive<T> {
     pub fn style_for(&self, state: MouseState, active: bool) -> &T {
         if active {
-            if state.hovered {
+            self.active.as_ref().unwrap_or(&self.default)
-                self.active_hover
+        } else if state.hovered {
-                    .as_ref()
+            self.hover.as_ref().unwrap_or(&self.default)
-                    .or(self.active.as_ref())
-                    .unwrap_or(&self.default)
-            } else {
-                self.active.as_ref().unwrap_or(&self.default)
-            }
         } else {
-            if state.hovered {
+            &self.default
-                self.hover.as_ref().unwrap_or(&self.default)
-            } else {
-                &self.default
-            }
         }
     }
+
+    pub fn disabled_style(&self) -> &T {
+        self.disabled.as_ref().unwrap_or(&self.default)
+    }
 }
 
 impl<'de, T: DeserializeOwned> Deserialize<'de> for Interactive<T> {
@@ -545,7 +540,7 @@ impl<'de, T: DeserializeOwned> Deserialize<'de> for Interactive<T> {
             default: Value,
             hover: Option<Value>,
             active: Option<Value>,
-            active_hover: Option<Value>,
+            disabled: Option<Value>,
         }
 
         let json = Helper::deserialize(deserializer)?;
@@ -571,14 +566,14 @@ impl<'de, T: DeserializeOwned> Deserialize<'de> for Interactive<T> {
 
         let hover = deserialize_state(json.hover)?;
         let active = deserialize_state(json.active)?;
-        let active_hover = deserialize_state(json.active_hover)?;
+        let disabled = deserialize_state(json.disabled)?;
         let default = serde_json::from_value(json.default).map_err(serde::de::Error::custom)?;
 
         Ok(Interactive {
             default,
             hover,
             active,
-            active_hover,
+            disabled,
         })
     }
 }

crates/workspace/src/pane.rs

@@ -136,13 +136,13 @@ pub struct ItemNavHistory {
     item: Rc<dyn WeakItemHandle>,
 }
 
-#[derive(Default)]
+struct NavHistory {
-pub struct NavHistory {
     mode: NavigationMode,
     backward_stack: VecDeque<NavigationEntry>,
     forward_stack: VecDeque<NavigationEntry>,
     closed_stack: VecDeque<NavigationEntry>,
     paths_by_item: HashMap<usize, ProjectPath>,
+    pane: WeakViewHandle<Pane>,
 }
 
 #[derive(Copy, Clone)]
@@ -173,13 +173,23 @@ impl Pane {
             items: Vec::new(),
             active_item_index: 0,
             autoscroll: false,
-            nav_history: Default::default(),
+            nav_history: Rc::new(RefCell::new(NavHistory {
+                mode: NavigationMode::Normal,
+                backward_stack: Default::default(),
+                forward_stack: Default::default(),
+                closed_stack: Default::default(),
+                paths_by_item: Default::default(),
+                pane: handle.clone(),
+            })),
             toolbar: cx.add_view(|_| Toolbar::new(handle)),
         }
     }
 
-    pub fn nav_history(&self) -> &Rc<RefCell<NavHistory>> {
+    pub fn nav_history_for_item<T: Item>(&self, item: &ViewHandle<T>) -> ItemNavHistory {
-        &self.nav_history
+        ItemNavHistory {
+            history: self.nav_history.clone(),
+            item: Rc::new(item.downgrade()),
+        }
     }
 
     pub fn activate(&self, cx: &mut ViewContext<Self>) {
@@ -224,6 +234,26 @@ impl Pane {
         )
     }
 
+    pub fn disable_history(&mut self) {
+        self.nav_history.borrow_mut().disable();
+    }
+
+    pub fn enable_history(&mut self) {
+        self.nav_history.borrow_mut().enable();
+    }
+
+    pub fn can_navigate_backward(&self) -> bool {
+        !self.nav_history.borrow().backward_stack.is_empty()
+    }
+
+    pub fn can_navigate_forward(&self) -> bool {
+        !self.nav_history.borrow().forward_stack.is_empty()
+    }
+
+    fn history_updated(&mut self, cx: &mut ViewContext<Self>) {
+        self.toolbar.update(cx, |_, cx| cx.notify());
+    }
+
     fn navigate_history(
         workspace: &mut Workspace,
         pane: ViewHandle<Pane>,
@@ -235,7 +265,7 @@ impl Pane {
         let to_load = pane.update(cx, |pane, cx| {
             loop {
                 // Retrieve the weak item handle from the history.
-                let entry = pane.nav_history.borrow_mut().pop(mode)?;
+                let entry = pane.nav_history.borrow_mut().pop(mode, cx)?;
 
                 // If the item is still present in this pane, then activate it.
                 if let Some(index) = entry
@@ -368,7 +398,6 @@ impl Pane {
             return;
         }
 
-        item.set_nav_history(pane.read(cx).nav_history.clone(), cx);
         item.added_to_pane(workspace, pane.clone(), cx);
         pane.update(cx, |pane, cx| {
             // If there is already an active item, then insert the new item
@@ -626,11 +655,16 @@ impl Pane {
                             .borrow_mut()
                             .set_mode(NavigationMode::Normal);
 
-                        let mut nav_history = pane.nav_history().borrow_mut();
                         if let Some(path) = item.project_path(cx) {
-                            nav_history.paths_by_item.insert(item.id(), path);
+                            pane.nav_history
+                                .borrow_mut()
+                                .paths_by_item
+                                .insert(item.id(), path);
                         } else {
-                            nav_history.paths_by_item.remove(&item.id());
+                            pane.nav_history
+                                .borrow_mut()
+                                .paths_by_item
+                                .remove(&item.id());
                         }
                     }
                 });
@@ -954,57 +988,56 @@ impl View for Pane {
 }
 
 impl ItemNavHistory {
-    pub fn new<T: Item>(history: Rc<RefCell<NavHistory>>, item: &ViewHandle<T>) -> Self {
+    pub fn push<D: 'static + Any>(&self, data: Option<D>, cx: &mut MutableAppContext) {
-        Self {
+        self.history.borrow_mut().push(data, self.item.clone(), cx);
-            history,
-            item: Rc::new(item.downgrade()),
-        }
     }
 
-    pub fn history(&self) -> Rc<RefCell<NavHistory>> {
+    pub fn pop_backward(&self, cx: &mut MutableAppContext) -> Option<NavigationEntry> {
-        self.history.clone()
+        self.history.borrow_mut().pop(NavigationMode::GoingBack, cx)
     }
 
-    pub fn push<D: 'static + Any>(&self, data: Option<D>) {
+    pub fn pop_forward(&self, cx: &mut MutableAppContext) -> Option<NavigationEntry> {
-        self.history.borrow_mut().push(data, self.item.clone());
+        self.history
+            .borrow_mut()
+            .pop(NavigationMode::GoingForward, cx)
     }
 }
 
 impl NavHistory {
-    pub fn disable(&mut self) {
-        self.mode = NavigationMode::Disabled;
-    }
-
-    pub fn enable(&mut self) {
-        self.mode = NavigationMode::Normal;
-    }
-
-    pub fn pop_backward(&mut self) -> Option<NavigationEntry> {
-        self.backward_stack.pop_back()
-    }
-
-    pub fn pop_forward(&mut self) -> Option<NavigationEntry> {
-        self.forward_stack.pop_back()
-    }
-
-    pub fn pop_closed(&mut self) -> Option<NavigationEntry> {
-        self.closed_stack.pop_back()
-    }
-
-    fn pop(&mut self, mode: NavigationMode) -> Option<NavigationEntry> {
-        match mode {
-            NavigationMode::Normal | NavigationMode::Disabled | NavigationMode::ClosingItem => None,
-            NavigationMode::GoingBack => self.pop_backward(),
-            NavigationMode::GoingForward => self.pop_forward(),
-            NavigationMode::ReopeningClosedItem => self.pop_closed(),
-        }
-    }
-
     fn set_mode(&mut self, mode: NavigationMode) {
         self.mode = mode;
     }
 
-    pub fn push<D: 'static + Any>(&mut self, data: Option<D>, item: Rc<dyn WeakItemHandle>) {
+    fn disable(&mut self) {
+        self.mode = NavigationMode::Disabled;
+    }
+
+    fn enable(&mut self) {
+        self.mode = NavigationMode::Normal;
+    }
+
+    fn pop(&mut self, mode: NavigationMode, cx: &mut MutableAppContext) -> Option<NavigationEntry> {
+        let entry = match mode {
+            NavigationMode::Normal | NavigationMode::Disabled | NavigationMode::ClosingItem => {
+                return None
+            }
+            NavigationMode::GoingBack => &mut self.backward_stack,
+            NavigationMode::GoingForward => &mut self.forward_stack,
+            NavigationMode::ReopeningClosedItem => &mut self.closed_stack,
+        }
+        .pop_back();
+        if entry.is_some() {
+            self.did_update(cx);
+        }
+        entry
+    }
+
+    fn push<D: 'static + Any>(
+        &mut self,
+        data: Option<D>,
+        item: Rc<dyn WeakItemHandle>,
+        cx: &mut MutableAppContext,
+    ) {
         match self.mode {
             NavigationMode::Disabled => {}
             NavigationMode::Normal | NavigationMode::ReopeningClosedItem => {
@@ -1045,5 +1078,12 @@ impl NavHistory {
                 });
             }
         }
+        self.did_update(cx);
+    }
+
+    fn did_update(&self, cx: &mut MutableAppContext) {
+        if let Some(pane) = self.pane.upgrade(cx) {
+            cx.defer(move |cx| pane.update(cx, |pane, cx| pane.history_updated(cx)));
+        }
     }
 }

crates/workspace/src/toolbar.rs

@@ -101,6 +101,14 @@ impl View for Toolbar {
         }
 
         let pane = self.pane.clone();
+        let mut enable_go_backward = false;
+        let mut enable_go_forward = false;
+        if let Some(pane) = pane.upgrade(cx) {
+            let pane = pane.read(cx);
+            enable_go_backward = pane.can_navigate_backward();
+            enable_go_forward = pane.can_navigate_forward();
+        }
+
         let container_style = theme.container;
         let height = theme.height;
         let button_style = theme.nav_button;
@@ -111,6 +119,7 @@ impl View for Toolbar {
                     .with_child(nav_button(
                         "icons/arrow-left.svg",
                         button_style,
+                        enable_go_backward,
                         spacing,
                         super::GoBack {
                             pane: Some(pane.clone()),
@@ -120,6 +129,7 @@ impl View for Toolbar {
                     .with_child(nav_button(
                         "icons/arrow-right.svg",
                         button_style,
+                        enable_go_forward,
                         spacing,
                         super::GoForward {
                             pane: Some(pane.clone()),
@@ -142,12 +152,17 @@ impl View for Toolbar {
 fn nav_button<A: Action + Clone>(
     svg_path: &'static str,
     style: theme::Interactive<theme::IconButton>,
+    enabled: bool,
     spacing: f32,
     action: A,
     cx: &mut RenderContext<Toolbar>,
 ) -> ElementBox {
     MouseEventHandler::new::<A, _, _>(0, cx, |state, _| {
-        let style = style.style_for(state, false);
+        let style = if enabled {
+            style.style_for(state, false)
+        } else {
+            style.disabled_style()
+        };
         Svg::new(svg_path)
             .with_color(style.color)
             .constrained()
@@ -160,7 +175,11 @@ fn nav_button<A: Action + Clone>(
             .with_height(style.button_width)
             .boxed()
     })
-    .with_cursor_style(CursorStyle::PointingHand)
+    .with_cursor_style(if enabled {
+        CursorStyle::PointingHand
+    } else {
+        CursorStyle::default()
+    })
     .on_mouse_down(move |_, cx| cx.dispatch_action(action.clone()))
     .contained()
     .with_margin_right(spacing)

crates/workspace/src/workspace.rs

@@ -414,7 +414,6 @@ pub trait ItemHandle: 'static + fmt::Debug {
     fn project_entry_ids(&self, cx: &AppContext) -> SmallVec<[ProjectEntryId; 3]>;
     fn is_singleton(&self, cx: &AppContext) -> bool;
     fn boxed_clone(&self) -> Box<dyn ItemHandle>;
-    fn set_nav_history(&self, nav_history: Rc<RefCell<NavHistory>>, cx: &mut MutableAppContext);
     fn clone_on_split(&self, cx: &mut MutableAppContext) -> Option<Box<dyn ItemHandle>>;
     fn added_to_pane(
         &self,
@@ -484,12 +483,6 @@ impl<T: Item> ItemHandle for ViewHandle<T> {
         Box::new(self.clone())
     }
 
-    fn set_nav_history(&self, nav_history: Rc<RefCell<NavHistory>>, cx: &mut MutableAppContext) {
-        self.update(cx, |item, cx| {
-            item.set_nav_history(ItemNavHistory::new(nav_history, &cx.handle()), cx);
-        })
-    }
-
     fn clone_on_split(&self, cx: &mut MutableAppContext) -> Option<Box<dyn ItemHandle>> {
         self.update(cx, |item, cx| {
             cx.add_option_view(|cx| item.clone_on_split(cx))
@@ -503,6 +496,9 @@ impl<T: Item> ItemHandle for ViewHandle<T> {
         pane: ViewHandle<Pane>,
         cx: &mut ViewContext<Workspace>,
     ) {
+        let history = pane.read(cx).nav_history_for_item(self);
+        self.update(cx, |this, cx| this.set_nav_history(history, cx));
+
         if let Some(followed_item) = self.to_followable_item_handle(cx) {
             if let Some(message) = followed_item.to_state_proto(cx) {
                 workspace.update_followers(

styles/src/styleTree/workspace.ts

@@ -140,13 +140,15 @@ export default function workspace(theme: Theme) {
       border: border(theme, "secondary", { bottom: true }),
       itemSpacing: 8,
       navButton: {
-        color: iconColor(theme, "secondary"),
+        color: iconColor(theme, "primary"),
         iconWidth: 8,
         buttonWidth: 12,
-        margin: { left: 8, right: 8 },
         hover: {
           color: iconColor(theme, "active"),
         },
+        disabled: {
+          color: iconColor(theme, "muted")
+        }
       },
       padding: { left: 16, right: 8, top: 4, bottom: 4 },
     },