Skip to content

Overlay side-panel in autohide mode. #1100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 3, 2021
Merged

Overlay side-panel in autohide mode. #1100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0b92d01
refactor: ui: Use `show_left_panel` to set startup layout.
Rohitth007 Jul 29, 2021
0ee29bb
refactor: ui: Store Frame as a View attribute.
Rohitth007 Jul 29, 2021
7f26020
ui: Close side-panels when focus is shifted to center_panel.
Rohitth007 Jul 31, 2021
03b92ea
refactor: test_ui: Combine tests for the show_panel methods.
Rohitth007 Aug 1, 2021
45ff868
ui: Make side panels overlay the center panel in autohide layout.
Rohitth007 Jul 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 49 additions & 48 deletions tests/ui/test_ui.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,14 @@ def mock_external_classes(self, mocker: MockerFixture) -> None:
self.model = mocker.patch(CONTROLLER + ".model")
self.write_box = mocker.patch(MODULE + ".WriteBox")
self.search_box = mocker.patch(MODULE + ".SearchBox")

@pytest.fixture
def view(self, mocker: MockerFixture) -> View:
mocker.patch(MODULE + ".TabView")
mocker.patch(MODULE + ".LeftColumnView")
mocker.patch("zulipterminal.ui_tools.views.urwid.Frame")
mocker.patch("zulipterminal.ui_tools.views.MessageView")
mocker.patch(MODULE + ".RightColumnView")

@pytest.fixture
def view(self, mocker: MockerFixture) -> View:
# View is an urwid.Frame instance, a Box widget.
mocker.patch(VIEW + ".sizing", return_value=frozenset({"box"}))

Expand Down Expand Up @@ -82,15 +81,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(
Expand All @@ -105,7 +104,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)
Expand Down Expand Up @@ -179,6 +178,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]"
Expand All @@ -202,7 +202,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),
],
Expand All @@ -225,58 +225,49 @@ 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)])
def test_show_left_panel(
@pytest.mark.parametrize("visible", [True, False])
@pytest.mark.parametrize("test_method", ["left_panel", "right_panel"])
def test_show_panel_methods(
self,
mocker: MockerFixture,
view: View,
visible: bool,
width: int,
autohide: bool,
test_method: str,
) -> None:
view.body = mocker.Mock()
view.body.contents = [view.left_panel, mocker.Mock(), mocker.Mock()]
view.controller.autohide = autohide
self.controller.autohide = autohide
view = View(self.controller)
view.frame.body = view.body

view.show_left_panel(visible=visible)
tail = [None, 0, 0, "top", None, "relative", 100, None, 0, 0]
if test_method == "left_panel":
expected_overlay_options = ["left", None, "given", LEFT_WIDTH + 1] + tail
expected_tab = view.left_tab
expected_panel = view.left_panel

if autohide:
if visible:
assert view.body.contents[0][0] == view.left_panel
else:
assert view.body.contents[0][0] == view.left_tab
view.body.options.assert_called_once_with("given", width)
view.show_left_panel(visible=visible)
else:
view.body.options.assert_not_called()

@pytest.mark.parametrize("autohide", [True, False])
@pytest.mark.parametrize(
"visible, width", [(True, RIGHT_WIDTH), (False, TAB_WIDTH)]
)
def test_show_right_panel(
self,
mocker: MockerFixture,
view: View,
visible: bool,
width: int,
autohide: bool,
) -> None:
view.body = mocker.Mock()
view.body.contents = [mocker.Mock(), mocker.Mock(), view.right_panel]
view.controller.autohide = autohide
expected_overlay_options = ["right", None, "given", RIGHT_WIDTH + 1] + tail
expected_tab = view.right_tab
expected_panel = view.right_panel

view.show_right_panel(visible=visible)
view.show_right_panel(visible=visible)

if autohide:
if visible:
assert view.body.contents[2][0] == view.right_panel
assert (expected_panel, mocker.ANY) in view.frame.body.top_w.contents
assert view.frame.body.bottom_w == view.body
assert view.frame.body.contents[1][1] == tuple(expected_overlay_options)
else:
assert view.body.contents[2][0] == view.right_tab
view.body.options.assert_called_once_with("given", width)
assert (expected_tab, mocker.ANY) in view.frame.body.contents
assert view.body.focus_position == 1
else:
view.body.options.assert_not_called()
# No change
assert view.frame.body.contents[0][0] == view.left_panel
assert view.frame.body.contents[2][0] == view.right_panel

def test_keypress_normal_mode_navigation(
self,
Expand Down Expand Up @@ -308,27 +299,31 @@ def test_keypress_ALL_MENTIONS(
key: str,
widget_size: Callable[[Widget], urwid_Box],
) -> None:
view.body = mocker.Mock()
view.body.focus_col = None
view.mentioned_button = mocker.Mock()
view.mentioned_button.activate = mocker.Mock()
view.controller.is_in_editor_mode = lambda: False
size = widget_size(view)
view.model.controller.narrow_to_all_mentions = mocker.Mock()

view.keypress(size, key)

view.model.controller.narrow_to_all_mentions.assert_called_once_with()
assert view.body.focus_col == 1
view.mentioned_button.activate.assert_called_once_with(key)

@pytest.mark.parametrize("key", keys_for_command("STREAM_MESSAGE"))
@pytest.mark.parametrize("autohide", [True, False], ids=["autohide", "no_autohide"])
def test_keypress_STREAM_MESSAGE(
self,
view: View,
mocker: MockerFixture,
key: str,
autohide: bool,
widget_size: Callable[[Widget], urwid_Box],
) -> None:
mocked_middle_column = mocker.patch.object(view, "middle_column", create=True)
view.body = mocker.Mock()
view.controller.autohide = autohide
view.body.contents = ["streams", "messages", "users"]
view.left_panel = mocker.Mock()
view.right_panel = mocker.Mock()
view.controller.is_in_editor_mode = lambda: False
size = widget_size(view)

Expand Down Expand Up @@ -411,16 +406,22 @@ def test_keypress_autohide_streams(
],
)
@pytest.mark.parametrize("key", keys_for_command("OPEN_DRAFT"))
@pytest.mark.parametrize("autohide", [True, False], ids=["autohide", "no_autohide"])
def test_keypress_OPEN_DRAFT(
self,
view: View,
mocker: MockerFixture,
draft: Composition,
key: str,
autohide: bool,
widget_size: Callable[[Widget], urwid_Box],
) -> None:
view.body = mocker.Mock()
view.body.contents = ["streams", "messages", "users"]
view.left_panel = mocker.Mock()
view.middle_column = mocker.Mock()
view.right_panel = mocker.Mock()
view.controller.autohide = autohide
view.controller.report_error = mocker.Mock()
view.controller.is_in_editor_mode = lambda: False
view.model.stream_id_from_name.return_value = 10
Expand Down
73 changes: 52 additions & 21 deletions zulipterminal/ui.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,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
Expand Down Expand Up @@ -142,7 +142,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),
]
Expand Down Expand Up @@ -177,26 +177,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 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
Comment on lines +227 to +229
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain this further?

Copy link
Collaborator Author

@Rohitth007 Rohitth007 Jul 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neiljp The best way to understand this is to comment out the line and run it.

Basically keypress pretty much handles navigation focus change for us but except when initialization we make the left_tab as the focus_column (which is actually not a selectable widget but can be forcefully focused like we did)

To work around that we have to initialize focus_column as center_panel but that would mean a message would be read at startup. Hence, the FIXME.

The reordering below is a result of this change. Though it actually works fine without it, the tests broke hence the reordering.

The reordering actually also connects the 2nd commit in #1072 (redirecting keypress)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mentioned this in the stream, but while I can confirm this does resolve one problem with setting focus, it doesn't solve opening compose or message-search from a panel, and I suggest we may want to use get/set_focus_path for this in general.


def keypress(self, size: urwid_Box, key: str) -> Optional[str]:
self.model.new_user_input = True
Expand All @@ -210,37 +240,38 @@ def keypress(self, size: urwid_Box, key: str) -> Optional[str]:
or is_command_key("STREAM_MESSAGE", key)
or is_command_key("PRIVATE_MESSAGE", key)
):
self.show_left_panel(visible=False)
self.show_right_panel(visible=False)
self.body.focus_col = 1
self.middle_column.keypress(size, key)
return key
elif is_command_key("ALL_PM", key):
self.model.controller.narrow_to_all_pm()
self.body.focus_col = 1
self.pm_button.activate(key)
elif is_command_key("ALL_STARRED", key):
self.model.controller.narrow_to_all_starred()
self.body.focus_col = 1
self.starred_button.activate(key)
elif is_command_key("ALL_MENTIONS", key):
self.model.controller.narrow_to_all_mentions()
self.body.focus_col = 1
self.mentioned_button.activate(key)
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)
Comment on lines -227 to +259
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this move is related to the focus setting in the panel methods - which we've just removed?
(this now overrides that being set there)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's because of the override but this was not the one we removed in one of the previous refactor commits. That was focus_setting when visible = True this is when visible = False

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()
if saved_draft:
self.show_left_panel(visible=False)
self.show_right_panel(visible=False)
if saved_draft["type"] == "stream":
stream_id = self.model.stream_id_from_name(saved_draft["to"])
self.write_box.stream_box_view(
Expand Down