Fix some visual bugs w/ edit predictions (#24591)

* correct the size of key binding icons
* avoid spurious modifier in 'jump to edit' popover when already
previewing
* fix height of the edit preview popover

Release Notes:

- N/A

Co-authored-by: agu-z <hi@aguz.me>

crates/editor/src/editor.rs

@@ -5606,10 +5606,10 @@ impl Editor {
         if provider.provider.needs_terms_acceptance(cx) {
             return Some(
                 h_flex()
-                    .h(self.edit_prediction_cursor_popover_height())
                     .min_w(min_width)
                     .flex_1()
                     .px_2()
+                    .py_1()
                     .gap_3()
                     .elevation_2(cx)
                     .hover(|style| style.bg(cx.theme().colors().element_hover))
@@ -5699,11 +5699,11 @@ impl Editor {
 
         Some(
             h_flex()
-                .h(self.edit_prediction_cursor_popover_height())
                 .min_w(min_width)
                 .max_w(max_width)
                 .flex_1()
                 .px_2()
+                .py_1()
                 .elevation_2(cx)
                 .child(completion)
                 .child(ui::Divider::vertical())

crates/editor/src/element.rs

@@ -3560,13 +3560,11 @@ impl EditorElement {
 
         match &active_inline_completion.completion {
             InlineCompletion::Move { target, .. } => {
-                let previewing = false;
                 let target_display_point = target.to_display_point(editor_snapshot);
                 if target_display_point.row().as_f32() < scroll_top {
                     let mut element = inline_completion_accept_indicator(
                         "Jump to Edit",
                         Some(IconName::ArrowUp),
-                        previewing,
                         editor,
                         window,
                         cx,
@@ -3579,7 +3577,6 @@ impl EditorElement {
                     let mut element = inline_completion_accept_indicator(
                         "Jump to Edit",
                         Some(IconName::ArrowDown),
-                        previewing,
                         editor,
                         window,
                         cx,
@@ -3595,7 +3592,6 @@ impl EditorElement {
                     let mut element = inline_completion_accept_indicator(
                         "Jump to Edit",
                         None,
-                        previewing,
                         editor,
                         window,
                         cx,
@@ -3658,12 +3654,7 @@ impl EditorElement {
                         let (mut element, origin) = self.editor.update(cx, |editor, cx| {
                             Some((
                                 inline_completion_accept_indicator(
-                                    "Accept",
+                                    "Accept", None, editor, window, cx,
-                                    None,
-                                    editor.previewing_inline_completion,
-                                    editor,
-                                    window,
-                                    cx,
                                 )?,
                                 editor.display_to_pixel_point(
                                     target_line_end,
@@ -5669,7 +5660,6 @@ fn header_jump_data(
 fn inline_completion_accept_indicator(
     label: impl Into<SharedString>,
     icon: Option<IconName>,
-    previewing: bool,
     editor: &Editor,
     window: &mut Window,
     cx: &App,
@@ -5683,7 +5673,7 @@ fn inline_completion_accept_indicator(
         .text_size(TextSize::XSmall.rems(cx))
         .text_color(cx.theme().colors().text)
         .gap_1()
-        .when(!previewing, |parent| {
+        .when(!editor.previewing_inline_completion, |parent| {
             parent.children(ui::render_modifiers(
                 &accept_keystroke.modifiers,
                 PlatformStyle::platform(),

crates/gpui/src/geometry.rs

@@ -2975,6 +2975,22 @@ impl AbsoluteLength {
             AbsoluteLength::Rems(rems) => rems.to_pixels(rem_size),
         }
     }
+
+    /// Converts an `AbsoluteLength` to `Rems` based on a given `rem_size`.
+    ///
+    /// # Arguments
+    ///
+    /// * `rem_size` - The size of one rem in pixels.
+    ///
+    /// # Returns
+    ///
+    /// Returns the `AbsoluteLength` as `Pixels`.
+    pub fn to_rems(&self, rem_size: Pixels) -> Rems {
+        match self {
+            AbsoluteLength::Pixels(pixels) => Rems(pixels.0 / rem_size.0),
+            AbsoluteLength::Rems(rems) => *rems,
+        }
+    }
 }
 
 impl Default for AbsoluteLength {

crates/ui/src/components/icon.rs

@@ -66,7 +66,7 @@ pub enum IconSize {
     Medium,
     /// 48px
     XLarge,
-    Custom(Pixels),
+    Custom(Rems),
 }
 
 impl IconSize {
@@ -77,7 +77,7 @@ impl IconSize {
             IconSize::Small => rems_from_px(14.),
             IconSize::Medium => rems_from_px(16.),
             IconSize::XLarge => rems_from_px(48.),
-            IconSize::Custom(size) => rems_from_px(size.into()),
+            IconSize::Custom(size) => size,
         }
     }
 
@@ -95,7 +95,7 @@ impl IconSize {
             IconSize::Medium => DynamicSpacing::Base02.px(cx),
             IconSize::XLarge => DynamicSpacing::Base02.px(cx),
             // TODO: Wire into dynamic spacing
-            IconSize::Custom(size) => px(size.into()),
+            IconSize::Custom(size) => size.to_pixels(window.rem_size()),
         };
 
         (icon_size, padding)

crates/ui/src/components/keybinding.rs

@@ -15,7 +15,7 @@ pub struct KeyBinding {
 
     /// The [`PlatformStyle`] to use when displaying this keybinding.
     platform_style: PlatformStyle,
-    size: Option<Pixels>,
+    size: Option<AbsoluteLength>,
 }
 
 impl KeyBinding {
@@ -59,8 +59,8 @@ impl KeyBinding {
     }
 
     /// Sets the size for this [`KeyBinding`].
-    pub fn size(mut self, size: Pixels) -> Self {
+    pub fn size(mut self, size: impl Into<AbsoluteLength>) -> Self {
-        self.size = Some(size);
+        self.size = Some(size.into());
         self
     }
 }
@@ -105,7 +105,7 @@ pub fn render_key(
     keystroke: &Keystroke,
     platform_style: PlatformStyle,
     color: Option<Color>,
-    size: Option<Pixels>,
+    size: Option<AbsoluteLength>,
 ) -> AnyElement {
     let key_icon = icon_for_key(keystroke, platform_style);
     match key_icon {
@@ -144,7 +144,7 @@ pub fn render_modifiers(
     modifiers: &Modifiers,
     platform_style: PlatformStyle,
     color: Option<Color>,
-    size: Option<Pixels>,
+    size: Option<AbsoluteLength>,
     standalone: bool,
 ) -> impl Iterator<Item = AnyElement> {
     enum KeyOrIcon {
@@ -224,14 +224,13 @@ pub fn render_modifiers(
 pub struct Key {
     key: SharedString,
     color: Option<Color>,
-    size: Option<Pixels>,
+    size: Option<AbsoluteLength>,
 }
 
 impl RenderOnce for Key {
     fn render(self, _window: &mut Window, cx: &mut App) -> impl IntoElement {
         let single_char = self.key.len() == 1;
-        let size = self.size.unwrap_or(px(14.));
+        let size = self.size.unwrap_or(px(14.).into());
-        let size_f32: f32 = size.into();
 
         div()
             .py_0()
@@ -242,7 +241,7 @@ impl RenderOnce for Key {
                     this.px_0p5()
                 }
             })
-            .h(rems_from_px(size_f32))
+            .h(size)
             .text_size(size)
             .line_height(relative(1.))
             .text_color(self.color.unwrap_or(Color::Muted).color(cx))
@@ -259,7 +258,7 @@ impl Key {
         }
     }
 
-    pub fn size(mut self, size: impl Into<Option<Pixels>>) -> Self {
+    pub fn size(mut self, size: impl Into<Option<AbsoluteLength>>) -> Self {
         self.size = size.into();
         self
     }
@@ -269,17 +268,15 @@ impl Key {
 pub struct KeyIcon {
     icon: IconName,
     color: Option<Color>,
-    size: Option<Pixels>,
+    size: Option<AbsoluteLength>,
 }
 
 impl RenderOnce for KeyIcon {
     fn render(self, window: &mut Window, _cx: &mut App) -> impl IntoElement {
-        let size = self
+        let size = self.size.unwrap_or(IconSize::Small.rems().into());
-            .size
-            .unwrap_or(IconSize::Small.rems().to_pixels(window.rem_size()));
 
         Icon::new(self.icon)
-            .size(IconSize::Custom(size))
+            .size(IconSize::Custom(size.to_rems(window.rem_size())))
             .color(self.color.unwrap_or(Color::Muted))
     }
 }
@@ -293,7 +290,7 @@ impl KeyIcon {
         }
     }
 
-    pub fn size(mut self, size: impl Into<Option<Pixels>>) -> Self {
+    pub fn size(mut self, size: impl Into<Option<AbsoluteLength>>) -> Self {
         self.size = size.into();
         self
     }

crates/ui/src/components/keybinding_hint.rs

@@ -199,7 +199,7 @@ impl RenderOnce for KeybindingHint {
                         blur_radius: px(0.),
                         spread_radius: px(0.),
                     }])
-                    .child(self.keybinding.size(kb_size)),
+                    .child(self.keybinding.size(rems_from_px(kb_size.0))),
             )
             .children(self.suffix)
     }