From 025d857be80e74cd1fb39eb3baaa01c6de3ae272 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Tue, 5 Apr 2022 18:40:25 -0600 Subject: [PATCH 1/6] Make UniformListState an Rc> instead of an Arc> We don't need to support multiple threads. --- crates/gpui/src/elements/uniform_list.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/crates/gpui/src/elements/uniform_list.rs b/crates/gpui/src/elements/uniform_list.rs index 4fbb9ca420..7bd3e28a2e 100644 --- a/crates/gpui/src/elements/uniform_list.rs +++ b/crates/gpui/src/elements/uniform_list.rs @@ -8,11 +8,10 @@ use crate::{ ElementBox, }; use json::ToJson; -use parking_lot::Mutex; -use std::{cmp, ops::Range, sync::Arc}; +use std::{cell::RefCell, cmp, ops::Range, rc::Rc}; #[derive(Clone, Default)] -pub struct UniformListState(Arc>); +pub struct UniformListState(Rc>); #[derive(Debug)] pub enum ScrollTarget { @@ -22,11 +21,11 @@ pub enum ScrollTarget { impl UniformListState { pub fn scroll_to(&self, scroll_to: ScrollTarget) { - self.0.lock().scroll_to = Some(scroll_to); + self.0.borrow_mut().scroll_to = Some(scroll_to); } pub fn scroll_top(&self) -> f32 { - self.0.lock().scroll_top + self.0.borrow().scroll_top } } @@ -96,7 +95,7 @@ where delta *= 20.; } - let mut state = self.state.0.lock(); + let mut state = self.state.0.borrow_mut(); state.scroll_top = (state.scroll_top - delta.y()).max(0.0).min(scroll_max); cx.notify(); @@ -104,7 +103,7 @@ where } fn autoscroll(&mut self, scroll_max: f32, list_height: f32, item_height: f32) { - let mut state = self.state.0.lock(); + let mut state = self.state.0.borrow_mut(); if let Some(scroll_to) = state.scroll_to.take() { let item_ix; @@ -141,7 +140,7 @@ where } fn scroll_top(&self) -> f32 { - self.state.0.lock().scroll_top + self.state.0.borrow().scroll_top } } From ab3bbe1e171a032f8113c290d7473610af988e12 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Tue, 5 Apr 2022 19:58:15 -0600 Subject: [PATCH 2/6] Make the tabs scrollable when they overflow This adds the ability to make a Flex element scrollable by passing a type tag and instance id, which we use to store the scroll position in an ElementStateHandle. Still need to allow the element to auto-scroll. --- crates/gpui/src/elements/flex.rs | 75 ++++++++++++++++++++++++++++++-- crates/workspace/src/pane.rs | 2 +- 2 files changed, 72 insertions(+), 5 deletions(-) diff --git a/crates/gpui/src/elements/flex.rs b/crates/gpui/src/elements/flex.rs index 2ec307bbc3..6afe80ce06 100644 --- a/crates/gpui/src/elements/flex.rs +++ b/crates/gpui/src/elements/flex.rs @@ -2,8 +2,8 @@ use std::{any::Any, f32::INFINITY}; use crate::{ json::{self, ToJson, Value}, - Axis, DebugContext, Element, ElementBox, Event, EventContext, LayoutContext, PaintContext, - SizeConstraint, Vector2FExt, + Axis, DebugContext, Element, ElementBox, ElementStateContext, ElementStateHandle, Event, + EventContext, LayoutContext, PaintContext, SizeConstraint, Vector2FExt, }; use pathfinder_geometry::{ rect::RectF, @@ -11,9 +11,15 @@ use pathfinder_geometry::{ }; use serde_json::json; +#[derive(Default)] +struct ScrollState { + scroll_position: f32, +} + pub struct Flex { axis: Axis, children: Vec, + scroll_state: Option>, } impl Flex { @@ -21,6 +27,7 @@ impl Flex { Self { axis, children: Default::default(), + scroll_state: None, } } @@ -32,6 +39,15 @@ impl Flex { Self::new(Axis::Vertical) } + pub fn scrollable(mut self, element_id: usize, cx: &mut C) -> Self + where + Tag: 'static, + C: ElementStateContext, + { + self.scroll_state = Some(cx.element_state::(element_id)); + self + } + fn layout_flex_children( &mut self, layout_expanded: bool, @@ -167,6 +183,13 @@ impl Element for Flex { size.set_y(constraint.max.y()); } + if let Some(scroll_state) = self.scroll_state.as_ref() { + scroll_state.update(cx, |scroll_state, _| { + scroll_state.scroll_position = + scroll_state.scroll_position.min(-remaining_space).max(0.); + }); + } + (size, remaining_space) } @@ -181,7 +204,16 @@ impl Element for Flex { if overflowing { cx.scene.push_layer(Some(bounds)); } + let mut child_origin = bounds.origin(); + if let Some(scroll_state) = self.scroll_state.as_ref() { + let scroll_position = scroll_state.read(cx).scroll_position; + match self.axis { + Axis::Horizontal => child_origin.set_x(child_origin.x() - scroll_position), + Axis::Vertical => child_origin.set_y(child_origin.y() - scroll_position), + } + } + for child in &mut self.children { if *remaining_space > 0. { if let Some(metadata) = child.metadata::() { @@ -208,8 +240,8 @@ impl Element for Flex { fn dispatch_event( &mut self, event: &Event, - _: RectF, - _: &mut Self::LayoutState, + bounds: RectF, + remaining_space: &mut Self::LayoutState, _: &mut Self::PaintState, cx: &mut EventContext, ) -> bool { @@ -217,6 +249,41 @@ impl Element for Flex { for child in &mut self.children { handled = child.dispatch_event(event, cx) || handled; } + if !handled { + if let &Event::ScrollWheel { + position, + delta, + precise, + } = event + { + if *remaining_space < 0. && bounds.contains_point(position) { + if let Some(scroll_state) = self.scroll_state.as_ref() { + scroll_state.update(cx, |scroll_state, cx| { + dbg!(precise, delta); + + let mut delta = match self.axis { + Axis::Horizontal => { + if delta.x() != 0. { + delta.x() + } else { + delta.y() + } + } + Axis::Vertical => delta.y(), + }; + if !precise { + delta *= 20.; + } + + scroll_state.scroll_position -= delta; + + handled = true; + cx.notify(); + }); + } + } + } + } handled } diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index fb13ef2fd0..76093883c4 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -633,7 +633,7 @@ impl Pane { enum Tabs {} let pane = cx.handle(); let tabs = MouseEventHandler::new::(0, cx, |mouse_state, cx| { - let mut row = Flex::row(); + let mut row = Flex::row().scrollable::(1, cx); for (ix, item) in self.items.iter().enumerate() { let is_active = ix == self.active_item_index; From eb995883681b47d9828ce401afdef2962db3a481 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Tue, 5 Apr 2022 20:02:45 -0600 Subject: [PATCH 3/6] Remove stray dbg! expressions --- crates/editor/src/display_map.rs | 5 +---- crates/gpui/src/elements/flex.rs | 2 -- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index 2bea851ec2..babcf5d0ff 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -1152,10 +1152,7 @@ pub mod tests { *markers[0].column_mut() += 1; } - assert_eq!( - unmarked_snapshot.clip_point(dbg!(markers[0]), bias), - markers[1] - ) + assert_eq!(unmarked_snapshot.clip_point(markers[0], bias), markers[1]) } }; } diff --git a/crates/gpui/src/elements/flex.rs b/crates/gpui/src/elements/flex.rs index 6afe80ce06..ff98d2948b 100644 --- a/crates/gpui/src/elements/flex.rs +++ b/crates/gpui/src/elements/flex.rs @@ -259,8 +259,6 @@ impl Element for Flex { if *remaining_space < 0. && bounds.contains_point(position) { if let Some(scroll_state) = self.scroll_state.as_ref() { scroll_state.update(cx, |scroll_state, cx| { - dbg!(precise, delta); - let mut delta = match self.axis { Axis::Horizontal => { if delta.x() != 0. { From 1453954ef4398535f017530ce52a30d7aea0ac34 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 6 Apr 2022 09:08:44 +0200 Subject: [PATCH 4/6] Autoscroll to active tab when activating a new item --- crates/gpui/src/elements/flex.rs | 29 +++++++++++++++++++++++++++-- crates/workspace/src/pane.rs | 12 ++++++++++-- 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/crates/gpui/src/elements/flex.rs b/crates/gpui/src/elements/flex.rs index ff98d2948b..f7d60e409b 100644 --- a/crates/gpui/src/elements/flex.rs +++ b/crates/gpui/src/elements/flex.rs @@ -13,6 +13,7 @@ use serde_json::json; #[derive(Default)] struct ScrollState { + scroll_to: Option, scroll_position: f32, } @@ -39,12 +40,19 @@ impl Flex { Self::new(Axis::Vertical) } - pub fn scrollable(mut self, element_id: usize, cx: &mut C) -> Self + pub fn scrollable( + mut self, + element_id: usize, + scroll_to: Option, + cx: &mut C, + ) -> Self where Tag: 'static, C: ElementStateContext, { - self.scroll_state = Some(cx.element_state::(element_id)); + let scroll_state = cx.element_state::(element_id); + scroll_state.update(cx, |scroll_state, _| scroll_state.scroll_to = scroll_to); + self.scroll_state = Some(scroll_state); self } @@ -185,6 +193,23 @@ impl Element for Flex { if let Some(scroll_state) = self.scroll_state.as_ref() { scroll_state.update(cx, |scroll_state, _| { + if let Some(scroll_to) = scroll_state.scroll_to.take() { + let visible_start = scroll_state.scroll_position; + let visible_end = visible_start + size.along(self.axis); + if let Some(child) = self.children.get(scroll_to) { + let child_start: f32 = self.children[..scroll_to] + .iter() + .map(|c| c.size().along(self.axis)) + .sum(); + let child_end = child_start + child.size().along(self.axis); + if child_start < visible_start { + scroll_state.scroll_position = child_start; + } else if child_end > visible_end { + scroll_state.scroll_position = child_end - size.along(self.axis); + } + } + } + scroll_state.scroll_position = scroll_state.scroll_position.min(-remaining_space).max(0.); }); diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 76093883c4..5d7f373b2f 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -100,6 +100,7 @@ pub enum Event { pub struct Pane { items: Vec>, active_item_index: usize, + autoscroll: bool, nav_history: Rc>, toolbar: ViewHandle, } @@ -141,6 +142,7 @@ impl Pane { Self { items: Vec::new(), active_item_index: 0, + autoscroll: false, nav_history: Default::default(), toolbar: cx.add_view(|_| Toolbar::new()), } @@ -388,6 +390,7 @@ impl Pane { self.focus_active_item(cx); self.activate(cx); } + self.autoscroll = true; cx.notify(); } } @@ -627,13 +630,18 @@ impl Pane { }); } - fn render_tabs(&self, cx: &mut RenderContext) -> ElementBox { + fn render_tabs(&mut self, cx: &mut RenderContext) -> ElementBox { let theme = cx.global::().theme.clone(); enum Tabs {} let pane = cx.handle(); let tabs = MouseEventHandler::new::(0, cx, |mouse_state, cx| { - let mut row = Flex::row().scrollable::(1, cx); + let autoscroll = if mem::take(&mut self.autoscroll) { + Some(self.active_item_index) + } else { + None + }; + let mut row = Flex::row().scrollable::(1, autoscroll, cx); for (ix, item) in self.items.iter().enumerate() { let is_active = ix == self.active_item_index; From d7342e28750b2e80faaf496c2d2ba97c0b2ff7f5 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 6 Apr 2022 09:31:54 +0200 Subject: [PATCH 5/6] Use `Pane::activate_item` when navigating to remove duplicated logic --- crates/workspace/src/pane.rs | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 5d7f373b2f..031b06fe27 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -201,27 +201,19 @@ impl Pane { .upgrade(cx) .and_then(|v| pane.index_for_item(v.as_ref())) { - if let Some(item) = pane.active_item() { - pane.nav_history.borrow_mut().set_mode(mode); - item.deactivated(cx); - pane.nav_history - .borrow_mut() - .set_mode(NavigationMode::Normal); - } + let prev_active_item_index = pane.active_item_index; + pane.nav_history.borrow_mut().set_mode(mode); + pane.activate_item(index, true, cx); + pane.nav_history + .borrow_mut() + .set_mode(NavigationMode::Normal); - let prev_active_index = mem::replace(&mut pane.active_item_index, index); - - let mut navigated = prev_active_index != pane.active_item_index; + let mut navigated = prev_active_item_index != pane.active_item_index; if let Some(data) = entry.data { navigated |= pane.active_item()?.navigate(data, cx); } if navigated { - pane.focus_active_item(cx); - pane.update_toolbar(cx); - pane.activate(cx); - cx.emit(Event::ActivateItem { local: true }); - cx.notify(); break None; } } @@ -377,10 +369,12 @@ impl Pane { } pub fn activate_item(&mut self, index: usize, local: bool, cx: &mut ViewContext) { + use NavigationMode::{GoingBack, GoingForward}; if index < self.items.len() { let prev_active_item_ix = mem::replace(&mut self.active_item_index, index); - if prev_active_item_ix != self.active_item_index - && prev_active_item_ix < self.items.len() + if matches!(self.nav_history.borrow().mode, GoingBack | GoingForward) + || (prev_active_item_ix != self.active_item_index + && prev_active_item_ix < self.items.len()) { self.items[prev_active_item_ix].deactivated(cx); cx.emit(Event::ActivateItem { local }); From 0214bec7f483f46e0de3707f925936336d48ed64 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Wed, 6 Apr 2022 10:59:03 -0600 Subject: [PATCH 6/6] Don't dispatch events to flex children outside of parent flex's bounds --- crates/gpui/src/elements/flex.rs | 6 ++++++ crates/gpui/src/platform/event.rs | 17 +++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/crates/gpui/src/elements/flex.rs b/crates/gpui/src/elements/flex.rs index f7d60e409b..1ae9c591c8 100644 --- a/crates/gpui/src/elements/flex.rs +++ b/crates/gpui/src/elements/flex.rs @@ -270,6 +270,12 @@ impl Element for Flex { _: &mut Self::PaintState, cx: &mut EventContext, ) -> bool { + if let Some(position) = event.position() { + if !bounds.contains_point(position) { + return false; + } + } + let mut handled = false; for child in &mut self.children { handled = child.dispatch_event(event, cx) || handled; diff --git a/crates/gpui/src/platform/event.rs b/crates/gpui/src/platform/event.rs index 72e1c24d6b..fe353fed4c 100644 --- a/crates/gpui/src/platform/event.rs +++ b/crates/gpui/src/platform/event.rs @@ -61,3 +61,20 @@ pub enum Event { left_mouse_down: bool, }, } + +impl Event { + pub fn position(&self) -> Option { + match self { + Event::KeyDown { .. } => None, + Event::ScrollWheel { position, .. } + | Event::LeftMouseDown { position, .. } + | Event::LeftMouseUp { position } + | Event::LeftMouseDragged { position } + | Event::RightMouseDown { position, .. } + | Event::RightMouseUp { position } + | Event::NavigateMouseDown { position, .. } + | Event::NavigateMouseUp { position, .. } + | Event::MouseMoved { position, .. } => Some(*position), + } + } +}