From 7e5cf6669ff985c96f1824a58e3e7cb2cd237021 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 6 Jul 2022 14:05:24 -0700 Subject: [PATCH 1/4] Add forward and backward navigation buttons to toolbar --- assets/icons/arrow-left.svg | 3 ++ assets/icons/arrow-right.svg | 3 ++ crates/theme/src/theme.rs | 1 + crates/workspace/src/pane.rs | 3 +- crates/workspace/src/toolbar.rs | 70 +++++++++++++++++++++++++++---- styles/src/styleTree/workspace.ts | 9 ++++ 6 files changed, 80 insertions(+), 9 deletions(-) create mode 100644 assets/icons/arrow-left.svg create mode 100644 assets/icons/arrow-right.svg diff --git a/assets/icons/arrow-left.svg b/assets/icons/arrow-left.svg new file mode 100644 index 0000000000000000000000000000000000000000..904fdaa1a73221efda6946614db7c707dc74c2ed --- /dev/null +++ b/assets/icons/arrow-left.svg @@ -0,0 +1,3 @@ + + + diff --git a/assets/icons/arrow-right.svg b/assets/icons/arrow-right.svg new file mode 100644 index 0000000000000000000000000000000000000000..b7e1bec6d8a0830b8ebfcf677cd1a5b3480ce5b0 --- /dev/null +++ b/assets/icons/arrow-right.svg @@ -0,0 +1,3 @@ + + + diff --git a/crates/theme/src/theme.rs b/crates/theme/src/theme.rs index 184b1880f0c0a05cc581059b80766b3d282cc9cc..c3052ff6c5afc0a7a5acde8135f555e7113ab6c1 100644 --- a/crates/theme/src/theme.rs +++ b/crates/theme/src/theme.rs @@ -108,6 +108,7 @@ pub struct Toolbar { pub container: ContainerStyle, pub height: f32, pub item_spacing: f32, + pub nav_button: Interactive, } #[derive(Clone, Deserialize, Default)] diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index bbc086395b12a4fd09d7c8ce07e6c52c6491cb53..0fb5225cc533266753c4b84acf225373ce0c6cf0 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -168,12 +168,13 @@ pub struct NavigationEntry { impl Pane { pub fn new(cx: &mut ViewContext) -> Self { + let handle = cx.weak_handle(); Self { items: Vec::new(), active_item_index: 0, autoscroll: false, nav_history: Default::default(), - toolbar: cx.add_view(|_| Toolbar::new()), + toolbar: cx.add_view(|_| Toolbar::new(handle)), } } diff --git a/crates/workspace/src/toolbar.rs b/crates/workspace/src/toolbar.rs index e9b20bf3a04e5876a7a6e202f06b3b78c8727a68..9d8274288be5b6e5a4d8a8a26287da028f347768 100644 --- a/crates/workspace/src/toolbar.rs +++ b/crates/workspace/src/toolbar.rs @@ -1,7 +1,7 @@ -use crate::ItemHandle; +use crate::{ItemHandle, Pane}; use gpui::{ - elements::*, AnyViewHandle, AppContext, ElementBox, Entity, MutableAppContext, RenderContext, - View, ViewContext, ViewHandle, + elements::*, platform::CursorStyle, Action, AnyViewHandle, AppContext, ElementBox, Entity, + MutableAppContext, RenderContext, View, ViewContext, ViewHandle, WeakViewHandle, }; use settings::Settings; @@ -42,6 +42,7 @@ pub enum ToolbarItemLocation { pub struct Toolbar { active_pane_item: Option>, + pane: WeakViewHandle, items: Vec<(Box, ToolbarItemLocation)>, } @@ -60,6 +61,7 @@ impl View for Toolbar { let mut primary_left_items = Vec::new(); let mut primary_right_items = Vec::new(); let mut secondary_item = None; + let spacing = theme.item_spacing; for (item, position) in &self.items { match *position { @@ -68,7 +70,7 @@ impl View for Toolbar { let left_item = ChildView::new(item.as_ref()) .aligned() .contained() - .with_margin_right(theme.item_spacing); + .with_margin_right(spacing); if let Some((flex, expanded)) = flex { primary_left_items.push(left_item.flex(flex, expanded).boxed()); } else { @@ -79,7 +81,7 @@ impl View for Toolbar { let right_item = ChildView::new(item.as_ref()) .aligned() .contained() - .with_margin_left(theme.item_spacing) + .with_margin_left(spacing) .flex_float(); if let Some((flex, expanded)) = flex { primary_right_items.push(right_item.flex(flex, expanded).boxed()); @@ -98,26 +100,78 @@ impl View for Toolbar { } } + let pane = self.pane.clone(); + let container_style = theme.container; + let height = theme.height; + let button_style = theme.nav_button; + Flex::column() .with_child( Flex::row() + .with_child(nav_button( + "icons/arrow-left.svg", + button_style, + spacing, + super::GoBack { + pane: Some(pane.clone()), + }, + cx, + )) + .with_child(nav_button( + "icons/arrow-right.svg", + button_style, + spacing, + super::GoForward { + pane: Some(pane.clone()), + }, + cx, + )) .with_children(primary_left_items) .with_children(primary_right_items) .constrained() - .with_height(theme.height) + .with_height(height) .boxed(), ) .with_children(secondary_item) .contained() - .with_style(theme.container) + .with_style(container_style) .boxed() } } +fn nav_button( + svg_path: &'static str, + style: theme::Interactive, + spacing: f32, + action: A, + cx: &mut RenderContext, +) -> ElementBox { + MouseEventHandler::new::(0, cx, |state, _| { + let style = style.style_for(state, false); + Svg::new(svg_path) + .with_color(style.color) + .constrained() + .with_width(style.icon_width) + .aligned() + .contained() + .with_style(style.container) + .constrained() + .with_width(style.button_width) + .with_height(style.button_width) + .boxed() + }) + .with_cursor_style(CursorStyle::PointingHand) + .on_mouse_down(move |_, cx| cx.dispatch_action(action.clone())) + .contained() + .with_margin_right(spacing) + .boxed() +} + impl Toolbar { - pub fn new() -> Self { + pub fn new(pane: WeakViewHandle) -> Self { Self { active_pane_item: None, + pane, items: Default::default(), } } diff --git a/styles/src/styleTree/workspace.ts b/styles/src/styleTree/workspace.ts index 2deadc02e7730041c2801d5fb87cf95c7bfb3ec8..ba3978fc74e1ef93d956cd30beb8d64c9f89df8b 100644 --- a/styles/src/styleTree/workspace.ts +++ b/styles/src/styleTree/workspace.ts @@ -139,6 +139,15 @@ export default function workspace(theme: Theme) { background: backgroundColor(theme, 500), border: border(theme, "secondary", { bottom: true }), itemSpacing: 8, + navButton: { + color: iconColor(theme, "secondary"), + iconWidth: 8, + buttonWidth: 12, + margin: { left: 8, right: 8 }, + hover: { + color: iconColor(theme, "active"), + }, + }, padding: { left: 16, right: 8, top: 4, bottom: 4 }, }, breadcrumbs: { From a378ec49ec8e4bd0a8ed08f32a35d4f830f16730 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 6 Jul 2022 15:43:42 -0700 Subject: [PATCH 2/4] 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 | 57 +++++++------ crates/theme/src/theme.rs | 29 +++---- crates/workspace/src/pane.rs | 128 ++++++++++++++++++++---------- crates/workspace/src/toolbar.rs | 23 +++++- crates/workspace/src/workspace.rs | 10 +-- styles/src/styleTree/workspace.ts | 6 +- 6 files changed, 158 insertions(+), 95 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 9dd40413fd254e7ef852643acfdb5cbe464ee415..c1e25575557bef78921841f1c2a9eb8e589085a2 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -4065,13 +4065,16 @@ impl Editor { } } - nav_history.push(Some(NavigationData { - cursor_anchor: position, - cursor_position: point, - scroll_position: self.scroll_position, - scroll_top_anchor: self.scroll_top_anchor.clone(), - scroll_top_row, - })); + nav_history.push( + Some(NavigationData { + cursor_anchor: position, + cursor_position: point, + scroll_position: self.scroll_position, + scroll_top_anchor: self.scroll_top_anchor.clone(), + scroll_top_row, + }), + cx, + ); } } @@ -4669,7 +4672,7 @@ impl Editor { definitions: Vec, cx: &mut ViewContext, ) { - 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(); - nav_history.borrow_mut().disable(); + let pane = workspace.active_pane().clone(); + 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 { + 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); diff --git a/crates/theme/src/theme.rs b/crates/theme/src/theme.rs index c3052ff6c5afc0a7a5acde8135f555e7113ab6c1..058dd5a331729c2ab84dcdd5d41f7349768ce48c 100644 --- a/crates/theme/src/theme.rs +++ b/crates/theme/src/theme.rs @@ -510,28 +510,23 @@ pub struct Interactive { pub default: T, pub hover: Option, pub active: Option, - pub active_hover: Option, + pub disabled: Option, } impl Interactive { pub fn style_for(&self, state: MouseState, active: bool) -> &T { if active { - if state.hovered { - self.active_hover - .as_ref() - .or(self.active.as_ref()) - .unwrap_or(&self.default) - } else { - self.active.as_ref().unwrap_or(&self.default) - } + self.active.as_ref().unwrap_or(&self.default) + } else if state.hovered { + self.hover.as_ref().unwrap_or(&self.default) } else { - if state.hovered { - self.hover.as_ref().unwrap_or(&self.default) - } else { - &self.default - } + &self.default } } + + pub fn disabled_style(&self) -> &T { + self.disabled.as_ref().unwrap_or(&self.default) + } } impl<'de, T: DeserializeOwned> Deserialize<'de> for Interactive { @@ -545,7 +540,7 @@ impl<'de, T: DeserializeOwned> Deserialize<'de> for Interactive { default: Value, hover: Option, active: Option, - active_hover: Option, + disabled: Option, } let json = Helper::deserialize(deserializer)?; @@ -571,14 +566,14 @@ impl<'de, T: DeserializeOwned> Deserialize<'de> for Interactive { 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, }) } } diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 0fb5225cc533266753c4b84acf225373ce0c6cf0..391ddfc1f7a6d0c9e6a727622d925b3d40ead78b 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -136,13 +136,13 @@ pub struct ItemNavHistory { item: Rc, } -#[derive(Default)] -pub struct NavHistory { +struct NavHistory { mode: NavigationMode, backward_stack: VecDeque, forward_stack: VecDeque, closed_stack: VecDeque, paths_by_item: HashMap, + pane: WeakViewHandle, } #[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> { - &self.nav_history + pub fn nav_history_for_item(&self, item: &ViewHandle) -> ItemNavHistory { + ItemNavHistory { + history: self.nav_history.clone(), + item: Rc::new(item.downgrade()), + } } pub fn activate(&self, cx: &mut ViewContext) { @@ -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.toolbar.update(cx, |_, cx| cx.notify()); + } + fn navigate_history( workspace: &mut Workspace, pane: ViewHandle, @@ -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(history: Rc>, item: &ViewHandle) -> Self { - Self { - history, - item: Rc::new(item.downgrade()), - } + pub fn push(&self, data: Option, cx: &mut MutableAppContext) { + self.history.borrow_mut().push(data, self.item.clone(), cx); } - pub fn history(&self) -> Rc> { - self.history.clone() + pub fn pop_backward(&self, cx: &mut MutableAppContext) -> Option { + self.history.borrow_mut().pop(NavigationMode::GoingBack, cx) } - pub fn push(&self, data: Option) { - self.history.borrow_mut().push(data, self.item.clone()); + pub fn pop_forward(&self, cx: &mut MutableAppContext) -> Option { + 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 { - self.backward_stack.pop_back() + fn set_mode(&mut self, mode: NavigationMode) { + self.mode = mode; } - pub fn pop_forward(&mut self) -> Option { - self.forward_stack.pop_back() + fn disable(&mut self) { + self.mode = NavigationMode::Disabled; } - pub fn pop_closed(&mut self) -> Option { - self.closed_stack.pop_back() + fn enable(&mut self) { + self.mode = NavigationMode::Normal; } - fn pop(&mut self, mode: NavigationMode) -> Option { - 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 pop(&mut self, mode: NavigationMode, cx: &mut MutableAppContext) -> Option { + 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 set_mode(&mut self, mode: NavigationMode) { - self.mode = mode; - } - - pub fn push(&mut self, data: Option, item: Rc) { + fn push( + &mut self, + data: Option, + item: Rc, + 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))); + } } } diff --git a/crates/workspace/src/toolbar.rs b/crates/workspace/src/toolbar.rs index 9d8274288be5b6e5a4d8a8a26287da028f347768..7525c0413d7097d7b4e2e1f338835737ba2eab25 100644 --- a/crates/workspace/src/toolbar.rs +++ b/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( svg_path: &'static str, style: theme::Interactive, + enabled: bool, spacing: f32, action: A, cx: &mut RenderContext, ) -> ElementBox { MouseEventHandler::new::(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( .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) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index f7d3032eabd33b327523af15603f642d8f3234d0..2ba1b4d008400d2429afac0874c164e296c673ef 100644 --- a/crates/workspace/src/workspace.rs +++ b/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; - fn set_nav_history(&self, nav_history: Rc>, cx: &mut MutableAppContext); fn clone_on_split(&self, cx: &mut MutableAppContext) -> Option>; fn added_to_pane( &self, @@ -484,12 +483,6 @@ impl ItemHandle for ViewHandle { Box::new(self.clone()) } - fn set_nav_history(&self, nav_history: Rc>, 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> { self.update(cx, |item, cx| { cx.add_option_view(|cx| item.clone_on_split(cx)) @@ -503,6 +496,9 @@ impl ItemHandle for ViewHandle { pane: ViewHandle, cx: &mut ViewContext, ) { + 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( diff --git a/styles/src/styleTree/workspace.ts b/styles/src/styleTree/workspace.ts index ba3978fc74e1ef93d956cd30beb8d64c9f89df8b..0b71453e7148c71a4664c135c0b0f6926759d1db 100644 --- a/styles/src/styleTree/workspace.ts +++ b/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 }, }, From 4e8dbbfd4b637fa2ad114f42298584643cc31637 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 6 Jul 2022 16:29:11 -0700 Subject: [PATCH 3/4] Add test for pane nav history covering notification of pane's toolbar --- crates/workspace/src/workspace.rs | 113 +++++++++++++++++++++++++++++- 1 file changed, 110 insertions(+), 3 deletions(-) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 2ba1b4d008400d2429afac0874c164e296c673ef..fdfa640718fdd066f801a81142c54e7a99c9c9b3 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -3142,25 +3142,104 @@ mod tests { item.read_with(cx, |item, _| assert_eq!(item.save_count, 5)); } - #[derive(Clone)] + #[gpui::test] + async fn test_pane_navigation( + deterministic: Arc, + cx: &mut gpui::TestAppContext, + ) { + deterministic.forbid_parking(); + Settings::test_async(cx); + let fs = FakeFs::new(cx.background()); + + let project = Project::test(fs, [], cx).await; + let (window_id, workspace) = cx.add_window(|cx| Workspace::new(project, cx)); + + let item = cx.add_view(window_id, |_| { + let mut item = TestItem::new(); + item.project_entry_ids = vec![ProjectEntryId::from_proto(1)]; + item + }); + let pane = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone()); + let toolbar = pane.read_with(cx, |pane, _| pane.toolbar().clone()); + let toolbar_notify_count = Rc::new(RefCell::new(0)); + + workspace.update(cx, |workspace, cx| { + workspace.add_item(Box::new(item.clone()), cx); + let toolbar_notification_count = toolbar_notify_count.clone(); + cx.observe(&toolbar, move |_, _, _| { + *toolbar_notification_count.borrow_mut() += 1 + }) + .detach(); + }); + + pane.read_with(cx, |pane, _| { + assert!(!pane.can_navigate_backward()); + assert!(!pane.can_navigate_forward()); + }); + + item.update(cx, |item, cx| { + item.set_state("one".to_string(), cx); + }); + + // Toolbar must be notified to re-render the navigation buttons + assert_eq!(*toolbar_notify_count.borrow(), 1); + + pane.read_with(cx, |pane, _| { + assert!(pane.can_navigate_backward()); + assert!(!pane.can_navigate_forward()); + }); + + workspace + .update(cx, |workspace, cx| { + Pane::go_back(workspace, Some(pane.clone()), cx) + }) + .await; + + assert_eq!(*toolbar_notify_count.borrow(), 3); + pane.read_with(cx, |pane, _| { + assert!(!pane.can_navigate_backward()); + assert!(pane.can_navigate_forward()); + }); + } + struct TestItem { + state: String, save_count: usize, save_as_count: usize, reload_count: usize, is_dirty: bool, + is_singleton: bool, has_conflict: bool, project_entry_ids: Vec, project_path: Option, - is_singleton: bool, + nav_history: Option, } enum TestItemEvent { Edit, } + impl Clone for TestItem { + fn clone(&self) -> Self { + Self { + state: self.state.clone(), + save_count: self.save_count, + save_as_count: self.save_as_count, + reload_count: self.reload_count, + is_dirty: self.is_dirty, + is_singleton: self.is_singleton, + has_conflict: self.has_conflict, + project_entry_ids: self.project_entry_ids.clone(), + project_path: self.project_path.clone(), + nav_history: None, + } + } + } + impl TestItem { fn new() -> Self { Self { + state: String::new(), save_count: 0, save_as_count: 0, reload_count: 0, @@ -3169,6 +3248,18 @@ mod tests { project_entry_ids: Vec::new(), project_path: None, is_singleton: true, + nav_history: None, + } + } + + fn set_state(&mut self, state: String, cx: &mut ViewContext) { + self.push_to_nav_history(cx); + self.state = state; + } + + fn push_to_nav_history(&mut self, cx: &mut ViewContext) { + if let Some(history) = &mut self.nav_history { + history.push(Some(Box::new(self.state.clone())), cx); } } } @@ -3204,7 +3295,23 @@ mod tests { self.is_singleton } - fn set_nav_history(&mut self, _: ItemNavHistory, _: &mut ViewContext) {} + fn set_nav_history(&mut self, history: ItemNavHistory, _: &mut ViewContext) { + self.nav_history = Some(history); + } + + fn navigate(&mut self, state: Box, _: &mut ViewContext) -> bool { + let state = *state.downcast::().unwrap_or_default(); + if state != self.state { + self.state = state; + true + } else { + false + } + } + + fn deactivated(&mut self, cx: &mut ViewContext) { + self.push_to_nav_history(cx); + } fn clone_on_split(&self, _: &mut ViewContext) -> Option where From 70cf6b4041f979bf3a5e0056f3bd0337ff89acc4 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 6 Jul 2022 16:33:44 -0700 Subject: [PATCH 4/4] Give nav buttons a background on hover --- crates/workspace/src/toolbar.rs | 1 + styles/src/styleTree/workspace.ts | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/workspace/src/toolbar.rs b/crates/workspace/src/toolbar.rs index 7525c0413d7097d7b4e2e1f338835737ba2eab25..9e0c085b1f45dbfc85816b7dae3591d6f21909c6 100644 --- a/crates/workspace/src/toolbar.rs +++ b/crates/workspace/src/toolbar.rs @@ -173,6 +173,7 @@ fn nav_button( .constrained() .with_width(style.button_width) .with_height(style.button_width) + .aligned() .boxed() }) .with_cursor_style(if enabled { diff --git a/styles/src/styleTree/workspace.ts b/styles/src/styleTree/workspace.ts index 0b71453e7148c71a4664c135c0b0f6926759d1db..e23e047eda29cc5240b420728297f5bf1100309d 100644 --- a/styles/src/styleTree/workspace.ts +++ b/styles/src/styleTree/workspace.ts @@ -142,13 +142,15 @@ export default function workspace(theme: Theme) { navButton: { color: iconColor(theme, "primary"), iconWidth: 8, - buttonWidth: 12, + buttonWidth: 18, + cornerRadius: 6, hover: { color: iconColor(theme, "active"), + background: backgroundColor(theme, 300), }, disabled: { color: iconColor(theme, "muted") - } + }, }, padding: { left: 16, right: 8, top: 4, bottom: 4 }, },