From a97c8384749997f85f704327c9e0dd85e4cb91c6 Mon Sep 17 00:00:00 2001
From: Joscha <joscha@plugh.de>
Date: Tue, 19 Jul 2022 23:31:08 +0200
Subject: [PATCH] Fix messages scrolling up on re-render

---
 src/ui/chat/tree/blocks.rs | 24 ++++++++++++++----------
 src/ui/chat/tree/layout.rs | 37 +++++++++++++++++--------------------
 2 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/src/ui/chat/tree/blocks.rs b/src/ui/chat/tree/blocks.rs
index 0175545..a058bec 100644
--- a/src/ui/chat/tree/blocks.rs
+++ b/src/ui/chat/tree/blocks.rs
@@ -1,6 +1,7 @@
 //! Intermediate representation of chat history as blocks of things.
 
 use std::collections::{vec_deque, VecDeque};
+use std::iter;
 
 use chrono::{DateTime, Utc};
 use toss::styled::Styled;
@@ -55,9 +56,9 @@ pub struct Block<I> {
 }
 
 impl<I> Block<I> {
-    pub fn bottom() -> Self {
+    pub fn bottom(line: i32) -> Self {
         Self {
-            line: 0,
+            line,
             time: None,
             indent: 0,
             body: BlockBody::Marker(MarkerBlock::Bottom),
@@ -143,16 +144,19 @@ pub struct Blocks<I> {
 
 impl<I> Blocks<I> {
     pub fn new() -> Self {
-        Self::new_below(0)
-    }
-
-    /// Create a new [`Blocks`] such that prepending a single line will result
-    /// in `top_line = bottom_line = line`.
-    pub fn new_below(line: i32) -> Self {
         Self {
             blocks: VecDeque::new(),
-            top_line: line + 1,
-            bottom_line: line,
+            top_line: 1,
+            bottom_line: 0,
+            roots: None,
+        }
+    }
+
+    pub fn new_bottom(line: i32) -> Self {
+        Self {
+            blocks: iter::once(Block::bottom(line)).collect(),
+            top_line: line,
+            bottom_line: line - 1,
             roots: None,
         }
     }
diff --git a/src/ui/chat/tree/layout.rs b/src/ui/chat/tree/layout.rs
index 5b208d8..473c008 100644
--- a/src/ui/chat/tree/layout.rs
+++ b/src/ui/chat/tree/layout.rs
@@ -50,9 +50,9 @@ impl<M: Msg, S: MsgStore<M>> InnerTreeViewState<M, S> {
         if let Some(block) = last_blocks.find(|b| cursor.matches_block(b)) {
             block.line
         } else if last_cursor_path < cursor_path {
-            // Not using size.height - 1 because markers like
-            // MarkerBlock::Bottom in the line below the last visible line are
-            // still relevant to us.
+            // If the cursor is bottom, the bottom marker needs to be located at
+            // the line below the last visible line. If it is a normal message
+            // cursor, it will be made visible again one way or another later.
             size.height.into()
         } else {
             0
@@ -133,13 +133,17 @@ impl<M: Msg, S: MsgStore<M>> InnerTreeViewState<M, S> {
             });
             blocks
         } else {
-            let mut blocks = Blocks::new_below(cursor_line);
-            blocks.push_front(Block::bottom());
-            blocks
+            Blocks::new_bottom(cursor_line)
         }
     }
 
     fn scroll_so_cursor_is_visible(blocks: &mut Blocks<M::Id>, cursor: &Cursor<M::Id>, size: Size) {
+        if !matches!(cursor, Cursor::Msg(_)) {
+            // In all other cases, there is special scrolling behaviour, so
+            // let's not interfere.
+            return;
+        }
+
         if let Some(block) = blocks.find(|b| cursor.matches_block(b)) {
             let min_line = 0;
             let max_line = size.height as i32 - block.height();
@@ -155,11 +159,9 @@ impl<M: Msg, S: MsgStore<M>> InnerTreeViewState<M, S> {
         }
     }
 
-    /// Try to obtain a normal cursor (i.e. no composing or placeholder cursor)
-    /// pointing to the block.
-    fn as_direct_cursor(block: &Block<M::Id>) -> Option<Cursor<M::Id>> {
+    /// Try to obtain a [`Cursor::Msg`] pointing to the block.
+    fn as_msg_cursor(block: &Block<M::Id>) -> Option<Cursor<M::Id>> {
         match &block.body {
-            BlockBody::Marker(MarkerBlock::Bottom) => Some(Cursor::Bottom),
             BlockBody::Msg(MsgBlock { id, .. }) => Some(Cursor::Msg(id.clone())),
             _ => None,
         }
@@ -170,14 +172,9 @@ impl<M: Msg, S: MsgStore<M>> InnerTreeViewState<M, S> {
         cursor: &mut Cursor<M::Id>,
         size: Size,
     ) {
-        if matches!(cursor, Cursor::Compose(_) | Cursor::Placeholder(_)) {
-            // In this case, we can't easily move the cursor since moving it
-            // would change how the entire layout is rendered in
-            // difficult-to-predict ways.
-            //
-            // Also, the user has initiated a reply to get into this state. This
-            // confirms that they want their cursor in precisely its current
-            // place. Moving it might lead to mis-replies and frustration.
+        if !matches!(cursor, Cursor::Msg(_)) {
+            // In all other cases, there is special scrolling behaviour, so
+            // let's not interfere.
             return;
         }
 
@@ -190,14 +187,14 @@ impl<M: Msg, S: MsgStore<M>> InnerTreeViewState<M, S> {
                 blocks
                     .iter()
                     .filter(|b| b.line >= min_line)
-                    .find_map(Self::as_direct_cursor)
+                    .find_map(Self::as_msg_cursor)
             } else if block.line > max_line {
                 // Move cursor to last possible visible block
                 blocks
                     .iter()
                     .rev()
                     .filter(|b| b.line <= max_line)
-                    .find_map(Self::as_direct_cursor)
+                    .find_map(Self::as_msg_cursor)
             } else {
                 None
             };