From 11f8d9f57c082547d8e5f40dd3e64f9503057f28 Mon Sep 17 00:00:00 2001 From: rsashank Date: Wed, 8 Nov 2023 23:17:06 +0530 Subject: [PATCH] core/boxes: Improve handling of pressing Esc during message compose. Introduces variable MAX_MESSAGE_LENGTH_CONFIRMATION_POPUP to set a threshold for maximum length of message in compose box beyond which it prompts a confirmation popup instead of the current instant exit when Esc is pressed. This specifically avoids prompting for - message editing, rather than new compositions - when the message content is identical to the saved draft This adds a call to the recently added _set_default_footer_after_autocomplete() in the exit_compose_box() method, to reset the footer in case autocomplete was in use. However, the footer is currently always reset near the top of the keypress() method, before that point (see added TODO comment). This is since autocomplete does not correctly fully resume even if compose itself is resumed with the appropriate EXIT_COMPOSE exclusion near that TODO comment - the autocomplete state itself must also somehow be resumed for this to work. Tests added and updated. Fixes #1342. --- tests/ui_tools/test_boxes.py | 68 +++++++++++++++++++++++++++++---- zulipterminal/core.py | 15 ++++++++ zulipterminal/ui_tools/boxes.py | 31 ++++++++++++++- 3 files changed, 105 insertions(+), 9 deletions(-) diff --git a/tests/ui_tools/test_boxes.py b/tests/ui_tools/test_boxes.py index 4b28a1e02b..adb417bc17 100644 --- a/tests/ui_tools/test_boxes.py +++ b/tests/ui_tools/test_boxes.py @@ -25,7 +25,12 @@ ) from zulipterminal.config.ui_mappings import StreamAccessType from zulipterminal.helper import Index, MinimalUserData -from zulipterminal.ui_tools.boxes import PanelSearchBox, WriteBox, _MessageEditState +from zulipterminal.ui_tools.boxes import ( + MAX_MESSAGE_LENGTH_CONFIRMATION_POPUP, + PanelSearchBox, + WriteBox, + _MessageEditState, +) from zulipterminal.urwid_types import urwid_Size @@ -236,7 +241,7 @@ def test_not_calling_send_private_message_without_recipients( assert not write_box.model.send_private_message.called @pytest.mark.parametrize("key", keys_for_command("EXIT_COMPOSE")) - def test__compose_attributes_reset_for_private_compose( + def test__compose_attributes_reset_for_private_compose__no_popup( self, key: str, mocker: MockerFixture, @@ -247,17 +252,41 @@ def test__compose_attributes_reset_for_private_compose( mocker.patch("urwid.connect_signal") write_box.model.user_id_email_dict = user_id_email_dict write_box.private_box_view(recipient_user_ids=[11]) - write_box.msg_write_box.edit_text = "random text" + + write_box.msg_write_box.edit_text = "." * ( + MAX_MESSAGE_LENGTH_CONFIRMATION_POPUP - 1 + ) size = widget_size(write_box) write_box.keypress(size, key) + write_box.view.controller.exit_compose_confirmation_popup.assert_not_called() assert write_box.to_write_box is None assert write_box.msg_write_box.edit_text == "" assert write_box.compose_box_status == "closed" @pytest.mark.parametrize("key", keys_for_command("EXIT_COMPOSE")) - def test__compose_attributes_reset_for_stream_compose( + def test__compose_attributes_reset_for_private_compose__popup( + self, + key: str, + mocker: MockerFixture, + write_box: WriteBox, + widget_size: Callable[[Widget], urwid_Size], + user_id_email_dict: Dict[int, str], + ) -> None: + mocker.patch("urwid.connect_signal") + write_box.model.user_id_email_dict = user_id_email_dict + write_box.private_box_view(recipient_user_ids=[11]) + + write_box.msg_write_box.edit_text = "." * MAX_MESSAGE_LENGTH_CONFIRMATION_POPUP + + size = widget_size(write_box) + write_box.keypress(size, key) + + write_box.view.controller.exit_compose_confirmation_popup.assert_called_once() + + @pytest.mark.parametrize("key", keys_for_command("EXIT_COMPOSE")) + def test__compose_attributes_reset_for_stream_compose__no_popup( self, key: str, mocker: MockerFixture, @@ -266,15 +295,37 @@ def test__compose_attributes_reset_for_stream_compose( ) -> None: mocker.patch(WRITEBOX + "._set_stream_write_box_style") write_box.stream_box_view(stream_id=1) - write_box.msg_write_box.edit_text = "random text" + + write_box.msg_write_box.edit_text = "." * ( + MAX_MESSAGE_LENGTH_CONFIRMATION_POPUP - 1 + ) size = widget_size(write_box) write_box.keypress(size, key) + write_box.view.controller.exit_compose_confirmation_popup.assert_not_called() assert write_box.stream_id is None assert write_box.msg_write_box.edit_text == "" assert write_box.compose_box_status == "closed" + @pytest.mark.parametrize("key", keys_for_command("EXIT_COMPOSE")) + def test__compose_attributes_reset_for_stream_compose__popup( + self, + key: str, + mocker: MockerFixture, + write_box: WriteBox, + widget_size: Callable[[Widget], urwid_Size], + ) -> None: + mocker.patch(WRITEBOX + "._set_stream_write_box_style") + write_box.stream_box_view(stream_id=1) + + write_box.msg_write_box.edit_text = "." * MAX_MESSAGE_LENGTH_CONFIRMATION_POPUP + + size = widget_size(write_box) + write_box.keypress(size, key) + + write_box.view.controller.exit_compose_confirmation_popup.assert_called_once_with() + @pytest.mark.parametrize( ["raw_recipients", "tidied_recipients"], [ @@ -1523,6 +1574,7 @@ def test_keypress_SEND_MESSAGE_no_topic( ) def test_keypress_typeahead_mode_autocomplete_key( self, + mocker: MockerFixture, write_box: WriteBox, widget_size: Callable[[Widget], urwid_Size], current_typeahead_mode: bool, @@ -1530,6 +1582,7 @@ def test_keypress_typeahead_mode_autocomplete_key( expect_footer_was_reset: bool, key: str, ) -> None: + write_box.msg_write_box = mocker.Mock(edit_text="") write_box.is_in_typeahead_mode = current_typeahead_mode size = widget_size(write_box) @@ -1537,9 +1590,10 @@ def test_keypress_typeahead_mode_autocomplete_key( assert write_box.is_in_typeahead_mode == expected_typeahead_mode if expect_footer_was_reset: - self.view.set_footer_text.assert_called_once_with() + # We may prefer called-once in future, but the key part is that we do reset + assert self.view.set_footer_text.called else: - self.view.set_footer_text.assert_not_called() + assert not self.view.set_footer_text.called @pytest.mark.parametrize( [ diff --git a/zulipterminal/core.py b/zulipterminal/core.py index 99d6cac23a..fb287634a4 100644 --- a/zulipterminal/core.py +++ b/zulipterminal/core.py @@ -532,6 +532,21 @@ def stream_muting_confirmation_popup( mute_this_stream = partial(self.model.toggle_stream_muted_status, stream_id) self.loop.widget = PopUpConfirmationView(self, question, mute_this_stream) + def exit_compose_confirmation_popup(self) -> None: + question = urwid.Text( + ( + "bold", + "Please confirm that you wish to exit the compose box.\n" + "(You can save the message as a draft upon returning to compose)", + ), + "center", + ) + write_box = self.view.write_box + popup_view = PopUpConfirmationView( + self, question, write_box.exit_compose_box, location="center" + ) + self.loop.widget = popup_view + def copy_to_clipboard(self, text: str, text_category: str) -> None: try: pyperclip.copy(text) diff --git a/zulipterminal/ui_tools/boxes.py b/zulipterminal/ui_tools/boxes.py index 40d9e48b81..9856685336 100644 --- a/zulipterminal/ui_tools/boxes.py +++ b/zulipterminal/ui_tools/boxes.py @@ -10,7 +10,7 @@ from typing import Any, Callable, Dict, List, NamedTuple, Optional, Tuple import urwid -from typing_extensions import Literal +from typing_extensions import Final, Literal from urwid_readline import ReadlineEdit from zulipterminal.api_types import Composition, PrivateComposition, StreamComposition @@ -49,6 +49,11 @@ from zulipterminal.urwid_types import urwid_Size +# This constant defines the maximum character length of a message +# in the compose box that does not trigger a confirmation popup. +MAX_MESSAGE_LENGTH_CONFIRMATION_POPUP: Final = 15 + + class _MessageEditState(NamedTuple): message_id: int old_topic: str @@ -710,6 +715,7 @@ def autocomplete_emojis( return emoji_typeahead, emojis def exit_compose_box(self) -> None: + self._set_default_footer_after_autocomplete() self._set_compose_attributes_to_defaults() self.view.controller.exit_editor_mode() self.main_view(False) @@ -724,6 +730,11 @@ def keypress(self, size: urwid_Size, key: str) -> Optional[str]: is_command_key("AUTOCOMPLETE", key) or is_command_key("AUTOCOMPLETE_REVERSE", key) ): + # As is, this exits autocomplete even if the user chooses to resume compose. + # Including a check for "EXIT_COMPOSE" in the above logic would avoid + # resetting the footer until actually exiting compose, but autocomplete + # itself does not continue on resume with such a solution. + # TODO: Fully implement resuming of autocomplete upon resuming compose. self._set_default_footer_after_autocomplete() if is_command_key("SEND_MESSAGE", key): @@ -807,8 +818,24 @@ def keypress(self, size: urwid_Size, key: str) -> Optional[str]: "Cannot narrow to message without specifying recipients." ) elif is_command_key("EXIT_COMPOSE", key): + saved_draft = self.model.session_draft_message() self.send_stop_typing_status() - self.exit_compose_box() + + compose_not_in_edit_mode = self.msg_edit_state is None + compose_box_content = self.msg_write_box.edit_text + saved_draft_content = saved_draft.get("content") if saved_draft else None + + exceeds_max_length = ( + len(compose_box_content) >= MAX_MESSAGE_LENGTH_CONFIRMATION_POPUP + ) + not_saved_as_draft = ( + saved_draft is None or compose_box_content != saved_draft_content + ) + + if compose_not_in_edit_mode and exceeds_max_length and not_saved_as_draft: + self.view.controller.exit_compose_confirmation_popup() + else: + self.exit_compose_box() elif is_command_key("MARKDOWN_HELP", key): self.view.controller.show_markdown_help() return key