From 7dace113d6d92fbb896f1d70e314292dfb54a61e Mon Sep 17 00:00:00 2001 From: Michael Augaitis Date: Sun, 10 Mar 2024 12:55:53 +0000 Subject: [PATCH 1/6] Add VerticalScroll and navigation CommandInfo to MsgPopup --- src/popups/msg.rs | 121 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 92 insertions(+), 29 deletions(-) diff --git a/src/popups/msg.rs b/src/popups/msg.rs index 6da957ce88..1b60748b60 100644 --- a/src/popups/msg.rs +++ b/src/popups/msg.rs @@ -1,13 +1,18 @@ +use std::cell::Cell; + 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 +27,13 @@ pub struct MsgPopup { visible: bool, theme: SharedTheme, key_config: SharedKeyConfig, + scroll: VerticalScroll, + current_width: Cell, } -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<()> { @@ -33,28 +42,51 @@ impl DrawableComponent for MsgPopup { } // determine the maximum width of text block - let lens = self + let width = self .msg - .split('\n') + .lines() .map(str::len) - .collect::>(); - 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() + .expect("Expected at least one line in the message") + .saturating_add(BORDER_WIDTH.into()) + .clamp(BORDER_WIDTH.into(), std::u16::MAX as usize); + let width = u16::try_from(width) + .expect("can't fail due to clamp above") + .clamp(MINIMUM_WIDTH, f.size().width); + + self.current_width.set(width); + + let wrapped_msg = bwrap::wrap!( + &self.msg, + width.saturating_sub(BORDER_WIDTH).into() + ); + + let msg_lines: Vec = + wrapped_msg.lines().map(String::from).collect(); + let line_num = msg_lines.len(); + let height = POPUP_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::>(); + + let area = + ui::centered_rect_absolute(width, POPUP_HEIGHT, f.size()); - 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,35 @@ impl MsgPopup { visible: false, theme: env.theme.clone(), key_config: env.key_config.clone(), + scroll: VerticalScroll::new(), + current_width: Cell::new(0), } } - /// - 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), + ) } } From 5634d532f1efaa2cd778dd99ad157061209ac19b Mon Sep 17 00:00:00 2001 From: Michael Augaitis Date: Sun, 10 Mar 2024 17:04:31 +0000 Subject: [PATCH 2/6] Addd changelog entry for scrollable popups --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ccdcd38c5c..e92a9b1fb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,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 ([#1138](https://github.com/extrawurst/gitui/issues/1138)) ## [0.25.1] - 2024-02-23 From a476532fa7e986e812d3fcce14042574dc9afba0 Mon Sep 17 00:00:00 2001 From: Michael Augaitis Date: Sat, 16 Mar 2024 18:29:18 +0000 Subject: [PATCH 3/6] Account for popup height going below fixed height If gitui window shrinks below set height, scrollbar will correctly be displayed and text will scroll correctly. Fix panic when clamping width if f.size().width went below MINIMUM_WIDTH. --- src/popups/msg.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/popups/msg.rs b/src/popups/msg.rs index 1b60748b60..2a31557b9d 100644 --- a/src/popups/msg.rs +++ b/src/popups/msg.rs @@ -41,6 +41,8 @@ impl DrawableComponent for MsgPopup { return Ok(()); } + let max_width = f.size().width.max(MINIMUM_WIDTH); + // determine the maximum width of text block let width = self .msg @@ -49,10 +51,9 @@ impl DrawableComponent for MsgPopup { .max() .expect("Expected at least one line in the message") .saturating_add(BORDER_WIDTH.into()) - .clamp(BORDER_WIDTH.into(), std::u16::MAX as usize); - let width = u16::try_from(width) - .expect("can't fail due to clamp above") - .clamp(MINIMUM_WIDTH, f.size().width); + .clamp(MINIMUM_WIDTH.into(), max_width.into()) + .try_into() + .expect("can't fail because we're clamping to u16 value"); self.current_width.set(width); @@ -64,7 +65,10 @@ impl DrawableComponent for MsgPopup { let msg_lines: Vec = wrapped_msg.lines().map(String::from).collect(); let line_num = msg_lines.len(); - let height = POPUP_HEIGHT.saturating_sub(BORDER_WIDTH); + + 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()); From e9c29dc8f178bdb0fbb9b5f156f71c1e116b3d45 Mon Sep 17 00:00:00 2001 From: Michael Augaitis Date: Sun, 24 Mar 2024 17:15:22 +0000 Subject: [PATCH 4/6] Use rect width for wrapping message lines Now if the window shrinks below defined width, the message lines will wrap correctly. Allow words to be split (broken) to fit the rect size by using wrap_maybrk. Handle panic if msg is an empty string. --- src/popups/msg.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/popups/msg.rs b/src/popups/msg.rs index 2a31557b9d..3baafed6b6 100644 --- a/src/popups/msg.rs +++ b/src/popups/msg.rs @@ -49,17 +49,22 @@ impl DrawableComponent for MsgPopup { .lines() .map(str::len) .max() - .expect("Expected at least one line in the message") + .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"); - self.current_width.set(width); - let wrapped_msg = bwrap::wrap!( + let area = + ui::centered_rect_absolute(width, POPUP_HEIGHT, f.size()); + + self.current_width.set(area.width); + + // Wrap lines and break words if there is not enough space + let wrapped_msg = bwrap::wrap_maybrk!( &self.msg, - width.saturating_sub(BORDER_WIDTH).into() + area.width.saturating_sub(BORDER_WIDTH).into() ); let msg_lines: Vec = @@ -85,9 +90,6 @@ impl DrawableComponent for MsgPopup { }) .collect::>(); - let area = - ui::centered_rect_absolute(width, POPUP_HEIGHT, f.size()); - f.render_widget(Clear, area); f.render_widget( Paragraph::new(scrolled_lines) From 9f7c1db837aee300eebe4b44db6ceaf55a4e2c69 Mon Sep 17 00:00:00 2001 From: Michael Augaitis Date: Sun, 24 Mar 2024 17:18:07 +0000 Subject: [PATCH 5/6] Fix changelog entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e92a9b1fb1..afbd86ea3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,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 ([#1138](https://github.com/extrawurst/gitui/issues/1138)) +* 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 From 8e9379da4e80f322da10b9169a39482f17b680b9 Mon Sep 17 00:00:00 2001 From: Michael Augaitis Date: Sun, 24 Mar 2024 17:22:17 +0000 Subject: [PATCH 6/6] Remove unused variable in MsgPopup --- src/popups/msg.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/popups/msg.rs b/src/popups/msg.rs index 3baafed6b6..759c3b2679 100644 --- a/src/popups/msg.rs +++ b/src/popups/msg.rs @@ -1,5 +1,3 @@ -use std::cell::Cell; - use crate::components::{ visibility_blocking, CommandBlocking, CommandInfo, Component, DrawableComponent, EventState, ScrollType, VerticalScroll, @@ -28,7 +26,6 @@ pub struct MsgPopup { theme: SharedTheme, key_config: SharedKeyConfig, scroll: VerticalScroll, - current_width: Cell, } const POPUP_HEIGHT: u16 = 25; @@ -55,12 +52,9 @@ impl DrawableComponent for MsgPopup { .try_into() .expect("can't fail because we're clamping to u16 value"); - let area = ui::centered_rect_absolute(width, POPUP_HEIGHT, f.size()); - self.current_width.set(area.width); - // Wrap lines and break words if there is not enough space let wrapped_msg = bwrap::wrap_maybrk!( &self.msg, @@ -183,7 +177,6 @@ impl MsgPopup { theme: env.theme.clone(), key_config: env.key_config.clone(), scroll: VerticalScroll::new(), - current_width: Cell::new(0), } }