ui: Make side panels to overlay the center panel in autohide layout.

This commit makes the side panels to overlay the body which avoids
squashing and stretching of the message view which improves UX in
autohide mode.

To do this this self.frame is stored in `main_window` and the
`show_panel` functions have been updated to use Overlay.

The focus setting in the `show_panel` functions which could be
avoided at a later point needed a reordering in the SEARCH keypress
which actually makes it the correct order, i.e.,
* first handle autohide
* then handle search

Tests updated.

Author: Rohitth007

tests/ui/test_ui.py

@@ -1,4 +1,4 @@
-from typing import Any, Callable, List, Optional, Tuple
+from typing import Any, Callable, List, Optional, Tuple, Union
 
 import pytest
 from pytest_mock import MockerFixture
@@ -82,15 +82,15 @@ def test_set_footer_text_default(self, view: View, mocker: MockerFixture) -> Non
 
         view.set_footer_text()
 
-        view._w.footer.set_text.assert_called_once_with(["some help text"])
+        view.frame.footer.set_text.assert_called_once_with(["some help text"])
         view.controller.update_screen.assert_called_once_with()
 
     def test_set_footer_text_specific_text(
         self, view: View, text: str = "blah"
     ) -> None:
         view.set_footer_text([text])
 
-        view._w.footer.set_text.assert_called_once_with([text])
+        view.frame.footer.set_text.assert_called_once_with([text])
         view.controller.update_screen.assert_called_once_with()
 
     def test_set_footer_text_with_duration(
@@ -105,7 +105,7 @@ def test_set_footer_text_with_duration(
 
         view.set_footer_text([custom_text], duration=duration)
 
-        view._w.footer.set_text.assert_has_calls(
+        view.frame.footer.set_text.assert_has_calls(
             [mocker.call([custom_text]), mocker.call(["some help text"])]
         )
         mock_sleep.assert_called_once_with(duration)
@@ -179,6 +179,7 @@ def just_set_message_view(self: Any) -> None:
         title_divider = mocker.patch(MODULE + ".urwid.Divider")
         text = mocker.patch(MODULE + ".urwid.Text")
         footer_view = mocker.patch(VIEW + ".footer_view")
+        show_left_panel = mocker.patch(VIEW + ".show_left_panel")
 
         full_name = "Bob James"
         email = "[email protected]"
@@ -202,7 +203,7 @@ def just_set_message_view(self: Any) -> None:
         expected_column_calls = [
             mocker.call(
                 [
-                    (LEFT_WIDTH, view.left_panel),
+                    (TAB_WIDTH, view.left_tab),
                     ("weight", 10, mocker.ANY),  # ANY is a center
                     (TAB_WIDTH, view.right_tab),
                 ],
@@ -225,58 +226,107 @@ def just_set_message_view(self: Any) -> None:
         frame.assert_called_once_with(
             view.body, col(), focus_part="body", footer=footer_view()
         )
+        assert view.frame == frame()
+        show_left_panel.assert_called_once_with(visible=True)
 
     @pytest.mark.parametrize("autohide", [True, False])
-    @pytest.mark.parametrize("visible, width", [(True, LEFT_WIDTH), (False, TAB_WIDTH)])
+    @pytest.mark.parametrize("visible", [True, False])
+    @pytest.mark.parametrize(
+        "expected_overlay_options",
+        [
+            (
+                "left",
+                None,
+                "given",
+                LEFT_WIDTH + 1,
+                None,
+                0,
+                0,
+                "top",
+                None,
+                "relative",
+                100,
+                None,
+                0,
+                0,
+            )
+        ],
+    )
     def test_show_left_panel(
         self,
         mocker: MockerFixture,
         view: View,
         visible: bool,
-        width: int,
         autohide: bool,
+        expected_overlay_options: Tuple[Union[str, int, None]],
     ) -> None:
-        view.body = mocker.Mock()
-        view.body.contents = [view.left_panel, mocker.Mock(), mocker.Mock()]
+        view.frame.body = view.body
         view.controller.autohide = autohide
 
         view.show_left_panel(visible=visible)
 
         if autohide:
             if visible:
-                assert view.body.contents[0][0] == view.left_panel
+                assert view.frame.body.top_w.contents[0][0] == view.left_panel
+                assert view.frame.body.bottom_w == view.body
+                assert view.frame.body.contents[1][1] == expected_overlay_options
             else:
-                assert view.body.contents[0][0] == view.left_tab
-            view.body.options.assert_called_once_with("given", width)
+                assert view.frame.body.contents[0][0] == view.left_tab
+                assert view.body.focus_position == 1
         else:
-            view.body.options.assert_not_called()
+            # No change
+            assert view.frame.body.contents[0][0] == view.left_tab
 
     @pytest.mark.parametrize("autohide", [True, False])
     @pytest.mark.parametrize(
         "visible, width", [(True, RIGHT_WIDTH), (False, TAB_WIDTH)]
     )
+    @pytest.mark.parametrize(
+        "expected_overlay_options",
+        [
+            (
+                "right",
+                None,
+                "given",
+                RIGHT_WIDTH + 1,
+                None,
+                0,
+                0,
+                "top",
+                None,
+                "relative",
+                100,
+                None,
+                0,
+                0,
+            )
+        ],
+    )
     def test_show_right_panel(
         self,
         mocker: MockerFixture,
         view: View,
         visible: bool,
         width: int,
         autohide: bool,
+        expected_overlay_options: Tuple[Union[str, int, None]],
     ) -> None:
-        view.body = mocker.Mock()
-        view.body.contents = [mocker.Mock(), mocker.Mock(), view.right_panel]
+        view.frame.body = view.body
         view.controller.autohide = autohide
 
         view.show_right_panel(visible=visible)
 
         if autohide:
             if visible:
-                assert view.body.contents[2][0] == view.right_panel
+                assert view.frame.body.top_w.contents[1][0] == view.right_panel
+                assert view.frame.body.bottom_w == view.body
+                assert view.frame.body.contents[1][1] == expected_overlay_options
             else:
-                assert view.body.contents[2][0] == view.right_tab
-            view.body.options.assert_called_once_with("given", width)
+                assert view.frame.body.contents[0][0] == view.right_tab
+                assert view.body.focus_position == 1
         else:
-            view.body.options.assert_not_called()
+            # No change
+            assert view.frame.body.contents[0][0] == view.right_tab
 
     def test_keypress_normal_mode_navigation(
         self,
@@ -356,7 +406,7 @@ def test_keypress_autohide_users(
         view.right_panel = mocker.Mock()
         size = widget_size(view)
         view.controller.is_in_editor_mode = lambda: False
-        view.body.focus_position = None
+        view.body.focus_position = 1
 
         view.keypress(size, key)
 

zulipterminal/ui.py

@@ -111,8 +111,8 @@ def set_footer_text(
             text = self.get_random_help()
         else:
             text = text_list
-        self._w.footer.set_text(text)
-        self._w.footer.set_attr_map({None: style})
+        self.frame.footer.set_text(text)
+        self.frame.footer.set_attr_map({None: style})
         self.controller.update_screen()
         if duration is not None:
             assert duration > 0
@@ -146,7 +146,7 @@ def main_window(self) -> Any:
         self.right_panel, self.right_tab = self.right_column_view()
         if self.controller.autohide:
             body = [
-                (LEFT_WIDTH, self.left_panel),
+                (TAB_WIDTH, self.left_tab),
                 ("weight", 10, self.center_panel),
                 (TAB_WIDTH, self.right_tab),
             ]
@@ -181,26 +181,56 @@ def main_window(self) -> Any:
             ]
         )
 
-        w = urwid.Frame(
+        self.frame = urwid.Frame(
             self.body, title_bar, focus_part="body", footer=self.footer_view()
         )
-        return w
+
+        # Show overlayed left panel on startup in autohide mode
+        self.show_left_panel(visible=True)
+
+        return self.frame
 
     def show_left_panel(self, *, visible: bool) -> None:
         if not self.controller.autohide:
             return
 
-        width = LEFT_WIDTH if visible else TAB_WIDTH
-        widget = self.left_panel if visible else self.left_tab
-        self.body.contents[0] = (widget, self.body.options("given", width))
+        if visible:
+            self.frame.body = urwid.Overlay(
+                urwid.Columns(
+                    [(LEFT_WIDTH, self.left_panel), (1, urwid.SolidFill("▏"))]
+                ),
+                self.body,
+                align="left",
+                width=LEFT_WIDTH + 1,
+                valign="top",
+                height=("relative", 100),
+            )
+        else:
+            self.frame.body = self.body
+            # FIXME: This can be avoided after fixing the "sacrificing 1st
+            # unread msg" issue and setting focus_column=1 when initializing.
+            self.body.focus_position = 1
 
     def show_right_panel(self, *, visible: bool) -> None:
         if not self.controller.autohide:
             return
 
-        width = RIGHT_WIDTH if visible else TAB_WIDTH
-        widget = self.right_panel if visible else self.right_tab
-        self.body.contents[2] = (widget, self.body.options("given", width))
+        if visible:
+            self.frame.body = urwid.Overlay(
+                urwid.Columns(
+                    [(1, urwid.SolidFill("▕")), (RIGHT_WIDTH, self.right_panel)]
+                ),
+                self.body,
+                align="right",
+                width=RIGHT_WIDTH + 1,
+                valign="top",
+                height=("relative", 100),
+            )
+        else:
+            self.frame.body = self.body
+            # FIXME: This can be avoided after fixing the "sacrificing 1st
+            # unread msg" issue and setting focus_column=1 when initializing.
+            self.body.focus_position = 1
 
     def keypress(self, size: urwid_Box, key: str) -> Optional[str]:
         self.model.new_user_input = True
@@ -228,19 +258,19 @@ def keypress(self, size: urwid_Box, key: str) -> Optional[str]:
             self.body.focus_col = 1
         elif is_command_key("SEARCH_PEOPLE", key):
             # Start User Search if not in editor_mode
-            self.body.focus_position = 2
-            self.users_view.keypress(size, key)
             self.show_left_panel(visible=False)
             self.show_right_panel(visible=True)
+            self.body.focus_position = 2
+            self.users_view.keypress(size, key)
             return key
         elif is_command_key("SEARCH_STREAMS", key) or is_command_key(
             "SEARCH_TOPICS", key
         ):
             # jump stream search
-            self.body.focus_position = 0
-            self.left_panel.keypress(size, key)
             self.show_right_panel(visible=False)
             self.show_left_panel(visible=True)
+            self.body.focus_position = 0
+            self.left_panel.keypress(size, key)
             return key
         elif is_command_key("OPEN_DRAFT", key):
             saved_draft = self.model.session_draft_message()