From 4fd0a50f51ccc953dcbabf0e7b6c930b191ec315 Mon Sep 17 00:00:00 2001 From: Subhasish-Behera Date: Sun, 16 Apr 2023 13:19:02 +0000 Subject: [PATCH 1/7] tests: boxes: Add baseline test for private_box_view recipient editing. This test verifies the assumption that the to_write_box is editable by default. This assumption will change when we create a specific "editing private message" box soon. Co-authored-by: zormit --- tests/ui_tools/test_boxes.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/ui_tools/test_boxes.py b/tests/ui_tools/test_boxes.py index 8fc050391f..e0e2744663 100644 --- a/tests/ui_tools/test_boxes.py +++ b/tests/ui_tools/test_boxes.py @@ -6,6 +6,7 @@ from pytest import param as case from pytest_mock import MockerFixture from urwid import Widget +from urwid_readline import ReadlineEdit from zulipterminal.api_types import ( TYPING_STARTED_EXPIRY_PERIOD, @@ -1718,6 +1719,35 @@ def focus_val(x: str) -> int: expected_focus_col_name ) + @pytest.mark.parametrize( + "recipient_ids, expected_recipient_text", + [ + ([], "To: "), + ([11], "To: Human 1 "), + ( + [11, 12], + "To: Human 1 , Human 2 ", + ), + ], + ) + def test_private_box_recipient_editing( + self, + mocker: MockerFixture, + write_box: WriteBox, + user_dict: List[Dict[str, Any]], + user_id_email_dict: Dict[int, str], + recipient_ids: List[int], + expected_recipient_text: str, + ) -> None: + write_box.model.user_id_email_dict = user_id_email_dict + write_box.model.user_dict = user_dict + mocker.patch("urwid.connect_signal") + + write_box.private_box_view(recipient_user_ids=recipient_ids) + + assert isinstance(write_box.to_write_box, ReadlineEdit) + assert write_box.to_write_box.text == expected_recipient_text + @pytest.mark.parametrize("key", keys_for_command("MARKDOWN_HELP")) def test_keypress_MARKDOWN_HELP( self, write_box: WriteBox, key: str, widget_size: Callable[[Widget], urwid_Size] From 9716ccc5635688716272e5de17a066985f53f3a0 Mon Sep 17 00:00:00 2001 From: Subhasish-Behera Date: Mon, 11 Sep 2023 19:42:32 +0530 Subject: [PATCH 2/7] refactor: boxes: Detail name of update_recipients method. Change the name to update_recipients_from_emails_in_widget, in order to be more specific in detail what this helper function is doing. This prepares for a change that will add another method on this class, that will update the recipient in a different way. Co-authored-by: zormit --- tests/ui_tools/test_boxes.py | 4 ++-- zulipterminal/ui_tools/boxes.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/ui_tools/test_boxes.py b/tests/ui_tools/test_boxes.py index e0e2744663..320cd515aa 100644 --- a/tests/ui_tools/test_boxes.py +++ b/tests/ui_tools/test_boxes.py @@ -421,7 +421,7 @@ def test_footer_notification_on_invalid_recipients( ), ], ) - def test_update_recipients( + def test_update_recipients_from_emails_in_widget( self, write_box: WriteBox, header: str, @@ -432,7 +432,7 @@ def test_update_recipients( assert write_box.to_write_box is not None write_box.to_write_box.edit_text = header - write_box.update_recipients(write_box.to_write_box) + write_box.update_recipients_from_emails_in_widget(write_box.to_write_box) assert write_box.recipient_emails == expected_recipient_emails assert write_box.recipient_user_ids == expected_recipient_user_ids diff --git a/zulipterminal/ui_tools/boxes.py b/zulipterminal/ui_tools/boxes.py index f6d3294241..2caf4268a7 100644 --- a/zulipterminal/ui_tools/boxes.py +++ b/zulipterminal/ui_tools/boxes.py @@ -265,7 +265,7 @@ def track_idleness_and_update_status() -> None: urwid.connect_signal(self.msg_write_box, "change", on_type_send_status) - def update_recipients(self, write_box: ReadlineEdit) -> None: + def update_recipients_from_emails_in_widget(self, write_box: ReadlineEdit) -> None: self.recipient_emails = re.findall(REGEX_RECIPIENT_EMAIL, write_box.edit_text) self._set_regular_and_typing_recipient_user_ids( [self.model.user_dict[email]["user_id"] for email in self.recipient_emails] @@ -761,7 +761,7 @@ def keypress(self, size: urwid_Size, key: str) -> Optional[str]: ) if not all_valid: return key - self.update_recipients(self.to_write_box) + self.update_recipients_from_emails_in_widget(self.to_write_box) if self.recipient_user_ids: success = self.model.send_private_message( recipients=self.recipient_user_ids, @@ -815,7 +815,7 @@ def keypress(self, size: urwid_Size, key: str) -> Optional[str]: ) if not all_valid: return key - self.update_recipients(self.to_write_box) + self.update_recipients_from_emails_in_widget(self.to_write_box) this_draft: Composition = PrivateComposition( type="private", to=self.recipient_user_ids, @@ -893,7 +893,7 @@ def keypress(self, size: urwid_Size, key: str) -> Optional[str]: # We extract recipients' user_ids and emails only once we know # that all the recipients are valid, to avoid including any # invalid ones. - self.update_recipients(self.to_write_box) + self.update_recipients_from_emails_in_widget(self.to_write_box) if not self.msg_body_edit_enabled: return key From 9feb30d903fb8f770c6e3a38859e9ac5875ede4b Mon Sep 17 00:00:00 2001 From: Subhasish-Behera Date: Sun, 16 Apr 2023 11:37:40 +0000 Subject: [PATCH 3/7] refactor: boxes: Extract method _setup_common_private_compose. Pull out a helper method when composing the private_box_view, similar to _setup_common_stream_compose. The helper mainly takes care of preparing the msg and header write boxes. This common functionality will be needed soon, when we build a different view for reply and edit state. Co-authored-by: zormit --- zulipterminal/ui_tools/boxes.py | 59 +++++++++++++++++---------------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/zulipterminal/ui_tools/boxes.py b/zulipterminal/ui_tools/boxes.py index 2caf4268a7..68ad54dcb6 100644 --- a/zulipterminal/ui_tools/boxes.py +++ b/zulipterminal/ui_tools/boxes.py @@ -173,15 +173,40 @@ def send_stop_typing_status(self) -> None: self.idle_status_tracking = False self.sent_start_typing_status = False + def _setup_common_private_compose(self) -> None: + self.set_editor_mode() + + self.compose_box_status = "open_with_private" + + self.msg_write_box = ReadlineEdit( + multiline=True, max_char=self.model.max_message_length + ) + self.msg_write_box.enable_autocomplete( + func=self.generic_autocomplete, + key=primary_key_for_command("AUTOCOMPLETE"), + key_reverse=primary_key_for_command("AUTOCOMPLETE_REVERSE"), + ) + self.msg_write_box.set_completer_delims(DELIMS_MESSAGE_COMPOSE) + + self.header_write_box = urwid.Columns([self.to_write_box]) + header_line_box = urwid.Pile( + [ + urwid.Divider(COMPOSE_HEADER_TOP), + self.header_write_box, + urwid.Divider(COMPOSE_HEADER_BOTTOM), + ] + ) + self.contents = [ + (header_line_box, self.options()), + (self.msg_write_box, self.options()), + ] + self.focus_position = self.FOCUS_CONTAINER_MESSAGE + def private_box_view( self, *, recipient_user_ids: Optional[List[int]] = None, ) -> None: - self.set_editor_mode() - - self.compose_box_status = "open_with_private" - if recipient_user_ids: self._set_regular_and_typing_recipient_user_ids(recipient_user_ids) self.recipient_emails = [ @@ -207,31 +232,7 @@ def private_box_view( key_reverse=primary_key_for_command("AUTOCOMPLETE_REVERSE"), ) self.to_write_box.set_completer_delims("") - - self.msg_write_box = ReadlineEdit( - multiline=True, max_char=self.model.max_message_length - ) - self.msg_write_box.enable_autocomplete( - func=self.generic_autocomplete, - key=primary_key_for_command("AUTOCOMPLETE"), - key_reverse=primary_key_for_command("AUTOCOMPLETE_REVERSE"), - ) - self.msg_write_box.set_completer_delims(DELIMS_MESSAGE_COMPOSE) - - self.header_write_box = urwid.Columns([self.to_write_box]) - header_line_box = urwid.Pile( - [ - urwid.Divider(COMPOSE_HEADER_TOP), - self.header_write_box, - urwid.Divider(COMPOSE_HEADER_BOTTOM), - ] - ) - self.contents = [ - (header_line_box, self.options()), - (self.msg_write_box, self.options()), - ] - self.focus_position = self.FOCUS_CONTAINER_MESSAGE - + self._setup_common_private_compose() start_period_delta = timedelta( milliseconds=self.model.typing_started_wait_period ) From 4921b982492e1a6d75583db53503d2e1ff7b15f2 Mon Sep 17 00:00:00 2001 From: Subhasish-Behera Date: Sun, 16 Apr 2023 11:48:24 +0000 Subject: [PATCH 4/7] refactor: boxes: Extract method update_recipients_from_user_ids. This helper method to build the recipient list will also be needed in the upcoming change. Co-authored-by: zormit --- zulipterminal/ui_tools/boxes.py | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/zulipterminal/ui_tools/boxes.py b/zulipterminal/ui_tools/boxes.py index 68ad54dcb6..9da5fa7521 100644 --- a/zulipterminal/ui_tools/boxes.py +++ b/zulipterminal/ui_tools/boxes.py @@ -202,18 +202,16 @@ def _setup_common_private_compose(self) -> None: ] self.focus_position = self.FOCUS_CONTAINER_MESSAGE - def private_box_view( - self, - *, - recipient_user_ids: Optional[List[int]] = None, - ) -> None: + def update_recipients_from_user_ids( + self, recipient_user_ids: Optional[List[int]] = None + ) -> str: if recipient_user_ids: self._set_regular_and_typing_recipient_user_ids(recipient_user_ids) self.recipient_emails = [ self.model.user_id_email_dict[user_id] for user_id in self.recipient_user_ids ] - recipient_info = ", ".join( + return ", ".join( [ f"{self.model.user_dict[email]['full_name']} <{email}>" for email in self.recipient_emails @@ -222,9 +220,15 @@ def private_box_view( else: self._set_regular_and_typing_recipient_user_ids(None) self.recipient_emails = [] - recipient_info = "" + return "" + def private_box_view( + self, + *, + recipient_user_ids: Optional[List[int]] = None, + ) -> None: self.send_next_typing_update = datetime.now() + recipient_info = self.update_recipients_from_user_ids(recipient_user_ids) self.to_write_box = ReadlineEdit("To: ", edit_text=recipient_info) self.to_write_box.enable_autocomplete( func=self._to_box_autocomplete, From 751db14810908b95240add08978b0ff2f61794f4 Mon Sep 17 00:00:00 2001 From: Subhasish-Behera Date: Tue, 25 Apr 2023 19:35:39 +0000 Subject: [PATCH 5/7] boxes: Disable cycling while editing a Private Message. In other words: CYCLE_COMPOSE_FOCUS is now deactivated for the case of editing a private message. This is one core change of this PR and solves the issue that it doesn't particularly make sense to move the focus to the recipients. Note that we don't yet disallow *clicking* into the recpient list, which will be a later change. Co-authored-by: zormit --- tests/ui_tools/test_boxes.py | 17 ++++++++++++++--- zulipterminal/ui_tools/boxes.py | 8 +++++++- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/tests/ui_tools/test_boxes.py b/tests/ui_tools/test_boxes.py index 320cd515aa..f97122eee3 100644 --- a/tests/ui_tools/test_boxes.py +++ b/tests/ui_tools/test_boxes.py @@ -1663,6 +1663,16 @@ def test_keypress_typeahead_mode_autocomplete_key( "HEADER_BOX_RECIPIENT", id="message_to_recipient_box", ), + case( + "CONTAINER_MESSAGE", + "MESSAGE_BOX_BODY", + "private", + True, + True, + "CONTAINER_MESSAGE", + "MESSAGE_BOX_BODY", + id="private_edit_box-no_cycle", + ), ], ) @pytest.mark.parametrize("tab_key", keys_for_command("CYCLE_COMPOSE_FOCUS")) @@ -1683,13 +1693,14 @@ def test_keypress_CYCLE_COMPOSE_FOCUS( ) -> None: mocker.patch(WRITEBOX + "._set_stream_write_box_style") + if message_being_edited: + write_box.msg_edit_state = _MessageEditState( + message_id=10, old_topic="some old topic" + ) if box_type == "stream": if message_being_edited: mocker.patch(MODULE + ".EditModeButton") write_box.stream_box_edit_view(stream_id) - write_box.msg_edit_state = _MessageEditState( - message_id=10, old_topic="some old topic" - ) else: write_box.stream_box_view(stream_id) else: diff --git a/zulipterminal/ui_tools/boxes.py b/zulipterminal/ui_tools/boxes.py index 9da5fa7521..e05a4cdff7 100644 --- a/zulipterminal/ui_tools/boxes.py +++ b/zulipterminal/ui_tools/boxes.py @@ -905,7 +905,13 @@ def keypress(self, size: urwid_Size, key: str) -> Optional[str]: if self.focus_position == self.FOCUS_CONTAINER_HEADER: self.focus_position = self.FOCUS_CONTAINER_MESSAGE else: - self.focus_position = self.FOCUS_CONTAINER_HEADER + if self.compose_box_status == "open_with_stream": + self.focus_position = self.FOCUS_CONTAINER_HEADER + if ( + self.compose_box_status == "open_with_private" + and self.msg_edit_state is None + ): + self.focus_position = self.FOCUS_CONTAINER_HEADER if self.compose_box_status == "open_with_stream": if self.msg_edit_state is not None: header.focus_col = self.FOCUS_HEADER_BOX_TOPIC From abab4f43123333c8b1070f34778f1d62f803f45c Mon Sep 17 00:00:00 2001 From: Subhasish-Behera Date: Sun, 16 Apr 2023 11:56:56 +0000 Subject: [PATCH 6/7] boxes/messages: Disable recpient editing when editing a private message. In order to fully disable editing (not only cycling to it, but e.g. prevent editing through mouse clicks) we need to make the editing of a message a different UI box than the usual reply. This new box uses a urwid.Text to display the recipient information rather than providing an editable field. Fixes #774. Co-authored-by: zormit --- zulipterminal/ui_tools/boxes.py | 9 +++++++++ zulipterminal/ui_tools/messages.py | 4 +++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/zulipterminal/ui_tools/boxes.py b/zulipterminal/ui_tools/boxes.py index e05a4cdff7..eb2f41dc09 100644 --- a/zulipterminal/ui_tools/boxes.py +++ b/zulipterminal/ui_tools/boxes.py @@ -222,6 +222,15 @@ def update_recipients_from_user_ids( self.recipient_emails = [] return "" + def private_box_edit_view( + self, + *, + recipient_user_ids: Optional[List[int]] = None, + ) -> None: + self.recipient_info = self.update_recipients_from_user_ids(recipient_user_ids) + self.to_write_box = urwid.Text("To: " + self.recipient_info) + self._setup_common_private_compose() + def private_box_view( self, *, diff --git a/zulipterminal/ui_tools/messages.py b/zulipterminal/ui_tools/messages.py index 3ddfa10a70..ade1407351 100644 --- a/zulipterminal/ui_tools/messages.py +++ b/zulipterminal/ui_tools/messages.py @@ -1096,7 +1096,9 @@ def keypress(self, size: urwid_Size, key: str) -> Optional[str]: ) if self.message["type"] == "private": - self.keypress(size, primary_key_for_command("REPLY_MESSAGE")) + self.model.controller.view.write_box.private_box_edit_view( + recipient_user_ids=self.recipient_ids, + ) elif self.message["type"] == "stream": self.model.controller.view.write_box.stream_box_edit_view( stream_id=self.stream_id, From 9b5ca868bbd83a6f3c4b6fc66c9a4ed9f3ba2286 Mon Sep 17 00:00:00 2001 From: Subhasish-Behera Date: Sun, 16 Apr 2023 13:22:21 +0000 Subject: [PATCH 7/7] tests: boxes: Ensure recipient box when editing is not writeable. Co-authored-by: zormit --- tests/ui_tools/test_boxes.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/ui_tools/test_boxes.py b/tests/ui_tools/test_boxes.py index f97122eee3..302f6ab38e 100644 --- a/tests/ui_tools/test_boxes.py +++ b/tests/ui_tools/test_boxes.py @@ -5,7 +5,7 @@ import pytest from pytest import param as case from pytest_mock import MockerFixture -from urwid import Widget +from urwid import Text, Widget from urwid_readline import ReadlineEdit from zulipterminal.api_types import ( @@ -1704,7 +1704,10 @@ def test_keypress_CYCLE_COMPOSE_FOCUS( else: write_box.stream_box_view(stream_id) else: - write_box.private_box_view() + if message_being_edited: + write_box.private_box_edit_view() + else: + write_box.private_box_view() size = widget_size(write_box) def focus_val(x: str) -> int: @@ -1730,6 +1733,7 @@ def focus_val(x: str) -> int: expected_focus_col_name ) + @pytest.mark.parametrize("message_being_edited", [(True), (False)]) @pytest.mark.parametrize( "recipient_ids, expected_recipient_text", [ @@ -1749,14 +1753,19 @@ def test_private_box_recipient_editing( user_id_email_dict: Dict[int, str], recipient_ids: List[int], expected_recipient_text: str, + message_being_edited: bool, ) -> None: write_box.model.user_id_email_dict = user_id_email_dict write_box.model.user_dict = user_dict mocker.patch("urwid.connect_signal") - write_box.private_box_view(recipient_user_ids=recipient_ids) + if message_being_edited: + write_box.private_box_edit_view(recipient_user_ids=recipient_ids) + assert isinstance(write_box.to_write_box, Text) + else: + write_box.private_box_view(recipient_user_ids=recipient_ids) + assert isinstance(write_box.to_write_box, ReadlineEdit) - assert isinstance(write_box.to_write_box, ReadlineEdit) assert write_box.to_write_box.text == expected_recipient_text @pytest.mark.parametrize("key", keys_for_command("MARKDOWN_HELP"))