1 files changed, 24 insertions, 5 deletions

diff --git a/helix-term/src/ui/popup.rs b/helix-term/src/ui/popup.rs
index 8f7921a1..bf7510a2 100644
--- a/helix-term/src/ui/popup.rs
+++ b/helix-term/src/ui/popup.rs
@@ -15,16 +15,20 @@ pub struct Popup<T: Component> {
     contents: T,
     position: Option<Position>,
     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,
             size: (0, 0),
+            child_size: (0, 0),
             scroll: 0,
+            id,
         }
     }
 
@@ -68,6 +72,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 +100,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 +122,21 @@ 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 (width, height) = self
             .contents
-            .required_size((120, 26)) // max width, max height
+            .required_size((max_width, max_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.min(max_width), height.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)
     }
@@ -143,4 +158,8 @@ impl<T: Component> Component for Popup<T> {
 
         self.contents.render(area, surface, cx);
     }
+
+    fn id(&self) -> Option<&'static str> {
+        Some(self.id)
+    }
 }