1 files changed, 42 insertions, 7 deletions

diff --git a/helix-term/src/ui/popup.rs b/helix-term/src/ui/popup.rs
index 8f7921a1..4d319423 100644
--- a/helix-term/src/ui/popup.rs
+++ b/helix-term/src/ui/popup.rs
@@ -6,7 +6,7 @@ use crossterm::event::Event;
 use tui::buffer::Buffer as Surface;
 
 use helix_core::Position;
-use helix_view::graphics::Rect;
+use helix_view::graphics::{Margin, Rect};
 
 // TODO: share logic with Menu, it's essentially Popup(render_fn), but render fn needs to return
 // a width/height hint. maybe Popup(Box<Component>)
@@ -14,17 +14,26 @@ use helix_view::graphics::Rect;
 pub struct Popup<T: Component> {
     contents: T,
     position: Option<Position>,
+    margin: Margin,
     size: (u16, u16),
+    child_size: (u16, u16),
     scroll: usize,
+    id: &'static str,
 }
 
 impl<T: Component> Popup<T> {
-    pub fn new(contents: T) -> Self {
+    pub fn new(id: &'static str, contents: T) -> Self {
         Self {
             contents,
             position: None,
+            margin: Margin {
+                vertical: 0,
+                horizontal: 0,
+            },
             size: (0, 0),
+            child_size: (0, 0),
             scroll: 0,
+            id,
         }
     }
 
@@ -32,6 +41,11 @@ impl<T: Component> Popup<T> {
         self.position = pos;
     }
 
+    pub fn margin(mut self, margin: Margin) -> Self {
+        self.margin = margin;
+        self
+    }
+
     pub fn get_rel_position(&mut self, viewport: Rect, cx: &Context) -> (u16, u16) {
         let position = self
             .position
@@ -68,6 +82,9 @@ impl<T: Component> Popup<T> {
     pub fn scroll(&mut self, offset: usize, direction: bool) {
         if direction {
             self.scroll += offset;
+
+            let max_offset = self.child_size.1.saturating_sub(self.size.1);
+            self.scroll = (self.scroll + offset).min(max_offset as usize);
         } else {
             self.scroll = self.scroll.saturating_sub(offset);
         }
@@ -93,7 +110,7 @@ impl<T: Component> Component for Popup<T> {
             _ => return EventResult::Ignored,
         };
 
-        let close_fn = EventResult::Consumed(Some(Box::new(|compositor: &mut Compositor| {
+        let close_fn = EventResult::Consumed(Some(Box::new(|compositor: &mut Compositor, _| {
             // remove the layer
             compositor.pop();
         })));
@@ -115,13 +132,26 @@ impl<T: Component> Component for Popup<T> {
         // tab/enter/ctrl-k or whatever will confirm the selection/ ctrl-n/ctrl-p for scroll.
     }
 
-    fn required_size(&mut self, _viewport: (u16, u16)) -> Option<(u16, u16)> {
+    fn required_size(&mut self, viewport: (u16, u16)) -> Option<(u16, u16)> {
+        let max_width = 120.min(viewport.0);
+        let max_height = 26.min(viewport.1.saturating_sub(2)); // add some spacing in the viewport
+
+        let inner = Rect::new(0, 0, max_width, max_height).inner(&self.margin);
+
         let (width, height) = self
             .contents
-            .required_size((120, 26)) // max width, max height
+            .required_size((inner.width, inner.height))
             .expect("Component needs required_size implemented in order to be embedded in a popup");
 
-        self.size = (width, height);
+        self.child_size = (width, height);
+        self.size = (
+            (width + self.margin.horizontal * 2).min(max_width),
+            (height + self.margin.vertical * 2).min(max_height),
+        );
+
+        // re-clamp scroll offset
+        let max_offset = self.child_size.1.saturating_sub(self.size.1);
+        self.scroll = self.scroll.min(max_offset as usize);
 
         Some(self.size)
     }
@@ -141,6 +171,11 @@ impl<T: Component> Component for Popup<T> {
         let background = cx.editor.theme.get("ui.popup");
         surface.clear_with(area, background);
 
-        self.contents.render(area, surface, cx);
+        let inner = area.inner(&self.margin);
+        self.contents.render(inner, surface, cx);
+    }
+
+    fn id(&self) -> Option<&'static str> {
+        Some(self.id)
     }
 }