4 files changed, 61 insertions, 18 deletions

diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs
index c149e62b..33d33440 100644
--- a/helix-term/src/commands/lsp.rs
+++ b/helix-term/src/commands/lsp.rs
@@ -598,7 +598,7 @@ pub fn code_action(cx: &mut Context) {
             });
             picker.move_down(); // pre-select the first item
 
-            let popup = Popup::new("code-action", picker);
+            let popup = Popup::new("code-action", picker).with_scrollbar(false);
             compositor.replace_or_push("code-action", popup);
         },
     )
diff --git a/helix-term/src/ui/completion.rs b/helix-term/src/ui/completion.rs
index 545b19d8..229dcda1 100644
--- a/helix-term/src/ui/completion.rs
+++ b/helix-term/src/ui/completion.rs
@@ -227,7 +227,7 @@ impl Completion {
                 }
             };
         });
-        let popup = Popup::new(Self::ID, menu);
+        let popup = Popup::new(Self::ID, menu).with_scrollbar(false);
         let mut completion = Self {
             popup,
             start_offset,
diff --git a/helix-term/src/ui/menu.rs b/helix-term/src/ui/menu.rs
index 1baaf40a..961b7451 100644
--- a/helix-term/src/ui/menu.rs
+++ b/helix-term/src/ui/menu.rs
@@ -320,11 +320,6 @@ impl<T: Item + 'static> Component for Menu<T> {
             (a + b - 1) / b
         }
 
-        let scroll_height = std::cmp::min(div_ceil(win_height.pow(2), len), win_height as usize);
-
-        let scroll_line = (win_height - scroll_height) * scroll
-            / std::cmp::max(1, len.saturating_sub(win_height));
-
         let rows = options.iter().map(|option| option.row(&self.editor_data));
         let table = Table::new(rows)
             .style(style)
@@ -357,20 +352,24 @@ impl<T: Item + 'static> Component for Menu<T> {
         let fits = len <= win_height;
 
         let scroll_style = theme.get("ui.menu.scroll");
-        for (i, _) in (scroll..(scroll + win_height).min(len)).enumerate() {
-            let cell = &mut surface[(area.x + area.width - 1, area.y + i as u16)];
+        if !fits {
+            let scroll_height = div_ceil(win_height.pow(2), len).min(win_height);
+            let scroll_line = (win_height - scroll_height) * scroll
+                / std::cmp::max(1, len.saturating_sub(win_height));
 
-            if !fits {
-                // Draw scroll track
-                cell.set_symbol("▐"); // right half block
-                cell.set_fg(scroll_style.bg.unwrap_or(helix_view::theme::Color::Reset));
-            }
+            let mut cell;
+            for i in 0..win_height {
+                cell = &mut surface[(area.right() - 1, area.top() + i as u16)];
 
-            let is_marked = i >= scroll_line && i < scroll_line + scroll_height;
+                cell.set_symbol("▐"); // right half block
 
-            if !fits && is_marked {
-                // Draw scroll thumb
-                cell.set_fg(scroll_style.fg.unwrap_or(helix_view::theme::Color::Reset));
+                if scroll_line <= i && i < scroll_line + scroll_height {
+                    // Draw scroll thumb
+                    cell.set_fg(scroll_style.fg.unwrap_or(helix_view::theme::Color::Reset));
+                } else {
+                    // Draw scroll track
+                    cell.set_fg(scroll_style.bg.unwrap_or(helix_view::theme::Color::Reset));
+                }
             }
         }
     }
diff --git a/helix-term/src/ui/popup.rs b/helix-term/src/ui/popup.rs
index 3c140da4..62a6785a 100644
--- a/helix-term/src/ui/popup.rs
+++ b/helix-term/src/ui/popup.rs
@@ -22,6 +22,7 @@ pub struct Popup<T: Component> {
     auto_close: bool,
     ignore_escape_key: bool,
     id: &'static str,
+    has_scrollbar: bool,
 }
 
 impl<T: Component> Popup<T> {
@@ -37,6 +38,7 @@ impl<T: Component> Popup<T> {
             auto_close: false,
             ignore_escape_key: false,
             id,
+            has_scrollbar: true,
         }
     }
 
@@ -128,6 +130,14 @@ impl<T: Component> Popup<T> {
         }
     }
 
+    /// Toggles the Popup's scrollbar.
+    /// Consider disabling the scrollbar in case the child
+    /// already has its own.
+    pub fn with_scrollbar(mut self, enable_scrollbar: bool) -> Self {
+        self.has_scrollbar = enable_scrollbar;
+        self
+    }
+
     pub fn contents(&self) -> &T {
         &self.contents
     }
@@ -228,6 +238,40 @@ impl<T: Component> Component for Popup<T> {
 
         let inner = area.inner(&self.margin);
         self.contents.render(inner, surface, cx);
+
+        // render scrollbar if contents do not fit
+        if self.has_scrollbar {
+            let win_height = inner.height as usize;
+            let len = self.child_size.1 as usize;
+            let fits = len <= win_height;
+            let scroll = self.scroll;
+            let scroll_style = cx.editor.theme.get("ui.menu.scroll");
+
+            const fn div_ceil(a: usize, b: usize) -> usize {
+                (a + b - 1) / b
+            }
+
+            if !fits {
+                let scroll_height = div_ceil(win_height.pow(2), len).min(win_height);
+                let scroll_line = (win_height - scroll_height) * scroll
+                    / std::cmp::max(1, len.saturating_sub(win_height));
+
+                let mut cell;
+                for i in 0..win_height {
+                    cell = &mut surface[(inner.right() - 1, inner.top() + i as u16)];
+
+                    cell.set_symbol("▐"); // right half block
+
+                    if scroll_line <= i && i < scroll_line + scroll_height {
+                        // Draw scroll thumb
+                        cell.set_fg(scroll_style.fg.unwrap_or(helix_view::theme::Color::Reset));
+                    } else {
+                        // Draw scroll track
+                        cell.set_fg(scroll_style.bg.unwrap_or(helix_view::theme::Color::Reset));
+                    }
+                }
+            }
+        }
     }
 
     fn id(&self) -> Option<&'static str> {