Repository: gitui-org/gitui (language: rust)

Issue:
View arbitrarily long multi-line annotations for git tags
**Is your feature request related to a problem? Please describe.**
Happily, [we can view git tag annotations](https://github.com/extrawurst/gitui/issues/1076), including annotations with multiple lines. Sadly, if the annotation is long and goes on for many lines, we can't see the whole thing. In my current terminal emulator, gitui only displays the first 23 lines.

This issue will increase in importance if we [support creating multiple-line annotations for git tags](https://github.com/extrawurst/gitui/issues/1137).

**Describe the solution you'd like**
Adding a scrollbar seems like a good solution to me.

If adding scrollbars to a "popup window" is not desirable, perhaps refactoring the UI so that annotations can be viewed in a multi-panel interface, like the files list, might work better.

**Additional context**
![Screenshot of gitui with a long git tag annotation and no way to scroll to read the rest of it](https://user-images.githubusercontent.com/80093/154760486-7ba62e33-ddd2-47eb-8d3c-66859bd48a4a.png)


Discussion / hints:
Are there obvious blockers to using `draw_scrollbar` as y'all once did for the revlog? https://github.com/extrawurst/gitui/issues/868
> Are there obvious blockers to using draw_scrollbar 

I don't think so
yeah i think this should be pretty simple to do using the machinery for scrollbars we use in a lot of places already
Hi, I started working on this, i'm adding a new popup with a scrollbar for the tag annotation instead of using MsgPopup. should have a PR up soon :)
Why not the exiting one and add the scrollbar?
Because currently the annotation is shown using:
```
	fn show_annotation(&self) {
		if let Some(tag) = self.selected_tag() {
			if let Some(annotation) = &tag.annotation {
				self.queue.push(InternalEvent::ShowInfoMsg(
					annotation.clone(),
				));
			}
		}
	}
```
Which calls 
```
			InternalEvent::ShowInfoMsg(msg) => {
				self.msg_popup.show_info(msg.as_str())?;
				flags
					.insert(NeedsUpdate::ALL | NeedsUpdate::COMMANDS);
			}
```

I didn't want to add a scrollbar to msg_popup as that is used in other places as well so decided to add a separate popup.

I'm happy to just add a scroll bar to the info popup if that's preferable
I want to choose the solution that's best for the project and keeps things consistent. Would you recommend adding a scrollbar to the MsgPopup, creating a new popup for tag annotations, or is there another approach you'd prefer? Thanks for your input!
you raised a good point. i had to take a look again. my first hunch was: lets use the new multiline textinput but that one does not support readonly right now and is already a rather complex beast.

So in order to keep things simple you can just add vertical scrolling to `MsgPopup` (using `DetailsComponent` maybe for inspiration as it supports multiline scrollable text view already).

Ideally we put that functionality into a separate `ScrollableTextView` component and reuse it between (at least) `MsgPopup` and `DetailsComponent`. Both doing this in one step or in two would be fine to me. WDYT?
@MichaelAug for more instant exchange amongst contributors feel free to join our discord: https://discord.gg/d2p2NS7m
Making a reusable component that is used in `MsgPopup` and `DetailsComponent` sounds good. I can make the change in a single PR or split into 2 (make the `MsgPopup` scrollable in the first PR, then refactor  `DetailsComponent` and MsgPopup to use the generic 'ScrollableTextView' in the second PR), whichever you prefer :)
> @MichaelAug for more instant exchange amongst contributors feel free to join our discord: https://discord.gg/d2p2NS7m

Thanks i'm already in the Discord :+1: 

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6c2a64b68b..e2d50e5b1f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 * re-enable clippy `missing_const_for_fn` linter warning and added const to functions where applicable ([#2116](https://github.com/extrawurst/gitui/issues/2116))
+* Make info and error message popups scrollable [[@MichaelAug](https://github.com/MichaelAug)] ([#1138](https://github.com/extrawurst/gitui/issues/1138))
 
 ## [0.25.1] - 2024-02-23
 
diff --git a/src/popups/msg.rs b/src/popups/msg.rs
index 6da957ce88..759c3b2679 100644
--- a/src/popups/msg.rs
+++ b/src/popups/msg.rs
@@ -1,13 +1,16 @@
 use crate::components::{
 	visibility_blocking, CommandBlocking, CommandInfo, Component,
-	DrawableComponent, EventState,
+	DrawableComponent, EventState, ScrollType, VerticalScroll,
 };
+use crate::strings::order;
 use crate::{
 	app::Environment,
 	keys::{key_match, SharedKeyConfig},
 	strings, ui,
 };
+use anyhow::Result;
 use crossterm::event::Event;
+use ratatui::text::Line;
 use ratatui::{
 	layout::{Alignment, Rect},
 	text::Span,
@@ -22,9 +25,12 @@ pub struct MsgPopup {
 	visible: bool,
 	theme: SharedTheme,
 	key_config: SharedKeyConfig,
+	scroll: VerticalScroll,
 }
 
-use anyhow::Result;
+const POPUP_HEIGHT: u16 = 25;
+const BORDER_WIDTH: u16 = 2;
+const MINIMUM_WIDTH: u16 = 60;
 
 impl DrawableComponent for MsgPopup {
 	fn draw(&self, f: &mut Frame, _rect: Rect) -> Result<()> {
@@ -32,29 +38,55 @@ impl DrawableComponent for MsgPopup {
 			return Ok(());
 		}
 
+		let max_width = f.size().width.max(MINIMUM_WIDTH);
+
 		// determine the maximum width of text block
-		let lens = self
+		let width = self
 			.msg
-			.split('\n')
+			.lines()
 			.map(str::len)
-			.collect::<Vec<usize>>();
-		let mut max = lens.iter().max().expect("max") + 2;
-		if max > std::u16::MAX as usize {
-			max = std::u16::MAX as usize;
-		}
-		let mut width = u16::try_from(max)
-			.expect("can't fail due to check above");
-		// dont overflow screen, and dont get too narrow
-		if width > f.size().width {
-			width = f.size().width;
-		} else if width < 60 {
-			width = 60;
-		}
+			.max()
+			.unwrap_or(0)
+			.saturating_add(BORDER_WIDTH.into())
+			.clamp(MINIMUM_WIDTH.into(), max_width.into())
+			.try_into()
+			.expect("can't fail because we're clamping to u16 value");
+
+		let area =
+			ui::centered_rect_absolute(width, POPUP_HEIGHT, f.size());
+
+		// Wrap lines and break words if there is not enough space
+		let wrapped_msg = bwrap::wrap_maybrk!(
+			&self.msg,
+			area.width.saturating_sub(BORDER_WIDTH).into()
+		);
+
+		let msg_lines: Vec<String> =
+			wrapped_msg.lines().map(String::from).collect();
+		let line_num = msg_lines.len();
+
+		let height = POPUP_HEIGHT
+			.saturating_sub(BORDER_WIDTH)
+			.min(f.size().height.saturating_sub(BORDER_WIDTH));
+
+		let top =
+			self.scroll.update_no_selection(line_num, height.into());
+
+		let scrolled_lines = msg_lines
+			.iter()
+			.skip(top)
+			.take(height.into())
+			.map(|line| {
+				Line::from(vec![Span::styled(
+					line.clone(),
+					self.theme.text(true, false),
+				)])
+			})
+			.collect::<Vec<Line>>();
 
-		let area = ui::centered_rect_absolute(width, 25, f.size());
 		f.render_widget(Clear, area);
 		f.render_widget(
-			Paragraph::new(self.msg.clone())
+			Paragraph::new(scrolled_lines)
 				.block(
 					Block::default()
 						.title(Span::styled(
@@ -69,6 +101,8 @@ impl DrawableComponent for MsgPopup {
 			area,
 		);
 
+		self.scroll.draw(f, area, &self.theme);
+
 		Ok(())
 	}
 }
@@ -84,6 +118,16 @@ impl Component for MsgPopup {
 			true,
 			self.visible,
 		));
+		out.push(
+			CommandInfo::new(
+				strings::commands::navigate_commit_message(
+					&self.key_config,
+				),
+				true,
+				self.visible,
+			)
+			.order(order::NAV),
+		);
 
 		visibility_blocking(self)
 	}
@@ -93,6 +137,14 @@ impl Component for MsgPopup {
 			if let Event::Key(e) = ev {
 				if key_match(e, self.key_config.keys.enter) {
 					self.hide();
+				} else if key_match(
+					e,
+					self.key_config.keys.popup_down,
+				) {
+					self.scroll.move_top(ScrollType::Down);
+				} else if key_match(e, self.key_config.keys.popup_up)
+				{
+					self.scroll.move_top(ScrollType::Up);
 				}
 			}
 			Ok(EventState::Consumed)
@@ -124,24 +176,34 @@ impl MsgPopup {
 			visible: false,
 			theme: env.theme.clone(),
 			key_config: env.key_config.clone(),
+			scroll: VerticalScroll::new(),
 		}
 	}
 
-	///
-	pub fn show_error(&mut self, msg: &str) -> Result<()> {
-		self.title = strings::msg_title_error(&self.key_config);
+	fn set_new_msg(
+		&mut self,
+		msg: &str,
+		title: String,
+	) -> Result<()> {
+		self.title = title;
 		self.msg = msg.to_string();
-		self.show()?;
+		self.scroll.reset();
+		self.show()
+	}
 
-		Ok(())
+	///
+	pub fn show_error(&mut self, msg: &str) -> Result<()> {
+		self.set_new_msg(
+			msg,
+			strings::msg_title_error(&self.key_config),
+		)
 	}
 
 	///
 	pub fn show_info(&mut self, msg: &str) -> Result<()> {
-		self.title = strings::msg_title_info(&self.key_config);
-		self.msg = msg.to_string();
-		self.show()?;
-
-		Ok(())
+		self.set_new_msg(
+			msg,
+			strings::msg_title_info(&self.key_config),
+		)
 	}
 }


Test patch: