From f07058f926c0e9b5bdaafb40422c8f7fd526efcf Mon Sep 17 00:00:00 2001 From: "neiljp (Neil Pilgrim)" Date: Thu, 27 Jul 2023 09:03:01 -0700 Subject: [PATCH 1/6] refactor: views: Require optional parameters to PopUpView to be named. Tests updated. --- tests/ui_tools/test_popups.py | 4 ++-- zulipterminal/ui_tools/views.py | 13 ++++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/tests/ui_tools/test_popups.py b/tests/ui_tools/test_popups.py index d58b6c3353..a2727fbdad 100644 --- a/tests/ui_tools/test_popups.py +++ b/tests/ui_tools/test_popups.py @@ -119,8 +119,8 @@ def pop_up_view_autouse(self, mocker: MockerFixture) -> None: self.command, self.width, self.title, - self.header, - self.footer, + header=self.header, + footer=self.footer, ) def test_init(self, mocker: MockerFixture) -> None: diff --git a/zulipterminal/ui_tools/views.py b/zulipterminal/ui_tools/views.py index c2034b3ef7..d3930c6299 100644 --- a/zulipterminal/ui_tools/views.py +++ b/zulipterminal/ui_tools/views.py @@ -957,6 +957,7 @@ def __init__( command: str, requested_width: int, title: str, + *, header: Optional[Any] = None, footer: Optional[Any] = None, ) -> None: @@ -1294,7 +1295,9 @@ def __init__(self, controller: Any, title: str) -> None: [("", rendered_menu_content)], column_widths ) - super().__init__(controller, body, "MARKDOWN_HELP", popup_width, title, header) + super().__init__( + controller, body, "MARKDOWN_HELP", popup_width, title, header=header + ) PopUpConfirmationViewLocation = Literal["top-left", "center"] @@ -1932,8 +1935,8 @@ def __init__( "MSG_INFO", max_cols, title, - urwid.Pile(msg_box.header), - urwid.Pile(msg_box.footer), + header=urwid.Pile(msg_box.header), + footer=urwid.Pile(msg_box.footer), ) def keypress(self, size: urwid_Size, key: str) -> str: @@ -1984,8 +1987,8 @@ def __init__( "MSG_INFO", max_cols, title, - urwid.Pile(msg_box.header), - urwid.Pile(msg_box.footer), + header=urwid.Pile(msg_box.header), + footer=urwid.Pile(msg_box.footer), ) def keypress(self, size: urwid_Size, key: str) -> str: From ddf2efbfbb8f66ce31f35d0e47695d887b2558ca Mon Sep 17 00:00:00 2001 From: "neiljp (Neil Pilgrim)" Date: Tue, 25 Jul 2023 22:17:53 -0700 Subject: [PATCH 2/6] refactor: core/views: Extract popup border style into PopUpFrame. --- zulipterminal/core.py | 12 ++---------- zulipterminal/ui_tools/views.py | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/zulipterminal/core.py b/zulipterminal/core.py index eec62826f5..d306018977 100644 --- a/zulipterminal/core.py +++ b/zulipterminal/core.py @@ -19,7 +19,6 @@ from typing_extensions import Literal from zulipterminal.api_types import Composition, Message -from zulipterminal.config.symbols import POPUP_CONTENT_BORDER, POPUP_TOP_LINE from zulipterminal.config.themes import ThemeSpec from zulipterminal.config.ui_sizes import ( MAX_LINEAR_SCALING_WIDTH, @@ -42,6 +41,7 @@ MsgInfoView, NoticeView, PopUpConfirmationView, + PopUpFrame, StreamInfoView, StreamMembersView, UserInfoView, @@ -222,16 +222,8 @@ def clamp(n: int, minn: int, maxn: int) -> int: return max_popup_cols, max_popup_rows def show_pop_up(self, to_show: Any, style: str) -> None: - text = urwid.Text(to_show.title, align="center") - title_map = urwid.AttrMap(urwid.Filler(text), style) - title_box_adapter = urwid.BoxAdapter(title_map, height=1) - title_top = urwid.AttrMap(urwid.Divider(POPUP_TOP_LINE), "popup_border") - title = urwid.Pile([title_top, title_box_adapter]) - - content = urwid.LineBox(to_show, **POPUP_CONTENT_BORDER) - self.loop.widget = urwid.Overlay( - urwid.AttrMap(urwid.Frame(header=title, body=content), "popup_border"), + PopUpFrame(to_show, to_show.title, style), self.view, align="center", valign="middle", diff --git a/zulipterminal/ui_tools/views.py b/zulipterminal/ui_tools/views.py index d3930c6299..8eda8dc3db 100644 --- a/zulipterminal/ui_tools/views.py +++ b/zulipterminal/ui_tools/views.py @@ -24,6 +24,8 @@ CHECK_MARK, COLUMN_TITLE_BAR_LINE, PINNED_STREAMS_DIVIDER, + POPUP_CONTENT_BORDER, + POPUP_TOP_LINE, SECTION_DIVIDER_LINE, ) from zulipterminal.config.ui_mappings import ( @@ -949,6 +951,21 @@ def __init__(self, text: str) -> None: PopUpViewTableContent = Sequence[Tuple[str, Sequence[Union[str, Tuple[str, Any]]]]] +class PopUpFrame(urwid.AttrMap): + def __init__(self, body: Any, title: str, style: str) -> None: + text = urwid.Text(title, align="center") + title_map = urwid.AttrMap(urwid.Filler(text), style) + title_box_adapter = urwid.BoxAdapter(title_map, height=1) + title_top = urwid.AttrMap(urwid.Divider(POPUP_TOP_LINE), "popup_border") + frame_title = urwid.Pile([title_top, title_box_adapter]) + + content = urwid.LineBox(body, **POPUP_CONTENT_BORDER) + + titled_content = urwid.Frame(body=content, header=frame_title) + + super().__init__(titled_content, "popup_border") + + class PopUpView(urwid.Frame): def __init__( self, From cc3fbf4f52151b2ea77910067a606f011c05d3e4 Mon Sep 17 00:00:00 2001 From: "neiljp (Neil Pilgrim)" Date: Thu, 27 Jul 2023 08:50:40 -0700 Subject: [PATCH 3/6] core/views: Support title-less PopUpFrame with reduced height. Ideally the extra height would be calculated automatically in PopUpFrame. --- zulipterminal/core.py | 10 ++++++++-- zulipterminal/ui_tools/views.py | 18 ++++++++++-------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/zulipterminal/core.py b/zulipterminal/core.py index d306018977..28355eb3b9 100644 --- a/zulipterminal/core.py +++ b/zulipterminal/core.py @@ -222,15 +222,21 @@ def clamp(n: int, minn: int, maxn: int) -> int: return max_popup_cols, max_popup_rows def show_pop_up(self, to_show: Any, style: str) -> None: + if to_show.title is not None: + # +2 to height, due to title enhancement + # TODO: Ideally this would be in PopUpFrame + extra_height = 2 + else: + extra_height = 0 + self.loop.widget = urwid.Overlay( PopUpFrame(to_show, to_show.title, style), self.view, align="center", valign="middle", # +2 to both of the following, due to LineBox - # +2 to height, due to title enhancement width=to_show.width + 2, - height=to_show.height + 4, + height=to_show.height + 2 + extra_height, ) def is_any_popup_open(self) -> bool: diff --git a/zulipterminal/ui_tools/views.py b/zulipterminal/ui_tools/views.py index 8eda8dc3db..435becf6ef 100644 --- a/zulipterminal/ui_tools/views.py +++ b/zulipterminal/ui_tools/views.py @@ -952,16 +952,18 @@ def __init__(self, text: str) -> None: class PopUpFrame(urwid.AttrMap): - def __init__(self, body: Any, title: str, style: str) -> None: - text = urwid.Text(title, align="center") - title_map = urwid.AttrMap(urwid.Filler(text), style) - title_box_adapter = urwid.BoxAdapter(title_map, height=1) - title_top = urwid.AttrMap(urwid.Divider(POPUP_TOP_LINE), "popup_border") - frame_title = urwid.Pile([title_top, title_box_adapter]) - + def __init__(self, body: Any, title: Optional[str], style: str) -> None: content = urwid.LineBox(body, **POPUP_CONTENT_BORDER) - titled_content = urwid.Frame(body=content, header=frame_title) + if title is not None: + text = urwid.Text(title, align="center") + title_map = urwid.AttrMap(urwid.Filler(text), style) + title_box_adapter = urwid.BoxAdapter(title_map, height=1) + title_top = urwid.AttrMap(urwid.Divider(POPUP_TOP_LINE), "popup_border") + frame_title = urwid.Pile([title_top, title_box_adapter]) + titled_content = urwid.Frame(body=content, header=frame_title) + else: + titled_content = urwid.Frame(body=content) super().__init__(titled_content, "popup_border") From b822126b0482b74c609dd217aa590759724f0765 Mon Sep 17 00:00:00 2001 From: "neiljp (Neil Pilgrim)" Date: Fri, 28 Jul 2023 23:49:07 -0700 Subject: [PATCH 4/6] core/views: Remove PopUpConfirmationView location option. These popups are all now implicitly centered. Tests updated and tidied. --- tests/ui_tools/test_popups.py | 16 ++++++++++++++-- zulipterminal/core.py | 16 ++++------------ zulipterminal/ui_tools/views.py | 26 ++++++-------------------- 3 files changed, 24 insertions(+), 34 deletions(-) diff --git a/tests/ui_tools/test_popups.py b/tests/ui_tools/test_popups.py index a2727fbdad..c6190faffe 100644 --- a/tests/ui_tools/test_popups.py +++ b/tests/ui_tools/test_popups.py @@ -45,11 +45,20 @@ class TestPopUpConfirmationView: @pytest.fixture def popup_view(self, mocker: MockerFixture) -> PopUpConfirmationView: self.controller = mocker.Mock() + self.controller.maximum_popup_dimensions.return_value = (70, 25) + + self.text = Text("Some question?") + self.callback = mocker.Mock() + self.list_walker = mocker.patch(LISTWALKER, return_value=[]) + self.divider = mocker.patch(MODULE + ".urwid.Divider") - self.text = mocker.patch(MODULE + ".urwid.Text") + self.divider.return_value.rows.return_value = 1 + self.wrapper_w = mocker.patch(MODULE + ".urwid.WidgetWrap") + self.wrapper_w.return_value.rows.return_value = 1 + return PopUpConfirmationView( self.controller, self.text, @@ -68,6 +77,7 @@ def test_exit_popup_yes( self, mocker: MockerFixture, popup_view: PopUpConfirmationView ) -> None: popup_view.exit_popup_yes(mocker.Mock()) + self.callback.assert_called_once_with() assert self.controller.exit_popup.called @@ -75,11 +85,12 @@ def test_exit_popup_no( self, mocker: MockerFixture, popup_view: PopUpConfirmationView ) -> None: popup_view.exit_popup_no(mocker.Mock()) + self.callback.assert_not_called() assert self.controller.exit_popup.called @pytest.mark.parametrize("key", keys_for_command("EXIT_POPUP")) - def test_exit_popup_EXIT_POPUP( + def test_keypress__EXIT_POPUP( self, popup_view: PopUpConfirmationView, key: str, @@ -87,6 +98,7 @@ def test_exit_popup_EXIT_POPUP( ) -> None: size = widget_size(popup_view) popup_view.keypress(size, key) + self.callback.assert_not_called() assert self.controller.exit_popup.called diff --git a/zulipterminal/core.py b/zulipterminal/core.py index 28355eb3b9..ce4079268c 100644 --- a/zulipterminal/core.py +++ b/zulipterminal/core.py @@ -492,9 +492,7 @@ def show_media_confirmation_popup( "?", ] ) - self.loop.widget = PopUpConfirmationView( - self, question, callback, location="center" - ) + self.loop.widget = PopUpConfirmationView(self, question, callback) def search_messages(self, text: str) -> None: # Search for a text in messages @@ -521,9 +519,7 @@ def save_draft_confirmation_popup(self, draft: Composition) -> None: "center", ) save_draft = partial(self.model.save_draft, draft) - self.loop.widget = PopUpConfirmationView( - self, question, save_draft, location="center" - ) + self.loop.widget = PopUpConfirmationView(self, question, save_draft) def stream_muting_confirmation_popup( self, stream_id: int, stream_name: str @@ -547,9 +543,7 @@ def exit_compose_confirmation_popup(self) -> None: "center", ) write_box = self.view.write_box - popup_view = PopUpConfirmationView( - self, question, write_box.exit_compose_box, location="center" - ) + popup_view = PopUpConfirmationView(self, question, write_box.exit_compose_box) self.loop.widget = popup_view def copy_to_clipboard(self, text: str, text_category: str) -> None: @@ -665,9 +659,7 @@ def prompting_exit_handler(self, signum: int, frame: Any) -> None: ("bold", " Please confirm that you wish to exit Zulip-Terminal "), "center", ) - popup_view = PopUpConfirmationView( - self, question, self.deregister_client, location="center" - ) + popup_view = PopUpConfirmationView(self, question, self.deregister_client) self.loop.widget = popup_view self.loop.run() diff --git a/zulipterminal/ui_tools/views.py b/zulipterminal/ui_tools/views.py index 435becf6ef..8803150dfc 100644 --- a/zulipterminal/ui_tools/views.py +++ b/zulipterminal/ui_tools/views.py @@ -36,7 +36,6 @@ STREAM_ACCESS_TYPE, STREAM_POST_POLICY, ) -from zulipterminal.config.ui_sizes import LEFT_WIDTH from zulipterminal.helper import ( TidiedUserInfo, asynch, @@ -1319,16 +1318,12 @@ def __init__(self, controller: Any, title: str) -> None: ) -PopUpConfirmationViewLocation = Literal["top-left", "center"] - - class PopUpConfirmationView(urwid.Overlay): def __init__( self, controller: Any, question: Any, success_callback: Callable[[], None], - location: PopUpConfirmationViewLocation = "top-left", ) -> None: self.controller = controller self.success_callback = success_callback @@ -1341,26 +1336,17 @@ def __init__( widgets = [question, urwid.Divider(), wrapped_widget] prompt = urwid.LineBox(urwid.ListBox(urwid.SimpleFocusListWalker(widgets))) - if location == "top-left": - align = "left" - valign = "top" - width = LEFT_WIDTH + 1 - height = 8 - else: - align = "center" - valign = "middle" - - max_cols, max_rows = controller.maximum_popup_dimensions() - # +2 to compensate for the LineBox characters. - width = min(max_cols, max(question.pack()[0], len("Yes"), len("No"))) + 2 - height = min(max_rows, sum(widget.rows((width,)) for widget in widgets)) + 2 + max_cols, max_rows = controller.maximum_popup_dimensions() + # +2 to compensate for the LineBox characters. + width = min(max_cols, max(question.pack()[0], len("Yes"), len("No"))) + 2 + height = min(max_rows, sum(widget.rows((width,)) for widget in widgets)) + 2 urwid.Overlay.__init__( self, prompt, self.controller.view, - align=align, - valign=valign, + align="center", + valign="middle", width=width, height=height, ) From 90842d541329ab113de44176739488c242d5d3dd Mon Sep 17 00:00:00 2001 From: "neiljp (Neil Pilgrim)" Date: Thu, 27 Jul 2023 08:50:48 -0700 Subject: [PATCH 5/6] refactor: core/views: Use show_pop_up for PopUpConfirmationView. Small comment added re importance of retaining self.loop.run() for signal handler. Tests updated. --- tests/core/test_core.py | 5 ++++- zulipterminal/core.py | 18 ++++++++++++------ zulipterminal/ui_tools/views.py | 23 +++++++++-------------- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/tests/core/test_core.py b/tests/core/test_core.py index be05d490cc..c9532d1ea2 100644 --- a/tests/core/test_core.py +++ b/tests/core/test_core.py @@ -477,6 +477,9 @@ def test_stream_muting_confirmation_popup( stream_name: str = "PTEST", ) -> None: pop_up = mocker.patch(MODULE + ".PopUpConfirmationView") + pop_up.return_value.width = 30 + pop_up.return_value.height = 6 + text = mocker.patch(MODULE + ".urwid.Text") partial = mocker.patch(MODULE + ".partial") controller.model.muted_streams = muted_streams @@ -484,7 +487,7 @@ def test_stream_muting_confirmation_popup( controller.stream_muting_confirmation_popup(stream_id, stream_name) - text.assert_called_with( + text.assert_any_call( ("bold", f"Confirm {action} of stream '{stream_name}' ?"), "center", ) diff --git a/zulipterminal/core.py b/zulipterminal/core.py index ce4079268c..50aa5a3387 100644 --- a/zulipterminal/core.py +++ b/zulipterminal/core.py @@ -492,7 +492,8 @@ def show_media_confirmation_popup( "?", ] ) - self.loop.widget = PopUpConfirmationView(self, question, callback) + popup = PopUpConfirmationView(self, question, callback) + self.show_pop_up(popup, "area:msg") def search_messages(self, text: str) -> None: # Search for a text in messages @@ -519,7 +520,9 @@ def save_draft_confirmation_popup(self, draft: Composition) -> None: "center", ) save_draft = partial(self.model.save_draft, draft) - self.loop.widget = PopUpConfirmationView(self, question, save_draft) + + popup = PopUpConfirmationView(self, question, save_draft) + self.show_pop_up(popup, "area:msg") def stream_muting_confirmation_popup( self, stream_id: int, stream_name: str @@ -531,7 +534,8 @@ def stream_muting_confirmation_popup( "center", ) mute_this_stream = partial(self.model.toggle_stream_muted_status, stream_id) - self.loop.widget = PopUpConfirmationView(self, question, mute_this_stream) + popup = PopUpConfirmationView(self, question, mute_this_stream) + self.show_pop_up(popup, "area:msg") def exit_compose_confirmation_popup(self) -> None: question = urwid.Text( @@ -543,8 +547,9 @@ def exit_compose_confirmation_popup(self) -> None: "center", ) write_box = self.view.write_box + popup_view = PopUpConfirmationView(self, question, write_box.exit_compose_box) - self.loop.widget = popup_view + self.show_pop_up(popup_view, "area:msg") def copy_to_clipboard(self, text: str, text_category: str) -> None: try: @@ -659,9 +664,10 @@ def prompting_exit_handler(self, signum: int, frame: Any) -> None: ("bold", " Please confirm that you wish to exit Zulip-Terminal "), "center", ) + popup_view = PopUpConfirmationView(self, question, self.deregister_client) - self.loop.widget = popup_view - self.loop.run() + self.show_pop_up(popup_view, "area:msg") + self.loop.run() # Appears necessary to return control from signal handler def _raise_exception(self, *args: Any, **kwargs: Any) -> Literal[True]: if self._exception_info is not None: diff --git a/zulipterminal/ui_tools/views.py b/zulipterminal/ui_tools/views.py index 8803150dfc..37a98cbe3c 100644 --- a/zulipterminal/ui_tools/views.py +++ b/zulipterminal/ui_tools/views.py @@ -1318,7 +1318,7 @@ def __init__(self, controller: Any, title: str) -> None: ) -class PopUpConfirmationView(urwid.Overlay): +class PopUpConfirmationView(urwid.Frame): def __init__( self, controller: Any, @@ -1334,23 +1334,18 @@ def __init__( display_widget = urwid.GridFlow([yes, no], 3, 5, 1, "center") wrapped_widget = urwid.WidgetWrap(display_widget) widgets = [question, urwid.Divider(), wrapped_widget] - prompt = urwid.LineBox(urwid.ListBox(urwid.SimpleFocusListWalker(widgets))) + prompt = urwid.ListBox(urwid.SimpleFocusListWalker(widgets)) + + self.title = None max_cols, max_rows = controller.maximum_popup_dimensions() - # +2 to compensate for the LineBox characters. - width = min(max_cols, max(question.pack()[0], len("Yes"), len("No"))) + 2 - height = min(max_rows, sum(widget.rows((width,)) for widget in widgets)) + 2 - - urwid.Overlay.__init__( - self, - prompt, - self.controller.view, - align="center", - valign="middle", - width=width, - height=height, + self.width = min(max_cols, max(question.pack()[0], len("Yes"), len("No"))) + self.height = min( + max_rows, sum(widget.rows((self.width,)) for widget in widgets) ) + super().__init__(prompt) + def exit_popup_yes(self, args: Any) -> None: self.success_callback() self.controller.exit_popup() From daca945e2e68c9ccac217b96f352aeefa8da2d40 Mon Sep 17 00:00:00 2001 From: "neiljp (Neil Pilgrim)" Date: Mon, 31 Jul 2023 17:31:29 -0700 Subject: [PATCH 6/6] refactor: views: Clarify PopupFrame base classes. --- zulipterminal/ui_tools/views.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/zulipterminal/ui_tools/views.py b/zulipterminal/ui_tools/views.py index 37a98cbe3c..e1cc937422 100644 --- a/zulipterminal/ui_tools/views.py +++ b/zulipterminal/ui_tools/views.py @@ -950,7 +950,7 @@ def __init__(self, text: str) -> None: PopUpViewTableContent = Sequence[Tuple[str, Sequence[Union[str, Tuple[str, Any]]]]] -class PopUpFrame(urwid.AttrMap): +class PopUpFrame(urwid.WidgetDecoration, urwid.WidgetWrap): def __init__(self, body: Any, title: Optional[str], style: str) -> None: content = urwid.LineBox(body, **POPUP_CONTENT_BORDER) @@ -964,7 +964,10 @@ def __init__(self, body: Any, title: Optional[str], style: str) -> None: else: titled_content = urwid.Frame(body=content) - super().__init__(titled_content, "popup_border") + styled = urwid.AttrMap(titled_content, "popup_border") + + urwid.WidgetDecoration.__init__(self, body) + urwid.WidgetWrap.__init__(self, styled) class PopUpView(urwid.Frame):