From d45aaa174563cbeb0d082aa7557f1a40346b2bc9 Mon Sep 17 00:00:00 2001 From: smit <0xtimsb@gmail.com> Date: Fri, 21 Feb 2025 15:21:26 +0530 Subject: [PATCH] scrollbar: Implement minimum thumb size (#25288) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR addresses 3 issues with the common scrollbar component used in the Terminal, Outline Panel, etc. 1. Extremely small or invisible scrollbar for long content. 2. Flickering issue when the thumb is already at the bottom-most position, and the user tries to overscroll. 3. Scrollbar appearing even when there is no excessive content to scroll. Before: image After: Screenshot 2025-02-21 at 3 26 32 AM Release Notes: - Fixed extremely small scrollbar thumb for long content in Terminal, Outline Panel, and more. --------- Co-authored-by: Danilo Co-authored-by: Richard Feldman --- .../terminal_view/src/terminal_scrollbar.rs | 5 +- crates/ui/src/components/scrollbar.rs | 351 +++++++++--------- 2 files changed, 184 insertions(+), 172 deletions(-) diff --git a/crates/terminal_view/src/terminal_scrollbar.rs b/crates/terminal_view/src/terminal_scrollbar.rs index 01b910071451411dbc63e4ea944e700f9020edea..e72a1e7419713189c5cceb69056ee84a95c08fa1 100644 --- a/crates/terminal_view/src/terminal_scrollbar.rs +++ b/crates/terminal_view/src/terminal_scrollbar.rs @@ -69,9 +69,10 @@ impl ScrollableHandle for TerminalScrollHandle { let offset_delta = (point.y.0 / state.line_height.0).round() as i32; let max_offset = state.total_lines - state.viewport_lines; - let display_offset = ((max_offset as i32 + offset_delta) as usize).min(max_offset); + let display_offset = (max_offset as i32 + offset_delta).clamp(0, max_offset as i32); - self.future_display_offset.set(Some(display_offset)); + self.future_display_offset + .set(Some(display_offset as usize)); } fn viewport(&self) -> Bounds { diff --git a/crates/ui/src/components/scrollbar.rs b/crates/ui/src/components/scrollbar.rs index a9793adea81e236bef522e6fa5f41343f4a95182..3775a0d3397c99f7cd5c986fca4c299116a07d8b 100644 --- a/crates/ui/src/components/scrollbar.rs +++ b/crates/ui/src/components/scrollbar.rs @@ -99,7 +99,7 @@ pub trait ScrollableHandle: Debug + 'static { #[derive(Clone, Debug)] pub struct ScrollbarState { // If Some(), there's an active drag, offset by percentage from the origin of a thumb. - drag: Rc>>, + drag: Rc>>, parent_id: Option, scroll_handle: Arc, } @@ -128,12 +128,12 @@ impl ScrollbarState { } fn thumb_range(&self, axis: ScrollbarAxis) -> Option> { - const MINIMUM_SCROLLBAR_PERCENTAGE_SIZE: f32 = 0.005; + const MINIMUM_THUMB_SIZE: f32 = 25.; let ContentSize { size: main_dimension_size, scroll_adjustment, } = self.scroll_handle.content_size()?; - let main_dimension_size = main_dimension_size.along(axis).0; + let content_size = main_dimension_size.along(axis).0; let mut current_offset = self.scroll_handle.offset().along(axis).min(px(0.)).abs().0; if let Some(adjustment) = scroll_adjustment.and_then(|adjustment| { let adjust = adjustment.along(axis).0; @@ -145,25 +145,21 @@ impl ScrollbarState { }) { current_offset -= adjustment; } - - let mut percentage = current_offset / main_dimension_size; - let viewport_size = self.scroll_handle.viewport().size; - let end_offset = (current_offset + viewport_size.along(axis).0) / main_dimension_size; - // Scroll handle might briefly report an offset greater than the length of a list; - // in such case we'll adjust the starting offset as well to keep the scrollbar thumb length stable. - let overshoot = (end_offset - 1.).clamp(0., 1.); - if overshoot > 0. { - percentage -= overshoot; - } - if percentage + MINIMUM_SCROLLBAR_PERCENTAGE_SIZE > 1.0 || end_offset > main_dimension_size - { + let viewport_size = self.scroll_handle.viewport().size.along(axis).0; + if content_size < viewport_size { return None; } - if main_dimension_size < viewport_size.along(axis).0 { + let visible_percentage = viewport_size / content_size; + let thumb_size = MINIMUM_THUMB_SIZE.max(viewport_size * visible_percentage); + if thumb_size > viewport_size { return None; } - let end_offset = end_offset.clamp(percentage + MINIMUM_SCROLLBAR_PERCENTAGE_SIZE, 1.); - Some(percentage..end_offset) + let max_offset = content_size - viewport_size; + current_offset = current_offset.clamp(0., max_offset); + let start_offset = (current_offset / max_offset) * (viewport_size - thumb_size); + let thumb_percentage_start = start_offset / viewport_size; + let thumb_percentage_end = (start_offset + thumb_size) / viewport_size; + Some(thumb_percentage_start..thumb_percentage_end) } } @@ -228,174 +224,189 @@ impl Element for Scrollbar { fn paint( &mut self, _id: Option<&GlobalElementId>, - bounds: Bounds, + padded_bounds: Bounds, _request_layout: &mut Self::RequestLayoutState, _prepaint: &mut Self::PrepaintState, window: &mut Window, cx: &mut App, ) { - window.with_content_mask(Some(ContentMask { bounds }), |window| { - let colors = cx.theme().colors(); - let thumb_background = colors - .surface_background - .blend(colors.scrollbar_thumb_background); - let is_vertical = self.kind == ScrollbarAxis::Vertical; - let extra_padding = px(5.0); - let padded_bounds = if is_vertical { - Bounds::from_corners( - bounds.origin + point(Pixels::ZERO, extra_padding), - bounds.bottom_right() - point(Pixels::ZERO, extra_padding * 3), - ) - } else { - Bounds::from_corners( - bounds.origin + point(extra_padding, Pixels::ZERO), - bounds.bottom_right() - point(extra_padding * 3, Pixels::ZERO), - ) - }; - - let mut thumb_bounds = if is_vertical { - let thumb_offset = self.thumb.start * padded_bounds.size.height; - let thumb_end = self.thumb.end * padded_bounds.size.height; - let thumb_upper_left = point( - padded_bounds.origin.x, - padded_bounds.origin.y + thumb_offset, - ); - let thumb_lower_right = point( - padded_bounds.origin.x + padded_bounds.size.width, - padded_bounds.origin.y + thumb_end, - ); - Bounds::from_corners(thumb_upper_left, thumb_lower_right) - } else { - let thumb_offset = self.thumb.start * padded_bounds.size.width; - let thumb_end = self.thumb.end * padded_bounds.size.width; - let thumb_upper_left = point( - padded_bounds.origin.x + thumb_offset, - padded_bounds.origin.y, - ); - let thumb_lower_right = point( - padded_bounds.origin.x + thumb_end, - padded_bounds.origin.y + padded_bounds.size.height, - ); - Bounds::from_corners(thumb_upper_left, thumb_lower_right) - }; - let corners = if is_vertical { - thumb_bounds.size.width /= 1.5; - Corners::all(thumb_bounds.size.width / 2.0) - } else { - thumb_bounds.size.height /= 1.5; - Corners::all(thumb_bounds.size.height / 2.0) - }; - window.paint_quad(quad( - thumb_bounds, - corners, - thumb_background, - Edges::default(), - Hsla::transparent_black(), - )); - - let scroll = self.state.scroll_handle.clone(); - let kind = self.kind; - let thumb_percentage_size = self.thumb.end - self.thumb.start; - - window.on_mouse_event({ - let scroll = scroll.clone(); - let state = self.state.clone(); + window.with_content_mask( + Some(ContentMask { + bounds: padded_bounds, + }), + |window| { + let colors = cx.theme().colors(); + let thumb_background = colors + .surface_background + .blend(colors.scrollbar_thumb_background); + let is_vertical = self.kind == ScrollbarAxis::Vertical; + let extra_padding = px(5.0); + let padded_bounds = if is_vertical { + Bounds::from_corners( + padded_bounds.origin + point(Pixels::ZERO, extra_padding), + padded_bounds.bottom_right() - point(Pixels::ZERO, extra_padding * 3), + ) + } else { + Bounds::from_corners( + padded_bounds.origin + point(extra_padding, Pixels::ZERO), + padded_bounds.bottom_right() - point(extra_padding * 3, Pixels::ZERO), + ) + }; + + let mut thumb_bounds = if is_vertical { + let thumb_offset = self.thumb.start * padded_bounds.size.height; + let thumb_end = self.thumb.end * padded_bounds.size.height; + let thumb_upper_left = point( + padded_bounds.origin.x, + padded_bounds.origin.y + thumb_offset, + ); + let thumb_lower_right = point( + padded_bounds.origin.x + padded_bounds.size.width, + padded_bounds.origin.y + thumb_end, + ); + Bounds::from_corners(thumb_upper_left, thumb_lower_right) + } else { + let thumb_offset = self.thumb.start * padded_bounds.size.width; + let thumb_end = self.thumb.end * padded_bounds.size.width; + let thumb_upper_left = point( + padded_bounds.origin.x + thumb_offset, + padded_bounds.origin.y, + ); + let thumb_lower_right = point( + padded_bounds.origin.x + thumb_end, + padded_bounds.origin.y + padded_bounds.size.height, + ); + Bounds::from_corners(thumb_upper_left, thumb_lower_right) + }; + let corners = if is_vertical { + thumb_bounds.size.width /= 1.5; + Corners::all(thumb_bounds.size.width / 2.0) + } else { + thumb_bounds.size.height /= 1.5; + Corners::all(thumb_bounds.size.height / 2.0) + }; + window.paint_quad(quad( + thumb_bounds, + corners, + thumb_background, + Edges::default(), + Hsla::transparent_black(), + )); + + let scroll = self.state.scroll_handle.clone(); let axis = self.kind; - move |event: &MouseDownEvent, phase, _, _| { - if !(phase.bubble() && bounds.contains(&event.position)) { - return; - } - if thumb_bounds.contains(&event.position) { - let thumb_offset = (event.position.along(axis) - - thumb_bounds.origin.along(axis)) - / bounds.size.along(axis); - state.drag.set(Some(thumb_offset)); - } else if let Some(ContentSize { - size: item_size, .. - }) = scroll.content_size() - { - match kind { - ScrollbarAxis::Horizontal => { - let percentage = - (event.position.x - bounds.origin.x) / bounds.size.width; - let max_offset = item_size.width; - let percentage = percentage.min(1. - thumb_percentage_size); - scroll - .set_offset(point(-max_offset * percentage, scroll.offset().y)); - } - ScrollbarAxis::Vertical => { - let percentage = - (event.position.y - bounds.origin.y) / bounds.size.height; - let max_offset = item_size.height; - let percentage = percentage.min(1. - thumb_percentage_size); - scroll - .set_offset(point(scroll.offset().x, -max_offset * percentage)); + window.on_mouse_event({ + let scroll = scroll.clone(); + let state = self.state.clone(); + move |event: &MouseDownEvent, phase, _, _| { + if !(phase.bubble() && padded_bounds.contains(&event.position)) { + return; + } + + if thumb_bounds.contains(&event.position) { + let offset = + event.position.along(axis) - thumb_bounds.origin.along(axis); + state.drag.set(Some(offset)); + } else if let Some(ContentSize { + size: item_size, .. + }) = scroll.content_size() + { + let click_offset = { + let viewport_size = padded_bounds.size.along(axis); + + let thumb_size = thumb_bounds.size.along(axis); + let thumb_start = (event.position.along(axis) + - padded_bounds.origin.along(axis) + - (thumb_size / 2.)) + .clamp(px(0.), viewport_size - thumb_size); + + let max_offset = + (item_size.along(axis) - viewport_size).max(px(0.)); + let percentage = if viewport_size > thumb_size { + thumb_start / (viewport_size - thumb_size) + } else { + 0. + }; + + -max_offset * percentage + }; + match axis { + ScrollbarAxis::Horizontal => { + scroll.set_offset(point(click_offset, scroll.offset().y)); + } + ScrollbarAxis::Vertical => { + scroll.set_offset(point(scroll.offset().x, click_offset)); + } } } } - } - }); - window.on_mouse_event({ - let scroll = scroll.clone(); - move |event: &ScrollWheelEvent, phase, window, _| { - if phase.bubble() && bounds.contains(&event.position) { - let current_offset = scroll.offset(); - scroll.set_offset( - current_offset + event.delta.pixel_delta(window.line_height()), - ); + }); + window.on_mouse_event({ + let scroll = scroll.clone(); + move |event: &ScrollWheelEvent, phase, window, _| { + if phase.bubble() && padded_bounds.contains(&event.position) { + let current_offset = scroll.offset(); + scroll.set_offset( + current_offset + event.delta.pixel_delta(window.line_height()), + ); + } } - } - }); - let state = self.state.clone(); - let kind = self.kind; - window.on_mouse_event(move |event: &MouseMoveEvent, _, _, cx| { - if let Some(drag_state) = state.drag.get().filter(|_| event.dragging()) { - if let Some(ContentSize { - size: item_size, .. - }) = scroll.content_size() - { - match kind { - ScrollbarAxis::Horizontal => { - let max_offset = item_size.width; - let percentage = (event.position.x - bounds.origin.x) - / bounds.size.width - - drag_state; - - let percentage = percentage.min(1. - thumb_percentage_size); - scroll - .set_offset(point(-max_offset * percentage, scroll.offset().y)); - } - ScrollbarAxis::Vertical => { - let max_offset = item_size.height; - let percentage = (event.position.y - bounds.origin.y) - / bounds.size.height - - drag_state; - - let percentage = percentage.min(1. - thumb_percentage_size); - scroll - .set_offset(point(scroll.offset().x, -max_offset * percentage)); + }); + let state = self.state.clone(); + let axis = self.kind; + window.on_mouse_event(move |event: &MouseMoveEvent, _, _, cx| { + if let Some(drag_state) = state.drag.get().filter(|_| event.dragging()) { + if let Some(ContentSize { + size: item_size, .. + }) = scroll.content_size() + { + let drag_offset = { + let viewport_size = padded_bounds.size.along(axis); + + let thumb_size = thumb_bounds.size.along(axis); + let thumb_start = (event.position.along(axis) + - padded_bounds.origin.along(axis) + - drag_state) + .clamp(px(0.), viewport_size - thumb_size); + + let max_offset = + (item_size.along(axis) - viewport_size).max(px(0.)); + let percentage = if viewport_size > thumb_size { + thumb_start / (viewport_size - thumb_size) + } else { + 0. + }; + + -max_offset * percentage + }; + match axis { + ScrollbarAxis::Horizontal => { + scroll.set_offset(point(drag_offset, scroll.offset().y)); + } + ScrollbarAxis::Vertical => { + scroll.set_offset(point(scroll.offset().x, drag_offset)); + } + }; + if let Some(id) = state.parent_id { + cx.notify(id); } - }; - + } + } else { + state.drag.set(None); + } + }); + let state = self.state.clone(); + window.on_mouse_event(move |_event: &MouseUpEvent, phase, _, cx| { + if phase.bubble() { + state.drag.take(); if let Some(id) = state.parent_id { cx.notify(id); } } - } else { - state.drag.set(None); - } - }); - let state = self.state.clone(); - window.on_mouse_event(move |_event: &MouseUpEvent, phase, _, cx| { - if phase.bubble() { - state.drag.take(); - if let Some(id) = state.parent_id { - cx.notify(id); - } - } - }); - }) + }); + }, + ) } }