From 298314d526e58cc953fc84d3efb6e4c1c47c73b1 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 11 Mar 2024 12:11:51 +0100 Subject: [PATCH] Fix regressions introduced by flicker fix (#9162) This pull request fixes a couple of easy regressions we discovered right after using #9012 on nightly: - Popover buttons for a chat message were being occluded by the message itself. - Scrolling was not working on the `List` element. Release Notes: - N/A --- crates/collab_ui/src/chat_panel.rs | 8 +++--- crates/gpui/src/elements/list.rs | 43 +++++++++++++++--------------- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/crates/collab_ui/src/chat_panel.rs b/crates/collab_ui/src/chat_panel.rs index 85f7f548b2..bf34ad4326 100644 --- a/crates/collab_ui/src/chat_panel.rs +++ b/crates/collab_ui/src/chat_panel.rs @@ -466,10 +466,6 @@ impl ChatPanel { .relative() .group("") .when(!is_continuation_from_previous, |this| this.pt_2()) - .child( - self.render_popover_buttons(&cx, message_id, can_delete_message) - .neg_mt_2p5(), - ) .child( div() .group("") @@ -583,6 +579,10 @@ impl ChatPanel { ) }, ) + .child( + self.render_popover_buttons(&cx, message_id, can_delete_message) + .neg_mt_2p5(), + ) } fn has_open_menu(&self, message_id: Option) -> bool { diff --git a/crates/gpui/src/elements/list.rs b/crates/gpui/src/elements/list.rs index 7d99704bf7..b353da5e63 100644 --- a/crates/gpui/src/elements/list.rs +++ b/crates/gpui/src/elements/list.rs @@ -8,12 +8,11 @@ use crate::{ point, px, size, AnyElement, AvailableSpace, Bounds, ContentMask, DispatchPhase, Edges, - Element, ElementContext, HitboxId, IntoElement, Pixels, Point, ScrollWheelEvent, Size, Style, + Element, ElementContext, Hitbox, IntoElement, Pixels, Point, ScrollWheelEvent, Size, Style, StyleRefinement, Styled, WindowContext, }; use collections::VecDeque; use refineable::Refineable as _; -use smallvec::SmallVec; use std::{cell::RefCell, ops::Range, rc::Rc}; use sum_tree::{Bias, SumTree}; use taffy::style::Overflow; @@ -97,11 +96,10 @@ struct LayoutItemsResponse { item_elements: VecDeque, } -/// Frame state used by the [List] element. -#[derive(Default)] -pub struct ListFrameState { - scroll_top: ListOffset, - items: SmallVec<[AnyElement; 32]>, +/// Frame state used by the [List] element after layout. +pub struct ListAfterLayoutState { + hitbox: Hitbox, + layout: LayoutItemsResponse, } #[derive(Clone)] @@ -524,8 +522,8 @@ pub struct ListOffset { } impl Element for List { - type BeforeLayout = ListFrameState; - type AfterLayout = HitboxId; + type BeforeLayout = (); + type AfterLayout = ListAfterLayoutState; fn before_layout( &mut self, @@ -588,21 +586,23 @@ impl Element for List { }) } }; - (layout_id, ListFrameState::default()) + (layout_id, ()) } fn after_layout( &mut self, bounds: Bounds, - before_layout: &mut Self::BeforeLayout, + _: &mut Self::BeforeLayout, cx: &mut ElementContext, - ) -> HitboxId { + ) -> ListAfterLayoutState { let state = &mut *self.state.0.borrow_mut(); state.reset = false; let mut style = Style::default(); style.refine(&self.style); + let hitbox = cx.insert_hitbox(bounds, false); + // If the width of the list has changed, invalidate all cached item heights if state.last_layout_bounds.map_or(true, |last_bounds| { last_bounds.size.width != bounds.size.width @@ -623,10 +623,9 @@ impl Element for List { cx.with_content_mask(Some(ContentMask { bounds }), |cx| { let mut item_origin = bounds.origin + Point::new(px(0.), padding.top); item_origin.y -= layout_response.scroll_top.offset_in_item; - for mut item_element in layout_response.item_elements { + for mut item_element in &mut layout_response.item_elements { let item_size = item_element.measure(layout_response.available_item_space, cx); item_element.layout(item_origin, layout_response.available_item_space, cx); - before_layout.items.push(item_element); item_origin.y += item_size.height; } }); @@ -634,27 +633,29 @@ impl Element for List { state.last_layout_bounds = Some(bounds); state.last_padding = Some(padding); - - cx.insert_hitbox(bounds, false).id + ListAfterLayoutState { + hitbox, + layout: layout_response, + } } fn paint( &mut self, bounds: Bounds, - before_layout: &mut Self::BeforeLayout, - hitbox_id: &mut HitboxId, + _: &mut Self::BeforeLayout, + after_layout: &mut Self::AfterLayout, cx: &mut crate::ElementContext, ) { cx.with_content_mask(Some(ContentMask { bounds }), |cx| { - for item in &mut before_layout.items { + for item in &mut after_layout.layout.item_elements { item.paint(cx); } }); let list_state = self.state.clone(); let height = bounds.size.height; - let scroll_top = before_layout.scroll_top; - let hitbox_id = *hitbox_id; + let scroll_top = after_layout.layout.scroll_top; + let hitbox_id = after_layout.hitbox.id; cx.on_mouse_event(move |event: &ScrollWheelEvent, phase, cx| { if phase == DispatchPhase::Bubble && hitbox_id.is_hovered(cx) { list_state.0.borrow_mut().scroll(