From 490c71af0608d0a8ea6bc643beaabc6fb2b1d59b Mon Sep 17 00:00:00 2001 From: Ben Kunkle Date: Fri, 22 May 2026 14:21:54 -0400 Subject: [PATCH] ep: Fix edit prediction diff popover being occluded by right docs and sidebar (#57519) Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Closes #ISSUE Release Notes: - Fixed an issue where edit prediction previews that appeared in the diff popover could be occluded by open docks or the sidebar on the right side of the editor --- crates/editor/src/edit_prediction.rs | 8 +- crates/editor/src/edit_prediction_tests.rs | 148 ++++++++++++++++++++- 2 files changed, 149 insertions(+), 7 deletions(-) diff --git a/crates/editor/src/edit_prediction.rs b/crates/editor/src/edit_prediction.rs index 762513fe75..e7c942bd00 100644 --- a/crates/editor/src/edit_prediction.rs +++ b/crates/editor/src/edit_prediction.rs @@ -1956,6 +1956,7 @@ impl Editor { let mut element = h_flex() .items_start() + .debug_selector(|| "edit_prediction_diff_popover".into()) .child( h_flex() .bg(cx.theme().colors().editor_background) @@ -2022,6 +2023,7 @@ impl Editor { right: -right_margin, ..Default::default() }); + let popover_right_bound = cmp::min(text_bounds.right(), viewport_bounds.right()); let x_after_longest = Pixels::from( ScrollPixelOffset::from( @@ -2031,14 +2033,12 @@ impl Editor { let element_bounds = element.layout_as_root(AvailableSpace::min_size(), window, cx); - // Fully visible if it can be displayed within the window (allow overlapping other - // panes). However, this is only allowed if the popover starts within text_bounds. let can_position_to_the_right = x_after_longest < text_bounds.right() - && x_after_longest + element_bounds.width < viewport_bounds.right(); + && element_bounds.width <= popover_right_bound - text_bounds.left(); let mut origin = if can_position_to_the_right { point( - x_after_longest, + x_after_longest.min(popover_right_bound - element_bounds.width), text_bounds.origin.y + Pixels::from( edit_start.row().as_f64() * ScrollPixelOffset::from(line_height) diff --git a/crates/editor/src/edit_prediction_tests.rs b/crates/editor/src/edit_prediction_tests.rs index f59c76e4ea..684fea4d54 100644 --- a/crates/editor/src/edit_prediction_tests.rs +++ b/crates/editor/src/edit_prediction_tests.rs @@ -3,7 +3,8 @@ use edit_prediction_types::{ }; use futures::StreamExt; use gpui::{ - Entity, KeyBinding, KeybindingKeystroke, Keystroke, Modifiers, NoAction, Task, prelude::*, + Entity, Focusable, KeyBinding, KeybindingKeystroke, Keystroke, Modifiers, NoAction, Pixels, + Task, prelude::*, size, }; use indoc::indoc; use language::EditPredictionsMode; @@ -25,13 +26,154 @@ use ui::prelude::*; use crate::{ AcceptEditPrediction, CodeContextMenu, CompletionContext, CompletionProvider, EditPrediction, EditPredictionKeybindAction, EditPredictionKeybindSurface, MenuEditPredictionsPolicy, - ShowCompletions, + MultiBuffer, ShowCompletions, editor_tests::{init_test, update_test_language_settings}, - test::{editor_lsp_test_context::EditorLspTestContext, editor_test_context::EditorTestContext}, + test::{ + build_editor, editor_lsp_test_context::EditorLspTestContext, + editor_test_context::EditorTestContext, + }, }; use rpc::proto::PeerId; use workspace::CollaboratorId; +struct EditorWithRightOccluders { + editor: Entity, + editor_width: Pixels, + right_dock_width: Option, + right_sidebar_width: Option, +} + +impl Render for EditorWithRightOccluders { + fn render(&mut self, _window: &mut Window, _cx: &mut Context) -> impl IntoElement { + h_flex() + .size_full() + .child( + div() + .h_full() + .w(self.editor_width) + .overflow_hidden() + .child(self.editor.clone()), + ) + .when_some(self.right_dock_width, |this, width| { + this.child( + div() + .h_full() + .w(width) + .flex_shrink_0() + .occlude() + .debug_selector(|| "right_dock".into()), + ) + }) + .when_some(self.right_sidebar_width, |this, width| { + this.child( + div() + .h_full() + .w(width) + .flex_shrink_0() + .occlude() + .debug_selector(|| "right_sidebar".into()), + ) + }) + } +} + +async fn assert_edit_prediction_diff_popover_avoids_right_occluders( + cx: &mut gpui::TestAppContext, + right_dock_width: Option, + right_sidebar_width: Option, +) { + init_test(cx, |_| {}); + + let editor_width = px(700.); + let window_width = editor_width + + right_dock_width.unwrap_or_default() + + right_sidebar_width.unwrap_or_default(); + let buffer = cx.update(|cx| MultiBuffer::build_simple("", cx)); + let window = cx.add_window(|window, cx| { + let editor = cx.new(|cx| build_editor(buffer, window, cx)); + window.focus(&editor.focus_handle(cx), cx); + EditorWithRightOccluders { + editor, + editor_width, + right_dock_width, + right_sidebar_width, + } + }); + let editor = window + .read_with(cx, |root, _| root.editor.clone()) + .expect("test window should contain editor"); + let mut cx = gpui::VisualTestContext::from_window(*window, cx); + cx.simulate_resize(size(window_width, px(500.))); + cx.run_until_parked(); + + let mut cx = EditorTestContext::for_editor_in(editor, &mut cx).await; + let provider = cx.new(|_| FakeEditPredictionDelegate::default()); + assign_editor_completion_provider(provider.clone(), &mut cx); + cx.update_editor(|editor, _, _| { + editor.set_menu_edit_predictions_policy(MenuEditPredictionsPolicy::Never); + }); + cx.set_state("abcdefghijklmnopqrstuvwxyzabcdefghijklmnˇopqrstuvwxyzabcdef"); + + propose_edits(&provider, vec![(40..41, "REPLACEMENT")], &mut cx); + cx.update_editor(|editor, window, cx| editor.update_visible_edit_prediction(window, cx)); + cx.editor(|editor, _, _| { + assert!(!editor.edit_prediction_visible_in_cursor_popover(true)); + assert!(matches!( + editor + .active_edit_prediction + .as_ref() + .map(|state| &state.completion), + Some(EditPrediction::Edit { + display_mode: crate::EditDisplayMode::DiffPopover, + .. + }) + )); + }); + cx.cx.update(|window, cx| { + window.refresh(); + let _ = window.draw(cx); + }); + + cx.editor(|editor, _, _| { + assert!( + editor.last_position_map.is_some(), + "editor should have rendered a position map" + ); + }); + + let popover_bounds = cx + .cx + .debug_bounds("edit_prediction_diff_popover") + .expect("diff popover should render"); + + for selector in ["right_dock", "right_sidebar"] { + if let Some(occluder_bounds) = cx.cx.debug_bounds(selector) { + assert!( + !popover_bounds.intersects(&occluder_bounds), + "diff popover {popover_bounds:?} should not overlap {selector} {occluder_bounds:?}" + ); + } + } +} + +#[gpui::test] +async fn test_edit_prediction_diff_popover_avoids_right_sidebar(cx: &mut gpui::TestAppContext) { + assert_edit_prediction_diff_popover_avoids_right_occluders(cx, None, Some(px(300.))).await; +} + +#[gpui::test] +async fn test_edit_prediction_diff_popover_avoids_right_dock(cx: &mut gpui::TestAppContext) { + assert_edit_prediction_diff_popover_avoids_right_occluders(cx, Some(px(300.)), None).await; +} + +#[gpui::test] +async fn test_edit_prediction_diff_popover_avoids_right_dock_and_sidebar( + cx: &mut gpui::TestAppContext, +) { + assert_edit_prediction_diff_popover_avoids_right_occluders(cx, Some(px(300.)), Some(px(300.))) + .await; +} + #[gpui::test] async fn test_edit_prediction_insert(cx: &mut gpui::TestAppContext) { init_test(cx, |_| {});