From 2b3ca360c34e394c03663b2d106e8baf18d2db12 Mon Sep 17 00:00:00 2001 From: Danilo Leal <67129314+danilo-leal@users.noreply.github.com> Date: Fri, 12 Sep 2025 14:22:35 -0300 Subject: [PATCH] Fix flicker in short context menus that have documentation aside (#38074) Menu items in the context menu component have the ability to display a documentation aside popover. However, because this docs aside popover was setup as a sibling flex container to the actual menu popover, if the menu had a short amount of items and the docs popover is bigger than the menu, this flickering would happen, making it essentially unusable: https://github.com/user-attachments/assets/74956254-fff6-4c5c-9f79-02998c64a105 So, this PR makes the docs aside popover in wide window sizes absolutely-positioned relative to the menu container, which removes all flickering. On top of that, I'm adding a `DocumentationEdge` enum that allows to control the edge anchor of the docs aside, which is useful in this particular mode selector example to make the layout work well. https://github.com/user-attachments/assets/a3e811e1-86b4-4839-a219-c3b0734532b3 When the window is small, the docs aside continue to be a sibling flex container, which causes a super subtle shift in the items within the menu popover. This is something I want to pursue fixing, but didn't want to delay this PR too much. Release Notes: - N/A --- crates/agent_ui/src/acp/mode_selector.rs | 10 ++- crates/agent_ui/src/profile_selector.rs | 12 +-- .../src/edit_prediction_button.rs | 14 ++-- crates/language_tools/src/lsp_button.rs | 6 +- crates/ui/src/components/context_menu.rs | 83 ++++++++++++------- crates/zed/src/zed/quick_action_bar.rs | 8 +- 6 files changed, 79 insertions(+), 54 deletions(-) diff --git a/crates/agent_ui/src/acp/mode_selector.rs b/crates/agent_ui/src/acp/mode_selector.rs index b68643859efdcd7fcac5e2ca5f652372a58cc577..d4d424f41a36652881a50b65bc3dbe00c18fdcde 100644 --- a/crates/agent_ui/src/acp/mode_selector.rs +++ b/crates/agent_ui/src/acp/mode_selector.rs @@ -5,8 +5,8 @@ use fs::Fs; use gpui::{Context, Entity, FocusHandle, WeakEntity, Window, prelude::*}; use std::{rc::Rc, sync::Arc}; use ui::{ - Button, ContextMenu, ContextMenuEntry, KeyBinding, PopoverMenu, PopoverMenuHandle, Tooltip, - prelude::*, + Button, ContextMenu, ContextMenuEntry, DocumentationEdge, DocumentationSide, KeyBinding, + PopoverMenu, PopoverMenuHandle, Tooltip, prelude::*, }; use crate::{CycleModeSelector, ToggleProfileSelector}; @@ -91,7 +91,7 @@ impl ModeSelector { .toggleable(IconPosition::End, is_selected); let entry = if let Some(description) = &mode.description { - entry.documentation_aside(ui::DocumentationSide::Left, { + entry.documentation_aside(DocumentationSide::Left, DocumentationEdge::Bottom, { let description = description.clone(); move |cx| { @@ -223,6 +223,10 @@ impl Render for ModeSelector { ) .anchor(gpui::Corner::BottomRight) .with_handle(self.menu_handle.clone()) + .offset(gpui::Point { + x: px(0.0), + y: px(-2.0), + }) .menu(move |window, cx| { Some(this.update(cx, |this, cx| this.build_context_menu(window, cx))) }) diff --git a/crates/agent_ui/src/profile_selector.rs b/crates/agent_ui/src/profile_selector.rs index 07e31ee5dbd3d38319af10c4624b2988192b80e6..85f74a0f7445df03a24bf083f31d56f97e5a07b2 100644 --- a/crates/agent_ui/src/profile_selector.rs +++ b/crates/agent_ui/src/profile_selector.rs @@ -8,8 +8,8 @@ use gpui::{Action, Entity, FocusHandle, Subscription, prelude::*}; use settings::{Settings as _, SettingsStore, update_settings_file}; use std::sync::Arc; use ui::{ - ContextMenu, ContextMenuEntry, DocumentationSide, PopoverMenu, PopoverMenuHandle, TintColor, - Tooltip, prelude::*, + ContextMenu, ContextMenuEntry, DocumentationEdge, DocumentationSide, PopoverMenu, + PopoverMenuHandle, TintColor, Tooltip, prelude::*, }; /// Trait for types that can provide and manage agent profiles @@ -129,9 +129,11 @@ impl ProfileSelector { .toggleable(IconPosition::End, profile_id == thread_profile_id); let entry = if let Some(doc_text) = documentation { - entry.documentation_aside(documentation_side(settings.dock), move |_| { - Label::new(doc_text).into_any_element() - }) + entry.documentation_aside( + documentation_side(settings.dock), + DocumentationEdge::Top, + move |_| Label::new(doc_text).into_any_element(), + ) } else { entry }; diff --git a/crates/edit_prediction_button/src/edit_prediction_button.rs b/crates/edit_prediction_button/src/edit_prediction_button.rs index 0e3fe8cb1a449e494592d8f517feb26131d89f65..d4c1763c405825f834b640dfb03b38bd3016288d 100644 --- a/crates/edit_prediction_button/src/edit_prediction_button.rs +++ b/crates/edit_prediction_button/src/edit_prediction_button.rs @@ -24,8 +24,8 @@ use std::{ }; use supermaven::{AccountStatus, Supermaven}; use ui::{ - Clickable, ContextMenu, ContextMenuEntry, DocumentationSide, IconButton, IconButtonShape, - Indicator, PopoverMenu, PopoverMenuHandle, ProgressBar, Tooltip, prelude::*, + Clickable, ContextMenu, ContextMenuEntry, DocumentationEdge, DocumentationSide, IconButton, + IconButtonShape, Indicator, PopoverMenu, PopoverMenuHandle, ProgressBar, Tooltip, prelude::*, }; use workspace::{ StatusItemView, Toast, Workspace, create_and_open_local_file, item::ItemHandle, @@ -447,7 +447,7 @@ impl EditPredictionButton { menu = menu.item( entry .disabled(true) - .documentation_aside(DocumentationSide::Left, move |_cx| { + .documentation_aside(DocumentationSide::Left, DocumentationEdge::Top, move |_cx| { Label::new(format!("Edit predictions cannot be toggled for this buffer because they are disabled for {}", language.name())) .into_any_element() }) @@ -499,7 +499,7 @@ impl EditPredictionButton { .item( ContextMenuEntry::new("Eager") .toggleable(IconPosition::Start, eager_mode) - .documentation_aside(DocumentationSide::Left, move |_| { + .documentation_aside(DocumentationSide::Left, DocumentationEdge::Top, move |_| { Label::new("Display predictions inline when there are no language server completions available.").into_any_element() }) .handler({ @@ -512,7 +512,7 @@ impl EditPredictionButton { .item( ContextMenuEntry::new("Subtle") .toggleable(IconPosition::Start, subtle_mode) - .documentation_aside(DocumentationSide::Left, move |_| { + .documentation_aside(DocumentationSide::Left, DocumentationEdge::Top, move |_| { Label::new("Display predictions inline only when holding a modifier key (alt by default).").into_any_element() }) .handler({ @@ -543,7 +543,7 @@ impl EditPredictionButton { .toggleable(IconPosition::Start, data_collection.is_enabled()) .icon(icon_name) .icon_color(icon_color) - .documentation_aside(DocumentationSide::Left, move |cx| { + .documentation_aside(DocumentationSide::Left, DocumentationEdge::Top, move |cx| { let (msg, label_color, icon_name, icon_color) = match (is_open_source, is_collecting) { (true, true) => ( "Project identified as open source, and you're sharing data.", @@ -626,7 +626,7 @@ impl EditPredictionButton { ContextMenuEntry::new("Configure Excluded Files") .icon(IconName::LockOutlined) .icon_color(Color::Muted) - .documentation_aside(DocumentationSide::Left, |_| { + .documentation_aside(DocumentationSide::Left, DocumentationEdge::Top, |_| { Label::new(indoc!{" Open your settings to add sensitive paths for which Zed will never predict edits."}).into_any_element() }) diff --git a/crates/language_tools/src/lsp_button.rs b/crates/language_tools/src/lsp_button.rs index 59beceff98ff2544aa22accc470b4e497b88c6ca..b750ad1621a4711ccbf827d6054ce50d168d6b29 100644 --- a/crates/language_tools/src/lsp_button.rs +++ b/crates/language_tools/src/lsp_button.rs @@ -17,8 +17,8 @@ use project::{ }; use settings::{Settings as _, SettingsStore}; use ui::{ - Context, ContextMenu, ContextMenuEntry, ContextMenuItem, DocumentationAside, DocumentationSide, - Indicator, PopoverMenu, PopoverMenuHandle, Tooltip, Window, prelude::*, + Context, ContextMenu, ContextMenuEntry, ContextMenuItem, DocumentationAside, DocumentationEdge, + DocumentationSide, Indicator, PopoverMenu, PopoverMenuHandle, Tooltip, Window, prelude::*, }; use workspace::{StatusItemView, Workspace}; @@ -121,7 +121,6 @@ impl LanguageServerHealthStatus { impl LanguageServerState { fn fill_menu(&self, mut menu: ContextMenu, cx: &mut Context) -> ContextMenu { - menu = menu.align_popover_bottom(); let lsp_logs = cx .try_global::() .map(|lsp_logs| lsp_logs.0.clone()); @@ -357,6 +356,7 @@ impl LanguageServerState { message.map(|server_message| { DocumentationAside::new( DocumentationSide::Right, + DocumentationEdge::Bottom, Rc::new(move |_| Label::new(server_message.clone()).into_any_element()), ) }), diff --git a/crates/ui/src/components/context_menu.rs b/crates/ui/src/components/context_menu.rs index 21ab283d883eb631e1fa2fd5ec3ca571ba0f898c..e57f02be915fdecec7a5af4894c6f4fdd72f48bc 100644 --- a/crates/ui/src/components/context_menu.rs +++ b/crates/ui/src/components/context_menu.rs @@ -128,10 +128,12 @@ impl ContextMenuEntry { pub fn documentation_aside( mut self, side: DocumentationSide, + edge: DocumentationEdge, render: impl Fn(&mut App) -> AnyElement + 'static, ) -> Self { self.documentation_aside = Some(DocumentationAside { side, + edge, render: Rc::new(render), }); @@ -161,7 +163,6 @@ pub struct ContextMenu { keep_open_on_confirm: bool, documentation_aside: Option<(usize, DocumentationAside)>, fixed_width: Option, - align_popover_top: bool, } #[derive(Copy, Clone, PartialEq, Eq)] @@ -170,15 +171,27 @@ pub enum DocumentationSide { Right, } +#[derive(Copy, Default, Clone, PartialEq, Eq)] +pub enum DocumentationEdge { + #[default] + Top, + Bottom, +} + #[derive(Clone)] pub struct DocumentationAside { side: DocumentationSide, + edge: DocumentationEdge, render: Rc AnyElement>, } impl DocumentationAside { - pub fn new(side: DocumentationSide, render: Rc AnyElement>) -> Self { - Self { side, render } + pub fn new( + side: DocumentationSide, + edge: DocumentationEdge, + render: Rc AnyElement>, + ) -> Self { + Self { side, edge, render } } } @@ -218,7 +231,6 @@ impl ContextMenu { key_context: "menu".into(), _on_blur_subscription, keep_open_on_confirm: false, - align_popover_top: true, documentation_aside: None, fixed_width: None, end_slot_action: None, @@ -261,7 +273,6 @@ impl ContextMenu { key_context: "menu".into(), _on_blur_subscription, keep_open_on_confirm: true, - align_popover_top: true, documentation_aside: None, fixed_width: None, end_slot_action: None, @@ -302,7 +313,6 @@ impl ContextMenu { |this: &mut ContextMenu, window, cx| this.cancel(&menu::Cancel, window, cx), ), keep_open_on_confirm: false, - align_popover_top: true, documentation_aside: None, fixed_width: None, end_slot_action: None, @@ -788,11 +798,6 @@ impl ContextMenu { self } - pub fn align_popover_bottom(mut self) -> Self { - self.align_popover_top = false; - self - } - fn render_menu_item( &self, ix: usize, @@ -1102,6 +1107,7 @@ impl Render for ContextMenu { WithRemSize::new(ui_font_size) .occlude() .elevation_2(cx) + .w_full() .p_2() .overflow_hidden() .when(is_wide_window, |this| this.max_w_96()) @@ -1109,31 +1115,19 @@ impl Render for ContextMenu { .child((aside.render)(cx)) }; - h_flex() - .when(is_wide_window, |this| this.flex_row()) - .when(!is_wide_window, |this| this.flex_col()) - .w_full() - .map(|div| { - if self.align_popover_top { - div.items_start() - } else { - div.items_end() - } - }) - .gap_1() - .child(div().children(aside.clone().and_then(|(_, aside)| { - (aside.side == DocumentationSide::Left).then(|| render_aside(aside, cx)) - }))) - .child( + let render_menu = + |cx: &mut Context, window: &mut Window| { WithRemSize::new(ui_font_size) .occlude() .elevation_2(cx) .flex() .flex_row() + .flex_shrink_0() .child( v_flex() .id("context-menu") .max_h(vh(0.75, window)) + .flex_shrink_0() .when_some(self.fixed_width, |this, width| { this.w(width).overflow_x_hidden() }) @@ -1178,11 +1172,36 @@ impl Render for ContextMenu { }), ), ), - ), - ) - .child(div().children(aside.and_then(|(_, aside)| { - (aside.side == DocumentationSide::Right).then(|| render_aside(aside, cx)) - }))) + ) + }; + + if is_wide_window { + div() + .relative() + .child(render_menu(cx, window)) + .children(aside.map(|(_item_index, aside)| { + h_flex() + .absolute() + .when(aside.side == DocumentationSide::Left, |this| { + this.right_full().mr_1() + }) + .when(aside.side == DocumentationSide::Right, |this| { + this.left_full().ml_1() + }) + .when(aside.edge == DocumentationEdge::Top, |this| this.top_0()) + .when(aside.edge == DocumentationEdge::Bottom, |this| { + this.bottom_0() + }) + .child(render_aside(aside, cx)) + })) + } else { + v_flex() + .w_full() + .gap_1() + .justify_end() + .children(aside.map(|(_, aside)| render_aside(aside, cx))) + .child(render_menu(cx, window)) + } } } diff --git a/crates/zed/src/zed/quick_action_bar.rs b/crates/zed/src/zed/quick_action_bar.rs index e57d5d3889b97290164eb4b26dd35f4cbc8b6721..a6f85000e9b2fd2b853880a9045984938b6a7445 100644 --- a/crates/zed/src/zed/quick_action_bar.rs +++ b/crates/zed/src/zed/quick_action_bar.rs @@ -20,8 +20,8 @@ use project::project_settings::DiagnosticSeverity; use search::{BufferSearchBar, buffer_search}; use settings::{Settings, SettingsStore}; use ui::{ - ButtonStyle, ContextMenu, ContextMenuEntry, DocumentationSide, IconButton, IconName, IconSize, - PopoverMenu, PopoverMenuHandle, Tooltip, prelude::*, + ButtonStyle, ContextMenu, ContextMenuEntry, DocumentationEdge, DocumentationSide, IconButton, + IconName, IconSize, PopoverMenu, PopoverMenuHandle, Tooltip, prelude::*, }; use vim_mode_setting::VimModeSetting; use workspace::{ @@ -401,7 +401,7 @@ impl Render for QuickActionBar { } }); if !edit_predictions_enabled_at_cursor { - edit_prediction_entry = edit_prediction_entry.documentation_aside(DocumentationSide::Left, |_| { + edit_prediction_entry = edit_prediction_entry.documentation_aside(DocumentationSide::Left, DocumentationEdge::Top, |_| { Label::new("You can't toggle edit predictions for this file as it is within the excluded files list.").into_any_element() }); } @@ -452,7 +452,7 @@ impl Render for QuickActionBar { } }); if !diagnostics_enabled { - inline_diagnostics_item = inline_diagnostics_item.disabled(true).documentation_aside(DocumentationSide::Left, |_| Label::new("Inline diagnostics are not available until regular diagnostics are enabled.").into_any_element()); + inline_diagnostics_item = inline_diagnostics_item.disabled(true).documentation_aside(DocumentationSide::Left, DocumentationEdge::Top, |_| Label::new("Inline diagnostics are not available until regular diagnostics are enabled.").into_any_element()); } menu = menu.item(inline_diagnostics_item) }