Repository: osa1/tiny (language: rust)

Issue:
Invalid scroll value in compact layout after resizing
![Screenshot_2021-05-03_10-32-08](https://user-images.githubusercontent.com/448274/116851332-dd6cc980-abfa-11eb-9988-dd86bbbc7f06.png)

In the screenshot above I'm in a channel full of messages, but it shows empty. The reason is because I made the window very small, and scrolled to the top. Then I made the window large again, but the scroll value is not updated, so even though I have less lines to render now, say, 100, the scroll value is something like 1000. As a result the tab doesn't show any messages.

The repro instructions is similar to #318, except after making the window smaller:

- Scroll to the top with `page-up` or `home`
- Make the window larger

You should see the tab empty, as shown in the screenshot above.

cc @trevarj 
Invalid scroll value in compact layout after resizing
![Screenshot_2021-05-03_10-32-08](https://user-images.githubusercontent.com/448274/116851332-dd6cc980-abfa-11eb-9988-dd86bbbc7f06.png)

In the screenshot above I'm in a channel full of messages, but it shows empty. The reason is because I made the window very small, and scrolled to the top. Then I made the window large again, but the scroll value is not updated, so even though I have less lines to render now, say, 100, the scroll value is something like 1000. As a result the tab doesn't show any messages.

The repro instructions is similar to #318, except after making the window smaller:

- Scroll to the top with `page-up` or `home`
- Make the window larger

You should see the tab empty, as shown in the screenshot above.

cc @trevarj 


Discussion / hints:

Fix patch:
diff --git a/crates/libtiny_tui/src/msg_area/mod.rs b/crates/libtiny_tui/src/msg_area/mod.rs
index 24371209..32b204ec 100644
--- a/crates/libtiny_tui/src/msg_area/mod.rs
+++ b/crates/libtiny_tui/src/msg_area/mod.rs
@@ -68,8 +68,13 @@ impl MsgArea {
 
     pub(crate) fn resize(&mut self, width: i32, height: i32) {
         self.width = width;
+        let old_height = self.height;
         self.height = height;
+        let old_total_lines = self.lines_height;
         self.lines_height = None;
+
+        self.update_total_visible_lines();
+        self.recalculate_scroll(old_height, old_total_lines.unwrap());
     }
 
     pub(crate) fn layout(&self) -> Layout {
@@ -128,7 +133,8 @@ impl MsgArea {
 // Scrolling
 
 impl MsgArea {
-    fn lines_height(&mut self) -> i32 {
+    /// The total number of visible lines if each Line was rendered at the current screen width
+    fn update_total_visible_lines(&mut self) -> i32 {
         match self.lines_height {
             Some(height) => height,
             None => {
@@ -143,7 +149,7 @@ impl MsgArea {
     }
 
     pub(crate) fn scroll_up(&mut self) {
-        if self.scroll < max(0, self.lines_height() - self.height) {
+        if self.scroll < max(0, self.update_total_visible_lines() - self.height) {
             self.scroll += 1;
         }
     }
@@ -155,7 +161,7 @@ impl MsgArea {
     }
 
     pub(crate) fn scroll_top(&mut self) {
-        self.scroll = max(0, self.lines_height() - self.height);
+        self.scroll = max(0, self.update_total_visible_lines() - self.height);
     }
 
     pub(crate) fn scroll_bottom(&mut self) {
@@ -171,6 +177,15 @@ impl MsgArea {
     pub(crate) fn page_down(&mut self) {
         self.scroll = max(0, self.scroll - 10);
     }
+
+    /// Recalculate the scroll offset due to resizing of the window
+    fn recalculate_scroll(&mut self, old_height: i32, old_total_lines: i32) {
+        if self.scroll > 0 {
+            let ratio = (self.scroll as f32 + old_height as f32) / old_total_lines as f32;
+            let total_lines = self.update_total_visible_lines();
+            self.scroll = max(0, ((ratio * total_lines as f32) as i32) - self.height);
+        }
+    }
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -257,6 +272,6 @@ mod tests {
         // Will pop out "first" line
         msg_area.flush_line();
         assert_eq!(msg_area.lines.len(), 3);
-        assert_eq!(msg_area.lines_height(), 3);
+        assert_eq!(msg_area.update_total_visible_lines(), 3);
     }
 }


Test patch:
diff --git a/crates/libtiny_tui/src/tests/layout.rs b/crates/libtiny_tui/src/tests/layout.rs
new file mode 100644
index 00000000..7c92be0b
--- /dev/null
+++ b/crates/libtiny_tui/src/tests/layout.rs
@@ -0,0 +1,70 @@
+use std::panic::Location;
+
+use libtiny_common::{ChanNameRef, MsgTarget};
+
+use crate::msg_area::Layout;
+use crate::test_utils::expect_screen;
+use crate::tui::TUI;
+
+#[test]
+fn test_join_part_overflow() {
+    let mut tui = TUI::new_test(21, 4);
+    let serv = "irc.server_1.org";
+    let chan = ChanNameRef::new("#chan");
+    tui.new_server_tab(serv, None);
+    tui.set_nick(serv, "osa1");
+    tui.new_chan_tab(serv, chan);
+    tui.next_tab();
+    tui.next_tab();
+
+    let target = MsgTarget::Chan { serv, chan };
+    let ts = time::at_utc(time::Timespec::new(0, 0));
+    tui.add_nick("123456", Some(ts), &target);
+    tui.add_nick("abcdef", Some(ts), &target);
+    tui.add_nick("hijklm", Some(ts), &target);
+    tui.draw();
+
+    #[rustfmt::skip]
+    let screen =
+        "|00:00 +123456 +abcdef|
+         |+hijklm              |
+         |osa1:                |
+         |< #chan              |";
+
+    expect_screen(screen, &tui.get_front_buffer(), 21, 4, Location::caller());
+}
+
+#[test]
+fn test_alignment_long_string() {
+    let mut tui = TUI::new_test(40, 5);
+    tui.set_layout(Layout::Aligned { max_nick_len: 12 });
+    let serv = "irc.server_1.org";
+    let chan = ChanNameRef::new("#chan");
+    tui.new_server_tab(serv, None);
+    tui.set_nick(serv, "osa1");
+    tui.new_chan_tab(serv, chan);
+    tui.next_tab();
+    tui.next_tab();
+
+    let target = MsgTarget::Chan { serv, chan };
+    let ts = time::at_utc(time::Timespec::new(0, 0));
+    tui.add_privmsg(
+        "osa1",
+        "123456789012345678901234567890",
+        ts,
+        &target,
+        false,
+        false,
+    );
+    tui.draw();
+
+    #[rustfmt::skip]
+    let screen =
+        "|                                        |
+         |00:00         osa1: 12345678901234567890|
+         |                    1234567890          |
+         |osa1:                                   |
+         |mentions irc.server_1.org #chan         |";
+
+    expect_screen(screen, &tui.get_front_buffer(), 40, 5, Location::caller());
+}
diff --git a/crates/libtiny_tui/src/tests/mod.rs b/crates/libtiny_tui/src/tests/mod.rs
index 85ccc76b..f24a9a10 100644
--- a/crates/libtiny_tui/src/tests/mod.rs
+++ b/crates/libtiny_tui/src/tests/mod.rs
@@ -1,13 +1,12 @@
-use crate::msg_area::Layout;
-use crate::tui::TUI;
+use std::panic::Location;
 
-use crate::test_utils::expect_screen;
 use libtiny_common::{ChanNameRef, MsgTarget};
 use term_input::{Event, Key};
 
-use std::fs::File;
-use std::io::{BufRead, BufReader};
-use std::panic::Location;
+use crate::test_utils::expect_screen;
+use crate::tui::TUI;
+
+mod resize;
 
 fn enter_string(tui: &mut TUI, s: &str) {
     for c in s.chars() {
@@ -380,112 +379,3 @@ fn test_text_field_wrap() {
     // TODO: Test changing nick (osa: I don't understand how nick length is taken into account when
     // falling back to scrolling)
 }
-
-#[test]
-fn test_join_part_overflow() {
-    let mut tui = TUI::new_test(21, 4);
-    let serv = "irc.server_1.org";
-    let chan = ChanNameRef::new("#chan");
-    tui.new_server_tab(serv, None);
-    tui.set_nick(serv, "osa1");
-    tui.new_chan_tab(serv, chan);
-    tui.next_tab();
-    tui.next_tab();
-
-    let target = MsgTarget::Chan { serv, chan };
-    let ts = time::at_utc(time::Timespec::new(0, 0));
-    tui.add_nick("123456", Some(ts), &target);
-    tui.add_nick("abcdef", Some(ts), &target);
-    tui.add_nick("hijklm", Some(ts), &target);
-    tui.draw();
-
-    #[rustfmt::skip]
-    let screen =
-        "|00:00 +123456 +abcdef|
-         |+hijklm              |
-         |osa1:                |
-         |< #chan              |";
-
-    expect_screen(screen, &tui.get_front_buffer(), 21, 4, Location::caller());
-}
-
-#[test]
-fn test_alignment_long_string() {
-    let mut tui = TUI::new_test(40, 5);
-    tui.set_layout(Layout::Aligned { max_nick_len: 12 });
-    let serv = "irc.server_1.org";
-    let chan = ChanNameRef::new("#chan");
-    tui.new_server_tab(serv, None);
-    tui.set_nick(serv, "osa1");
-    tui.new_chan_tab(serv, chan);
-    tui.next_tab();
-    tui.next_tab();
-
-    let target = MsgTarget::Chan { serv, chan };
-    let ts = time::at_utc(time::Timespec::new(0, 0));
-    tui.add_privmsg(
-        "osa1",
-        "123456789012345678901234567890",
-        ts,
-        &target,
-        false,
-        false,
-    );
-    tui.draw();
-
-    #[rustfmt::skip]
-    let screen =
-        "|                                        |
-         |00:00         osa1: 12345678901234567890|
-         |                    1234567890          |
-         |osa1:                                   |
-         |mentions irc.server_1.org #chan         |";
-
-    expect_screen(screen, &tui.get_front_buffer(), 40, 5, Location::caller());
-}
-#[test]
-fn test_resize() {
-    let mut tui = TUI::new_test(80, 50);
-
-    let server = "<server>";
-    tui.new_server_tab(server, None);
-
-    let ts = time::empty_tm();
-    let target = MsgTarget::CurrentTab;
-
-    let f = File::open("test/lipsum.txt").unwrap();
-    let f = BufReader::new(f);
-    for line in f.lines() {
-        let line = line.unwrap();
-        tui.add_msg(&line, ts, &target);
-    }
-
-    let mut w = 80;
-    let mut h = 50;
-
-    for _ in 0..50 {
-        w -= 1;
-        h -= 1;
-        tui.set_size(w, h);
-        tui.draw();
-    }
-
-    for _ in 0..30 {
-        w -= 1;
-        tui.set_size(w, h);
-        tui.draw();
-    }
-
-    for _ in 0..50 {
-        w += 1;
-        h += 1;
-        tui.set_size(w, h);
-        tui.draw();
-    }
-
-    for _ in 0..30 {
-        w += 1;
-        tui.set_size(w, h);
-        tui.draw();
-    }
-}
diff --git a/crates/libtiny_tui/src/tests/resize.rs b/crates/libtiny_tui/src/tests/resize.rs
new file mode 100644
index 00000000..df1b9d92
--- /dev/null
+++ b/crates/libtiny_tui/src/tests/resize.rs
@@ -0,0 +1,317 @@
+use std::fs::File;
+use std::io::{BufRead, BufReader};
+use std::panic::Location;
+
+use libtiny_common::{ChanNameRef, MsgTarget};
+use term_input::{Arrow, Key};
+
+use crate::test_utils::expect_screen;
+use crate::tui::TUI;
+
+#[test]
+fn test_resize_recalc_scroll() {
+    let mut tui = TUI::new_test(15, 5);
+    let serv = "irc.server_1.org";
+    let chan = ChanNameRef::new("#chan");
+    tui.new_server_tab(serv, None);
+    tui.set_nick(serv, "osa1");
+    tui.new_chan_tab(serv, chan);
+    tui.next_tab();
+    tui.next_tab();
+
+    let target = MsgTarget::Chan { serv, chan };
+    let ts = time::at_utc(time::Timespec::new(0, 0));
+    tui.add_privmsg(
+        "osa1",
+        "s 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 1111 e",
+        ts,
+        &target,
+        false,
+        false,
+    );
+
+    tui.draw();
+
+    // at bottom with no scroll
+    #[rustfmt::skip]
+    let screen1 =
+       "|1111 1111 1111 |
+        |1111 1111 1111 |
+        |1111 e         |
+        |               |
+        |< #chan        |";
+
+    expect_screen(screen1, &tui.get_front_buffer(), 15, 5, Location::caller());
+
+    // hit the home key to go to the top of the messages and then resize the screen
+    let home = term_input::Event::Key(Key::Home);
+    tui.handle_input_event(home, &mut None);
+    tui.set_size(16, 7);
+    tui.draw();
+
+    // should be at the top of message after resize
+    #[rustfmt::skip]
+    let screen2 =
+       "|00:00 osa1: s   |
+        |1111 1111 1111  |
+        |1111 1111 1111  |
+        |1111 1111 1111  |
+        |1111 1111 1111  |
+        |                |
+        |< #chan         |";
+
+    expect_screen(screen2, &tui.get_front_buffer(), 16, 7, Location::caller());
+
+    // go back to the bottom
+    let end = term_input::Event::Key(Key::End);
+    tui.handle_input_event(end, &mut None);
+    tui.draw();
+
+    // go back to the bottom
+    #[rustfmt::skip]
+    let screen3 = 
+       "|1111 1111 1111  |
+        |1111 1111 1111  |
+        |1111 1111 1111  |
+        |1111 1111 1111  |
+        |1111 e          |
+        |                |
+        |< #chan         |";
+
+    expect_screen(screen3, &tui.get_front_buffer(), 16, 7, Location::caller());
+}
+
+#[test]
+fn test_resize_scroll_stick_to_top() {
+    let mut tui = TUI::new_test(18, 10);
+    let serv = "irc.server_1.org";
+    let chan = ChanNameRef::new("#chan");
+    tui.new_server_tab(serv, None);
+    tui.set_nick(serv, "osa1");
+    tui.new_chan_tab(serv, chan);
+    tui.next_tab();
+    tui.next_tab();
+
+    let target = MsgTarget::Chan { serv, chan };
+    let ts = time::at_utc(time::Timespec::new(0, 0));
+
+    for i in 0..15 {
+        tui.add_privmsg("osa1", &format!("line{}", i), ts, &target, false, false);
+    }
+
+    tui.draw();
+
+    #[rustfmt::skip]
+    let screen1 =
+       "|osa1: line7       |
+        |osa1: line8       |
+        |osa1: line9       |
+        |osa1: line10      |
+        |osa1: line11      |
+        |osa1: line12      |
+        |osa1: line13      |
+        |osa1: line14      |
+        |                  |
+        |< #chan           |";
+
+    expect_screen(screen1, &tui.get_front_buffer(), 18, 10, Location::caller());
+
+    // scroll up two lines, resize to add one extra line and verify that the next line on the bottom shows
+    for _ in 0..2 {
+        tui.handle_input_event(
+            term_input::Event::Key(Key::ShiftArrow(Arrow::Up)),
+            &mut None,
+        );
+    }
+    tui.draw();
+    tui.set_size(18, 11);
+    tui.draw();
+
+    #[rustfmt::skip]
+    let screen2 =
+       "|osa1: line5       |
+        |osa1: line6       |
+        |osa1: line7       |
+        |osa1: line8       |
+        |osa1: line9       |
+        |osa1: line10      |
+        |osa1: line11      |
+        |osa1: line12      |
+        |osa1: line13      |
+        |                  |
+        |< #chan           |";
+    expect_screen(screen2, &tui.get_front_buffer(), 18, 11, Location::caller());
+}
+
+#[test]
+fn test_resize_no_scroll_stay_on_bottom() {
+    let mut tui = TUI::new_test(18, 10);
+    let serv = "irc.server_1.org";
+    let chan = ChanNameRef::new("#chan");
+    tui.new_server_tab(serv, None);
+    tui.set_nick(serv, "osa1");
+    tui.new_chan_tab(serv, chan);
+    tui.next_tab();
+    tui.next_tab();
+
+    let target = MsgTarget::Chan { serv, chan };
+    let ts = time::at_utc(time::Timespec::new(0, 0));
+
+    for i in 0..15 {
+        tui.add_privmsg("osa1", &format!("line{}", i), ts, &target, false, false);
+    }
+
+    tui.draw();
+
+    #[rustfmt::skip]
+    let screen1 =
+       "|osa1: line7       |
+        |osa1: line8       |
+        |osa1: line9       |
+        |osa1: line10      |
+        |osa1: line11      |
+        |osa1: line12      |
+        |osa1: line13      |
+        |osa1: line14      |
+        |                  |
+        |< #chan           |";
+
+    expect_screen(screen1, &tui.get_front_buffer(), 18, 10, Location::caller());
+
+    tui.set_size(18, 11);
+    tui.draw();
+
+    // shows one extra line on the top of the screen
+    #[rustfmt::skip]
+    let screen2 =
+       "|osa1: line6       |
+        |osa1: line7       |
+        |osa1: line8       |
+        |osa1: line9       |
+        |osa1: line10      |
+        |osa1: line11      |
+        |osa1: line12      |
+        |osa1: line13      |
+        |osa1: line14      |
+        |                  |
+        |< #chan           |";
+    expect_screen(screen2, &tui.get_front_buffer(), 18, 11, Location::caller());
+
+    // resize back to original screen and verify last line is still on the bottom
+    tui.set_size(18, 10);
+    tui.draw();
+    expect_screen(screen1, &tui.get_front_buffer(), 18, 10, Location::caller());
+
+    tui.add_privmsg("osa1", "line15", ts, &target, false, false);
+    tui.set_size(18, 11);
+    tui.draw();
+
+    #[rustfmt::skip]
+    let screen3 =
+       "|osa1: line7       |
+        |osa1: line8       |
+        |osa1: line9       |
+        |osa1: line10      |
+        |osa1: line11      |
+        |osa1: line12      |
+        |osa1: line13      |
+        |osa1: line14      |
+        |osa1: line15      |
+        |                  |
+        |< #chan           |";
+    expect_screen(screen3, &tui.get_front_buffer(), 18, 11, Location::caller());
+}
+
+// Testing for crashes by resizing from 50x50 -> 0x0 -> 50x50
+#[test]
+fn test_resize() {
+    let mut tui = TUI::new_test(80, 50);
+
+    let server = "<server>";
+    tui.new_server_tab(server, None);
+
+    let ts = time::empty_tm();
+    let target = MsgTarget::CurrentTab;
+
+    let f = File::open("test/lipsum.txt").unwrap();
+    let f = BufReader::new(f);
+    for line in f.lines() {
+        let line = line.unwrap();
+        tui.add_msg(&line, ts, &target);
+    }
+
+    for i in (0..=50).rev().chain(0..=50) {
+        tui.set_size(i, i);
+        tui.draw();
+    }
+}
+
+// https://github.com/osa1/tiny/pull/351#issuecomment-915447462
+#[test]
+fn test_resize_scroll_resize() {
+    let mut tui = TUI::new_test(16, 10);
+    let serv = "irc.server_1.org";
+    let chan = ChanNameRef::new("#chan");
+    tui.new_server_tab(serv, None);
+    tui.set_nick(serv, "osa1");
+    tui.new_chan_tab(serv, chan);
+    tui.next_tab();
+    tui.next_tab();
+
+    let target = MsgTarget::Chan { serv, chan };
+    let ts = time::at_utc(time::Timespec::new(0, 0));
+
+    for i in 0..15 {
+        tui.add_privmsg(
+            "osa1",
+            &format!("a long line {}", i),
+            ts,
+            &target,
+            false,
+            false,
+        );
+    }
+
+    tui.draw();
+
+    #[rustfmt::skip]
+    let screen1 =
+       "|osa1: a long    |
+        |line 11         |
+        |osa1: a long    |
+        |line 12         |
+        |osa1: a long    |
+        |line 13         |
+        |osa1: a long    |
+        |line 14         |
+        |                |
+        |< #chan         |";
+
+    expect_screen(screen1, &tui.get_front_buffer(), 16, 10, Location::caller());
+    tui.handle_input_event(
+        term_input::Event::Key(Key::ShiftArrow(Arrow::Up)),
+        &mut None,
+    );
+    tui.set_size(20, 15);
+    tui.draw();
+
+    #[rustfmt::skip]
+    let screen1 =
+       "|osa1: a long line 2 |
+        |osa1: a long line 3 | 
+        |osa1: a long line 4 |
+        |osa1: a long line 5 |
+        |osa1: a long line 6 |
+        |osa1: a long line 7 |
+        |osa1: a long line 8 |
+        |osa1: a long line 9 |
+        |osa1: a long line 10|
+        |osa1: a long line 11|
+        |osa1: a long line 12|
+        |osa1: a long line 13|
+        |osa1: a long line 14|
+        |osa1:               |
+        |< #chan             |";
+
+    expect_screen(screen1, &tui.get_front_buffer(), 20, 15, Location::caller());
+}