From 15a7c86b742425c1f5982b018826db96c7591fee Mon Sep 17 00:00:00 2001 From: Niloth P <20315308+Niloth-p@users.noreply.github.com> Date: Tue, 23 Jul 2024 00:11:29 +0530 Subject: [PATCH 1/5] views: Use `self` to re-use instance variables in MsgInfoView class. And the TestMsgInfoView class. Tests updated. --- tests/ui_tools/test_popups.py | 66 +++++++++------------------------ zulipterminal/ui_tools/views.py | 28 +++++++------- 2 files changed, 32 insertions(+), 62 deletions(-) diff --git a/tests/ui_tools/test_popups.py b/tests/ui_tools/test_popups.py index 878be178c4..f994c75b4e 100644 --- a/tests/ui_tools/test_popups.py +++ b/tests/ui_tools/test_popups.py @@ -975,27 +975,28 @@ def mock_external_classes( "Tue Mar 13 10:55:22", "Tue Mar 13 10:55:37", ] + self.message = message_fixture self.msg_info_view = MsgInfoView( self.controller, - message_fixture, + self.message, "Message Information", OrderedDict(), OrderedDict(), list(), ) - def test_init(self, message_fixture: Message) -> None: - assert self.msg_info_view.msg == message_fixture + def test_init(self) -> None: + assert self.msg_info_view.msg == self.message assert self.msg_info_view.topic_links == OrderedDict() assert self.msg_info_view.message_links == OrderedDict() assert self.msg_info_view.time_mentions == list() - def test_pop_up_info_order(self, message_fixture: Message) -> None: + def test_pop_up_info_order(self) -> None: topic_links = OrderedDict([("https://bar.com", ("topic", 1, True))]) message_links = OrderedDict([("image.jpg", ("image", 1, True))]) msg_info_view = MsgInfoView( self.controller, - message_fixture, + self.message, title="Message Information", topic_links=topic_links, message_links=message_links, @@ -1029,7 +1030,6 @@ def test_keypress_any_key( ) def test_keypress_edit_history( self, - message_fixture: Message, key: str, widget_size: Callable[[Widget], urwid_Size], realm_allow_edit_history: bool, @@ -1041,21 +1041,13 @@ def test_keypress_edit_history( self.controller.model.initial_data = { "realm_allow_edit_history": realm_allow_edit_history, } - msg_info_view = MsgInfoView( - self.controller, - message_fixture, - title="Message Information", - topic_links=OrderedDict(), - message_links=OrderedDict(), - time_mentions=list(), - ) - size = widget_size(msg_info_view) + size = widget_size(self.msg_info_view) - msg_info_view.keypress(size, key) + self.msg_info_view.keypress(size, key) - if msg_info_view.show_edit_history_label: + if self.msg_info_view.show_edit_history_label: self.controller.show_edit_history.assert_called_once_with( - message=message_fixture, + message=self.message, topic_links=OrderedDict(), message_links=OrderedDict(), time_mentions=list(), @@ -1065,25 +1057,14 @@ def test_keypress_edit_history( @pytest.mark.parametrize("key", keys_for_command("FULL_RENDERED_MESSAGE")) def test_keypress_full_rendered_message( - self, - message_fixture: Message, - key: str, - widget_size: Callable[[Widget], urwid_Size], + self, key: str, widget_size: Callable[[Widget], urwid_Size] ) -> None: - msg_info_view = MsgInfoView( - self.controller, - message_fixture, - title="Message Information", - topic_links=OrderedDict(), - message_links=OrderedDict(), - time_mentions=list(), - ) - size = widget_size(msg_info_view) + size = widget_size(self.msg_info_view) - msg_info_view.keypress(size, key) + self.msg_info_view.keypress(size, key) self.controller.show_full_rendered_message.assert_called_once_with( - message=message_fixture, + message=self.message, topic_links=OrderedDict(), message_links=OrderedDict(), time_mentions=list(), @@ -1091,25 +1072,14 @@ def test_keypress_full_rendered_message( @pytest.mark.parametrize("key", keys_for_command("FULL_RAW_MESSAGE")) def test_keypress_full_raw_message( - self, - message_fixture: Message, - key: str, - widget_size: Callable[[Widget], urwid_Size], + self, key: str, widget_size: Callable[[Widget], urwid_Size] ) -> None: - msg_info_view = MsgInfoView( - self.controller, - message_fixture, - title="Message Information", - topic_links=OrderedDict(), - message_links=OrderedDict(), - time_mentions=list(), - ) - size = widget_size(msg_info_view) + size = widget_size(self.msg_info_view) - msg_info_view.keypress(size, key) + self.msg_info_view.keypress(size, key) self.controller.show_full_raw_message.assert_called_once_with( - message=message_fixture, + message=self.message, topic_links=OrderedDict(), message_links=OrderedDict(), time_mentions=list(), diff --git a/zulipterminal/ui_tools/views.py b/zulipterminal/ui_tools/views.py index 6d01a82566..1b4f207e5c 100644 --- a/zulipterminal/ui_tools/views.py +++ b/zulipterminal/ui_tools/views.py @@ -1585,7 +1585,7 @@ def __init__( self.time_mentions = time_mentions self.server_url = controller.model.server_url date_and_time = controller.model.formatted_local_time( - msg["timestamp"], show_seconds=True, show_year=True + self.msg["timestamp"], show_seconds=True, show_year=True ) view_in_browser_keys = "[{}]".format( ", ".join(map(str, display_keys_for_command("VIEW_IN_BROWSER"))) @@ -1602,8 +1602,8 @@ def __init__( "", [ ("Date & Time", date_and_time), - ("Sender", msg["sender_full_name"]), - ("Sender's Email ID", msg["sender_email"]), + ("Sender", self.msg["sender_full_name"]), + ("Sender's Email ID", self.msg["sender_email"]), ], ) ] @@ -1632,13 +1632,13 @@ def __init__( ) msg_info[1][1].append(("Edit History", keys)) # Render the category using the existing table methods if links exist. - if message_links: + if self.message_links: msg_info.append(("Message Links", [])) - if topic_links: + if self.topic_links: msg_info.append(("Topic Links", [])) - if time_mentions: - msg_info.append(("Time mentions", time_mentions)) - if msg["reactions"]: + if self.time_mentions: + msg_info.append(("Time mentions", self.time_mentions)) + if self.msg["reactions"]: reactions = sorted( (reaction["emoji_name"], reaction["user"]["full_name"]) for reaction in msg["reactions"] @@ -1658,9 +1658,9 @@ def __init__( # computing their slice indexes self.button_widgets: List[Any] = [] - if message_links: + if self.message_links: message_link_widgets, message_link_width = self.create_link_buttons( - controller, message_links + controller, self.message_links ) # slice_index = Number of labels before message links + 1 newline @@ -1669,16 +1669,16 @@ def __init__( slice_index = len(msg_info[0][1]) + len(msg_info[1][1]) + 2 + 2 slice_index += sum([len(w) + 2 for w in self.button_widgets]) - self.button_widgets.append(message_links) + self.button_widgets.append(self.message_links) widgets = ( widgets[:slice_index] + message_link_widgets + widgets[slice_index:] ) popup_width = max(popup_width, message_link_width) - if topic_links: + if self.topic_links: topic_link_widgets, topic_link_width = self.create_link_buttons( - controller, topic_links + controller, self.topic_links ) # slice_index = Number of labels before topic links + 1 newline @@ -1686,7 +1686,7 @@ def __init__( # + 2 for Viewing Actions category label and its newline slice_index = len(msg_info[0][1]) + len(msg_info[1][1]) + 2 + 2 slice_index += sum([len(w) + 2 for w in self.button_widgets]) - self.button_widgets.append(topic_links) + self.button_widgets.append(self.topic_links) widgets = widgets[:slice_index] + topic_link_widgets + widgets[slice_index:] popup_width = max(popup_width, topic_link_width) From c734d2a6b21db693843241a6c6752a727a552306 Mon Sep 17 00:00:00 2001 From: Niloth P <20315308+Niloth-p@users.noreply.github.com> Date: Mon, 22 Jul 2024 23:54:31 +0530 Subject: [PATCH 2/5] core/views: Stack inner popups, add support for exiting all popups. Support 2 functions. - Exiting the current popup - goes back to the previous popup. - Exiting all popups - clears all popups. Why stack popups? Currently, we create a new popup even when closing the uppermost popups. With stacking, we can re-use the popups. This allows us to not pass the entire data for creating the inner popups to the outer popups. Currently, the only popup that opens other popups is the MsgInfoView. Affected widgets: EditHistoryView, FullRenderedMsgView, FullRawMsgView. Updated them to use the newly added functions to exit popups. Hotkey behavior (from the widget): - Press Esc or respective command -> go to MsgInfoView popup - Press i from the widget -> close all popups (EXIT_POPUP command) By adding new keypress handling to their parent PopupView, we can pass their respective commands without having to implement custom keypress handlers. Tests updated. Renamed tests: - test_keypress_show_msg_info -> test_keypress_exit_to_msg_info. - test_keypress_exit_popup -> test_keypress_exit_all_popups. --- tests/ui_tools/test_popups.py | 37 +++++++--------------- zulipterminal/core.py | 7 +++++ zulipterminal/ui_tools/views.py | 55 ++++++++++----------------------- 3 files changed, 34 insertions(+), 65 deletions(-) diff --git a/tests/ui_tools/test_popups.py b/tests/ui_tools/test_popups.py index f994c75b4e..59e7a42dd6 100644 --- a/tests/ui_tools/test_popups.py +++ b/tests/ui_tools/test_popups.py @@ -520,14 +520,14 @@ def test_init(self, msg_box: MessageBox) -> None: assert self.full_rendered_message.footer.widget_list == msg_box.footer @pytest.mark.parametrize("key", keys_for_command("MSG_INFO")) - def test_keypress_exit_popup( + def test_keypress_exit_all_popups( self, key: str, widget_size: Callable[[Widget], urwid_Size] ) -> None: size = widget_size(self.full_rendered_message) self.full_rendered_message.keypress(size, key) - assert self.controller.exit_popup.called + assert self.controller.exit_all_popups.called def test_keypress_exit_popup_invalid_key( self, widget_size: Callable[[Widget], urwid_Size] @@ -546,19 +546,14 @@ def test_keypress_exit_popup_invalid_key( *keys_for_command("EXIT_POPUP"), }, ) - def test_keypress_show_msg_info( + def test_keypress_exit_to_msg_info( self, key: str, widget_size: Callable[[Widget], urwid_Size] ) -> None: size = widget_size(self.full_rendered_message) self.full_rendered_message.keypress(size, key) - self.controller.show_msg_info.assert_called_once_with( - msg=self.message, - topic_links=OrderedDict(), - message_links=OrderedDict(), - time_mentions=list(), - ) + assert self.controller.exit_popup.called class TestFullRawMsgView: @@ -596,14 +591,14 @@ def test_init(self, msg_box: MessageBox) -> None: assert self.full_raw_message.footer.widget_list == msg_box.footer @pytest.mark.parametrize("key", keys_for_command("MSG_INFO")) - def test_keypress_exit_popup( + def test_keypress_exit_all_popups( self, key: str, widget_size: Callable[[Widget], urwid_Size] ) -> None: size = widget_size(self.full_raw_message) self.full_raw_message.keypress(size, key) - assert self.controller.exit_popup.called + assert self.controller.exit_all_popups.called def test_keypress_exit_popup_invalid_key( self, widget_size: Callable[[Widget], urwid_Size] @@ -622,19 +617,14 @@ def test_keypress_exit_popup_invalid_key( *keys_for_command("EXIT_POPUP"), }, ) - def test_keypress_show_msg_info( + def test_keypress_exit_to_msg_info( self, key: str, widget_size: Callable[[Widget], urwid_Size] ) -> None: size = widget_size(self.full_raw_message) self.full_raw_message.keypress(size, key) - self.controller.show_msg_info.assert_called_once_with( - msg=self.message, - topic_links=OrderedDict(), - message_links=OrderedDict(), - time_mentions=list(), - ) + assert self.controller.exit_popup.called class TestEditHistoryView: @@ -671,14 +661,14 @@ def test_init(self) -> None: ) @pytest.mark.parametrize("key", keys_for_command("MSG_INFO")) - def test_keypress_exit_popup( + def test_keypress_exit_all_popups( self, key: str, widget_size: Callable[[Widget], urwid_Size] ) -> None: size = widget_size(self.edit_history_view) self.edit_history_view.keypress(size, key) - assert self.controller.exit_popup.called + assert self.controller.exit_all_popups.called def test_keypress_exit_popup_invalid_key( self, widget_size: Callable[[Widget], urwid_Size] @@ -700,12 +690,7 @@ def test_keypress_show_msg_info( self.edit_history_view.keypress(size, key) - self.controller.show_msg_info.assert_called_once_with( - msg=self.message, - topic_links=OrderedDict(), - message_links=OrderedDict(), - time_mentions=list(), - ) + assert self.controller.exit_popup.called @pytest.mark.parametrize( "snapshot", diff --git a/zulipterminal/core.py b/zulipterminal/core.py index a23b1596f1..6bc2aee480 100644 --- a/zulipterminal/core.py +++ b/zulipterminal/core.py @@ -86,6 +86,7 @@ def __init__( self.notify_enabled = notify self.maximum_footlinks = maximum_footlinks self.editor_command = editor_command + self.popup_stack: List[urwid.Widget] = [] self.debug_path = debug_path @@ -226,6 +227,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: + self.popup_stack.append(self.loop.widget) + 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) @@ -249,6 +252,10 @@ def is_any_popup_open(self) -> bool: return isinstance(self.loop.widget, urwid.Overlay) def exit_popup(self) -> None: + self.loop.widget = self.popup_stack.pop() + + def exit_all_popups(self) -> None: + self.popup_stack.clear() self.loop.widget = self.view def show_help(self) -> None: diff --git a/zulipterminal/ui_tools/views.py b/zulipterminal/ui_tools/views.py index 1b4f207e5c..08a54967c5 100644 --- a/zulipterminal/ui_tools/views.py +++ b/zulipterminal/ui_tools/views.py @@ -959,12 +959,14 @@ def __init__( title: str, header: Optional[Any] = None, footer: Optional[Any] = None, + command_full_exit: Optional[str] = None, ) -> None: self.controller = controller self.command = command self.title = title self.log = urwid.SimpleFocusListWalker(body) self.body = urwid.ListBox(self.log) + self.command_full_exit = command_full_exit max_cols, max_rows = controller.maximum_popup_dimensions() @@ -1060,7 +1062,8 @@ def make_table_with_categories( def keypress(self, size: urwid_Size, key: str) -> str: if is_command_key("EXIT_POPUP", key) or is_command_key(self.command, key): self.controller.exit_popup() - + elif self.command_full_exit and is_command_key(self.command_full_exit, key): + self.controller.exit_all_popups() return super().keypress(size, key) @@ -1831,7 +1834,14 @@ def __init__( ] widgets.append(urwid.Text(feedback, align="center")) - super().__init__(controller, widgets, "MSG_INFO", width, title) + super().__init__( + controller, + widgets, + "EDIT_HISTORY", + width, + title, + command_full_exit="MSG_INFO", + ) def _make_edit_block(self, snapshot: Dict[str, Any], tag: EditHistoryTag) -> Any: content = snapshot["content"] @@ -1896,17 +1906,6 @@ def _get_author_prefix(snapshot: Dict[str, Any], tag: EditHistoryTag) -> str: return author_prefix - def keypress(self, size: urwid_Size, key: str) -> str: - if is_command_key("EXIT_POPUP", key) or is_command_key("EDIT_HISTORY", key): - self.controller.show_msg_info( - msg=self.message, - topic_links=self.topic_links, - message_links=self.message_links, - time_mentions=self.time_mentions, - ) - return key - return super().keypress(size, key) - class FullRenderedMsgView(PopUpView): def __init__( @@ -1931,26 +1930,14 @@ def __init__( super().__init__( controller, [msg_box.content], - "MSG_INFO", + "FULL_RENDERED_MESSAGE", max_cols, title, urwid.Pile(msg_box.header), urwid.Pile(msg_box.footer), + command_full_exit="MSG_INFO", ) - def keypress(self, size: urwid_Size, key: str) -> str: - if is_command_key("EXIT_POPUP", key) or is_command_key( - "FULL_RENDERED_MESSAGE", key - ): - self.controller.show_msg_info( - msg=self.message, - topic_links=self.topic_links, - message_links=self.message_links, - time_mentions=self.time_mentions, - ) - return key - return super().keypress(size, key) - class FullRawMsgView(PopUpView): def __init__( @@ -1983,24 +1970,14 @@ def __init__( super().__init__( controller, body_list, - "MSG_INFO", + "FULL_RAW_MESSAGE", max_cols, title, urwid.Pile(msg_box.header), urwid.Pile(msg_box.footer), + command_full_exit="MSG_INFO", ) - def keypress(self, size: urwid_Size, key: str) -> str: - if is_command_key("EXIT_POPUP", key) or is_command_key("FULL_RAW_MESSAGE", key): - self.controller.show_msg_info( - msg=self.message, - topic_links=self.topic_links, - message_links=self.message_links, - time_mentions=self.time_mentions, - ) - return key - return super().keypress(size, key) - class EmojiPickerView(PopUpView): """ From e3163272d690916d50a4b95e44d27f24633c69ed Mon Sep 17 00:00:00 2001 From: Niloth P <20315308+Niloth-p@users.noreply.github.com> Date: Tue, 23 Jul 2024 00:17:06 +0530 Subject: [PATCH 3/5] views/core: Delete all message info popup parameters from inner popups. Now that the inner popups can exit to the outer popup without having to initialize it, we can remove the parameters of MsgInfoView class from its inner popup classes and their respective show_* functions. Tests updated. --- tests/ui_tools/test_popups.py | 18 ------------ zulipterminal/core.py | 50 ++++----------------------------- zulipterminal/ui_tools/views.py | 18 ------------ 3 files changed, 6 insertions(+), 80 deletions(-) diff --git a/tests/ui_tools/test_popups.py b/tests/ui_tools/test_popups.py index 59e7a42dd6..0afb6eaea0 100644 --- a/tests/ui_tools/test_popups.py +++ b/tests/ui_tools/test_popups.py @@ -503,9 +503,6 @@ def mock_external_classes(self, mocker: MockerFixture, msg_box: MessageBox) -> N self.full_rendered_message = FullRenderedMsgView( controller=self.controller, message=self.message, - topic_links=OrderedDict(), - message_links=OrderedDict(), - time_mentions=list(), title="Full Rendered Message", ) @@ -513,9 +510,6 @@ def test_init(self, msg_box: MessageBox) -> None: assert self.full_rendered_message.title == "Full Rendered Message" assert self.full_rendered_message.controller == self.controller assert self.full_rendered_message.message == self.message - assert self.full_rendered_message.topic_links == OrderedDict() - assert self.full_rendered_message.message_links == OrderedDict() - assert self.full_rendered_message.time_mentions == list() assert self.full_rendered_message.header.widget_list == msg_box.header assert self.full_rendered_message.footer.widget_list == msg_box.footer @@ -574,9 +568,6 @@ def mock_external_classes(self, mocker: MockerFixture, msg_box: MessageBox) -> N self.full_raw_message = FullRawMsgView( controller=self.controller, message=self.message, - topic_links=OrderedDict(), - message_links=OrderedDict(), - time_mentions=list(), title="Full Raw Message", ) @@ -584,9 +575,6 @@ def test_init(self, msg_box: MessageBox) -> None: assert self.full_raw_message.title == "Full Raw Message" assert self.full_raw_message.controller == self.controller assert self.full_raw_message.message == self.message - assert self.full_raw_message.topic_links == OrderedDict() - assert self.full_raw_message.message_links == OrderedDict() - assert self.full_raw_message.time_mentions == list() assert self.full_raw_message.header.widget_list == msg_box.header assert self.full_raw_message.footer.widget_list == msg_box.footer @@ -644,18 +632,12 @@ def mock_external_classes(self, mocker: MockerFixture) -> None: self.edit_history_view = EditHistoryView( controller=self.controller, message=self.message, - topic_links=OrderedDict(), - message_links=OrderedDict(), - time_mentions=list(), title="Edit History", ) def test_init(self) -> None: assert self.edit_history_view.controller == self.controller assert self.edit_history_view.message == self.message - assert self.edit_history_view.topic_links == OrderedDict() - assert self.edit_history_view.message_links == OrderedDict() - assert self.edit_history_view.time_mentions == list() self.controller.model.fetch_message_history.assert_called_once_with( message_id=self.message["id"], ) diff --git a/zulipterminal/core.py b/zulipterminal/core.py index 6bc2aee480..64a80b7833 100644 --- a/zulipterminal/core.py +++ b/zulipterminal/core.py @@ -348,61 +348,23 @@ def show_msg_sender_info(self, user_id: int) -> None: "area:user", ) - def show_full_rendered_message( - self, - message: Message, - topic_links: Dict[str, Tuple[str, int, bool]], - message_links: Dict[str, Tuple[str, int, bool]], - time_mentions: List[Tuple[str, str]], - ) -> None: + def show_full_rendered_message(self, message: Message) -> None: self.show_pop_up( FullRenderedMsgView( - self, - message, - topic_links, - message_links, - time_mentions, - f"Full rendered message {SCROLL_PROMPT}", + self, message, f"Full rendered message {SCROLL_PROMPT}" ), "area:msg", ) - def show_full_raw_message( - self, - message: Message, - topic_links: Dict[str, Tuple[str, int, bool]], - message_links: Dict[str, Tuple[str, int, bool]], - time_mentions: List[Tuple[str, str]], - ) -> None: + def show_full_raw_message(self, message: Message) -> None: self.show_pop_up( - FullRawMsgView( - self, - message, - topic_links, - message_links, - time_mentions, - f"Full raw message {SCROLL_PROMPT}", - ), + FullRawMsgView(self, message, f"Full raw message {SCROLL_PROMPT}"), "area:msg", ) - def show_edit_history( - self, - message: Message, - topic_links: Dict[str, Tuple[str, int, bool]], - message_links: Dict[str, Tuple[str, int, bool]], - time_mentions: List[Tuple[str, str]], - ) -> None: + def show_edit_history(self, message: Message) -> None: self.show_pop_up( - EditHistoryView( - self, - message, - topic_links, - message_links, - time_mentions, - f"Edit History {SCROLL_PROMPT}", - ), - "area:msg", + EditHistoryView(self, message, f"Edit History {SCROLL_PROMPT}"), "area:msg" ) def open_in_browser(self, url: str) -> None: diff --git a/zulipterminal/ui_tools/views.py b/zulipterminal/ui_tools/views.py index 08a54967c5..6b59a8275b 100644 --- a/zulipterminal/ui_tools/views.py +++ b/zulipterminal/ui_tools/views.py @@ -1798,16 +1798,10 @@ def __init__( self, controller: Any, message: Message, - topic_links: Dict[str, Tuple[str, int, bool]], - message_links: Dict[str, Tuple[str, int, bool]], - time_mentions: List[Tuple[str, str]], title: str, ) -> None: self.controller = controller self.message = message - self.topic_links = topic_links - self.message_links = message_links - self.time_mentions = time_mentions width = 64 widgets: List[Any] = [] @@ -1912,16 +1906,10 @@ def __init__( self, controller: Any, message: Message, - topic_links: Dict[str, Tuple[str, int, bool]], - message_links: Dict[str, Tuple[str, int, bool]], - time_mentions: List[Tuple[str, str]], title: str, ) -> None: self.controller = controller self.message = message - self.topic_links = topic_links - self.message_links = message_links - self.time_mentions = time_mentions max_cols, max_rows = controller.maximum_popup_dimensions() # Get rendered message @@ -1944,16 +1932,10 @@ def __init__( self, controller: Any, message: Message, - topic_links: Dict[str, Tuple[str, int, bool]], - message_links: Dict[str, Tuple[str, int, bool]], - time_mentions: List[Tuple[str, str]], title: str, ) -> None: self.controller = controller self.message = message - self.topic_links = topic_links - self.message_links = message_links - self.time_mentions = time_mentions max_cols, max_rows = controller.maximum_popup_dimensions() # Get rendered message header and footer From 9e5036e91425dd92f3118f13ec6ec67bd6dc58d5 Mon Sep 17 00:00:00 2001 From: Niloth P <20315308+Niloth-p@users.noreply.github.com> Date: Tue, 23 Jul 2024 00:22:32 +0530 Subject: [PATCH 4/5] views: Delete the calling arguments from MsgInfoView's show_* calls. Tests updated. --- tests/ui_tools/test_popups.py | 15 +++------------ zulipterminal/ui_tools/views.py | 21 +++------------------ 2 files changed, 6 insertions(+), 30 deletions(-) diff --git a/tests/ui_tools/test_popups.py b/tests/ui_tools/test_popups.py index 0afb6eaea0..4d89887869 100644 --- a/tests/ui_tools/test_popups.py +++ b/tests/ui_tools/test_popups.py @@ -1014,10 +1014,7 @@ def test_keypress_edit_history( if self.msg_info_view.show_edit_history_label: self.controller.show_edit_history.assert_called_once_with( - message=self.message, - topic_links=OrderedDict(), - message_links=OrderedDict(), - time_mentions=list(), + message=self.message ) else: self.controller.show_edit_history.assert_not_called() @@ -1031,10 +1028,7 @@ def test_keypress_full_rendered_message( self.msg_info_view.keypress(size, key) self.controller.show_full_rendered_message.assert_called_once_with( - message=self.message, - topic_links=OrderedDict(), - message_links=OrderedDict(), - time_mentions=list(), + message=self.message ) @pytest.mark.parametrize("key", keys_for_command("FULL_RAW_MESSAGE")) @@ -1046,10 +1040,7 @@ def test_keypress_full_raw_message( self.msg_info_view.keypress(size, key) self.controller.show_full_raw_message.assert_called_once_with( - message=self.message, - topic_links=OrderedDict(), - message_links=OrderedDict(), - time_mentions=list(), + message=self.message ) @pytest.mark.parametrize( diff --git a/zulipterminal/ui_tools/views.py b/zulipterminal/ui_tools/views.py index 6b59a8275b..6602a2285b 100644 --- a/zulipterminal/ui_tools/views.py +++ b/zulipterminal/ui_tools/views.py @@ -1725,30 +1725,15 @@ def create_link_buttons( def keypress(self, size: urwid_Size, key: str) -> str: if is_command_key("EDIT_HISTORY", key) and self.show_edit_history_label: - self.controller.show_edit_history( - message=self.msg, - topic_links=self.topic_links, - message_links=self.message_links, - time_mentions=self.time_mentions, - ) + self.controller.show_edit_history(message=self.msg) elif is_command_key("VIEW_IN_BROWSER", key): url = near_message_url(self.server_url[:-1], self.msg) self.controller.open_in_browser(url) elif is_command_key("FULL_RENDERED_MESSAGE", key): - self.controller.show_full_rendered_message( - message=self.msg, - topic_links=self.topic_links, - message_links=self.message_links, - time_mentions=self.time_mentions, - ) + self.controller.show_full_rendered_message(message=self.msg) return key elif is_command_key("FULL_RAW_MESSAGE", key): - self.controller.show_full_raw_message( - message=self.msg, - topic_links=self.topic_links, - message_links=self.message_links, - time_mentions=self.time_mentions, - ) + self.controller.show_full_raw_message(message=self.msg) return key return super().keypress(size, key) From 9051d4f64e4eb70f5adb0bf9c030652e7f704a30 Mon Sep 17 00:00:00 2001 From: Niloth P <20315308+Niloth-p@users.noreply.github.com> Date: Tue, 23 Jul 2024 00:25:44 +0530 Subject: [PATCH 5/5] refactor: views/helper/core/messages: Group message info in a TypedDict. Group the following fields that are shown in a message info popup, into a single piece of data. - topic_links - message_links - time_mentions Tests updated. Fixture added. --- tests/ui_tools/test_popups.py | 43 +++++++++++++++++++++--------- zulipterminal/core.py | 15 +++-------- zulipterminal/helper.py | 6 +++++ zulipterminal/ui_tools/messages.py | 9 +++++-- zulipterminal/ui_tools/views.py | 11 ++++---- 5 files changed, 52 insertions(+), 32 deletions(-) diff --git a/tests/ui_tools/test_popups.py b/tests/ui_tools/test_popups.py index 4d89887869..08f7dbe9d5 100644 --- a/tests/ui_tools/test_popups.py +++ b/tests/ui_tools/test_popups.py @@ -9,7 +9,11 @@ from zulipterminal.api_types import Message from zulipterminal.config.keys import is_command_key, keys_for_command from zulipterminal.config.ui_mappings import EDIT_MODE_CAPTIONS -from zulipterminal.helper import CustomProfileData, TidiedUserInfo +from zulipterminal.helper import ( + CustomProfileData, + MessageInfoPopupContent, + TidiedUserInfo, +) from zulipterminal.ui_tools.messages import MessageBox from zulipterminal.ui_tools.views import ( AboutView, @@ -922,10 +926,22 @@ def test_keypress_exit_popup( assert self.controller.exit_popup.called +@pytest.fixture +def message_info_content() -> MessageInfoPopupContent: + return MessageInfoPopupContent( + topic_links=OrderedDict(), + message_links=OrderedDict(), + time_mentions=list(), + ) + + class TestMsgInfoView: @pytest.fixture(autouse=True) def mock_external_classes( - self, mocker: MockerFixture, message_fixture: Message + self, + mocker: MockerFixture, + message_fixture: Message, + message_info_content: MessageInfoPopupContent, ) -> None: self.controller = mocker.Mock() mocker.patch.object( @@ -943,13 +959,12 @@ def mock_external_classes( "Tue Mar 13 10:55:37", ] self.message = message_fixture + self.message_info_content = message_info_content self.msg_info_view = MsgInfoView( self.controller, self.message, "Message Information", - OrderedDict(), - OrderedDict(), - list(), + self.message_info_content, ) def test_init(self) -> None: @@ -961,16 +976,22 @@ def test_init(self) -> None: def test_pop_up_info_order(self) -> None: topic_links = OrderedDict([("https://bar.com", ("topic", 1, True))]) message_links = OrderedDict([("image.jpg", ("image", 1, True))]) + message_info_content = MessageInfoPopupContent( + topic_links=topic_links, + message_links=message_links, + time_mentions=list(), + ) msg_info_view = MsgInfoView( self.controller, self.message, title="Message Information", - topic_links=topic_links, - message_links=message_links, - time_mentions=list(), + message_info_content=message_info_content, ) msg_links = msg_info_view.button_widgets - assert msg_links == [message_links, topic_links] + assert msg_links == [ + message_info_content["message_links"], + message_info_content["topic_links"], + ] def test_keypress_any_key( self, widget_size: Callable[[Widget], urwid_Size] @@ -1139,9 +1160,7 @@ def test_height_reactions( self.controller, varied_message, "Message Information", - OrderedDict(), - OrderedDict(), - list(), + self.message_info_content, ) # 12 = 7 labels + 2 blank lines + 1 'Reactions' (category) # + 4 reactions (excluding 'Message Links'). diff --git a/zulipterminal/core.py b/zulipterminal/core.py index 64a80b7833..ca1dbc46af 100644 --- a/zulipterminal/core.py +++ b/zulipterminal/core.py @@ -25,7 +25,7 @@ MAX_LINEAR_SCALING_WIDTH, MIN_SUPPORTED_POPUP_WIDTH, ) -from zulipterminal.helper import asynch, suppress_output +from zulipterminal.helper import MessageInfoPopupContent, asynch, suppress_output from zulipterminal.model import Model from zulipterminal.platform_code import PLATFORM from zulipterminal.ui import Screen, View @@ -270,19 +270,10 @@ def show_topic_edit_mode(self, button: Any) -> None: self.show_pop_up(EditModeView(self, button), "area:msg") def show_msg_info( - self, - msg: Message, - topic_links: Dict[str, Tuple[str, int, bool]], - message_links: Dict[str, Tuple[str, int, bool]], - time_mentions: List[Tuple[str, str]], + self, msg: Message, message_info_content: MessageInfoPopupContent ) -> None: msg_info_view = MsgInfoView( - self, - msg, - f"Message Information {SCROLL_PROMPT}", - topic_links, - message_links, - time_mentions, + self, msg, f"Message Information {SCROLL_PROMPT}", message_info_content ) self.show_pop_up(msg_info_view, "area:msg") diff --git a/zulipterminal/helper.py b/zulipterminal/helper.py index a71d055b74..361a9e59c1 100644 --- a/zulipterminal/helper.py +++ b/zulipterminal/helper.py @@ -57,6 +57,12 @@ class StreamData(TypedDict): description: str +class MessageInfoPopupContent(TypedDict): + topic_links: Dict[str, Tuple[str, int, bool]] + message_links: Dict[str, Tuple[str, int, bool]] + time_mentions: List[Tuple[str, str]] + + class EmojiData(TypedDict): code: str aliases: List[str] diff --git a/zulipterminal/ui_tools/messages.py b/zulipterminal/ui_tools/messages.py index b8552fdc92..e182e3454d 100644 --- a/zulipterminal/ui_tools/messages.py +++ b/zulipterminal/ui_tools/messages.py @@ -29,7 +29,7 @@ TIME_MENTION_MARKER, ) from zulipterminal.config.ui_mappings import STATE_ICON, STREAM_ACCESS_TYPE -from zulipterminal.helper import get_unused_fence +from zulipterminal.helper import MessageInfoPopupContent, get_unused_fence from zulipterminal.server_url import near_message_url from zulipterminal.ui_tools.tables import render_table from zulipterminal.urwid_types import urwid_MarkupTuple, urwid_Size @@ -1121,7 +1121,12 @@ def keypress(self, size: urwid_Size, key: str) -> Optional[str]: self.model.controller.view.middle_column.set_focus("footer") elif is_command_key("MSG_INFO", key): self.model.controller.show_msg_info( - self.message, self.topic_links, self.message_links, self.time_mentions + self.message, + MessageInfoPopupContent( + topic_links=self.topic_links, + message_links=self.message_links, + time_mentions=self.time_mentions, + ), ) elif is_command_key("ADD_REACTION", key): self.model.controller.show_emoji_picker(self.message) diff --git a/zulipterminal/ui_tools/views.py b/zulipterminal/ui_tools/views.py index 6602a2285b..1b2969e0cc 100644 --- a/zulipterminal/ui_tools/views.py +++ b/zulipterminal/ui_tools/views.py @@ -36,6 +36,7 @@ ) from zulipterminal.config.ui_sizes import LEFT_WIDTH from zulipterminal.helper import ( + MessageInfoPopupContent, TidiedUserInfo, asynch, match_emoji, @@ -1578,14 +1579,12 @@ def __init__( controller: Any, msg: Message, title: str, - topic_links: Dict[str, Tuple[str, int, bool]], - message_links: Dict[str, Tuple[str, int, bool]], - time_mentions: List[Tuple[str, str]], + message_info_content: MessageInfoPopupContent, ) -> None: self.msg = msg - self.topic_links = topic_links - self.message_links = message_links - self.time_mentions = time_mentions + self.topic_links = message_info_content["topic_links"] + self.message_links = message_info_content["message_links"] + self.time_mentions = message_info_content["time_mentions"] self.server_url = controller.model.server_url date_and_time = controller.model.formatted_local_time( self.msg["timestamp"], show_seconds=True, show_year=True