Repository: zulip/zulip-terminal (python)

Issue:
Improve handling of pressing `Esc` during message compose
Currently `Esc` can be pressed to exit compose, without sending the message.

This can be useful if one wants to cancel the send, but can also be frustrating if accidentally hitting the key after having written a good message which you intend to send.

This can be made worse, if a key is near the `Esc` key and used for composition, eg. the <kbd>`</kbd> key for inline code or quote/code blocks.

Ideas for resolving this:
- Prompt with a confirmation popup to exit compose, if there is more than some threshold of content present (could be empty message, could be 5, 10, 15, ... characters)
- Prompt with a variation on a confirmation popup, giving the option to save as a draft
- Automatically save as a draft, but prompt if a draft already exists (like we do now)
- Automatically save as a draft - *only possible if we support more than one draft in the session, ie. improve draft handling generally ~ #302)

Please discuss this in the stream before proceeding with this, since it would be good to get more views on this first.

Pull request:
Improve handling Esc during message compose
<!-- See README for documentation, or ask in #zulip-terminal if unclear -->
### What does this PR do, and why?
Uses the existing confirmation popup to confirm whether user wants to exit the compose box upon pressing `Esc`, Enabling the user to go back and save their draft from compose.


### Outstanding aspect(s)    <!-- DELETE SECTION IF EMPTY -->
<!-- In what ways is this not fully implemented/functioning? Compared to a discussion/issue? -->
<!-- Do you not understand something? Are you unsure about a certain approach? Want feedback? -->
- [ ] 

### External discussion & connections
<!-- [x] all that apply, specifying topic and adding numbers after # for issues/PRs -->
- [x] Discussed in **#zulip-terminal** in `Improve handling Esc during msg compose #T1342 #T1362 #T1442`
- [x] Fully fixes #1342 
- [ ] Partially fixes issue #
- [x] Builds upon previous unmerged work in PR #1362 
- [ ] Is a follow-up to work in PR #
- [ ] Requires merge of PR #
- [ ] Merge will enable work on #

### How did you test this?
<!-- [x] all that apply -->
- [x] Manually - Behavioral changes
- [x] Manually - Visual changes
- [ ] Adapting existing automated tests
- [ ] Adding automated tests for new behavior (or missing tests)
- [ ] Existing automated tests should already cover this (*only a refactor of tested code*)

### Self-review checklist for each commit
- [x] It is a [minimal coherent idea](https://github.com/zulip/zulip-terminal#structuring-commits---speeding-up-reviews-merging--development)
- [ ] It has a commit summary following the [documented style](https://github.com/zulip/zulip-terminal#structuring-commits---speeding-up-reviews-merging--development) (title & body)
- [ ] It has a commit summary describing the  motivation and reasoning for the change
- [ ] It individually passes linting and tests
- [ ] It contains test additions for any new behavior
- [ ] It flows clearly from a previous branch commit, and/or prepares for the next commit

### Visual changes    <!-- DELETE SECTION IF NO VISUAL CHANGE -->
<!-- Zulip tips at https://zulip.readthedocs.io/en/latest/tutorials/screenshot-and-gif-software.html -->
<!-- For video, try asciinema; after uploading, embed using
[![yourtitle](https://asciinema.org/a/<id>.png)](https://asciinema.org/a/<id>)
-->
<!-- NOTE: Attached videos/images will be clearer from smaller terminal windows -->


Patch:
diff --git a/zulipterminal/core.py b/zulipterminal/core.py
index 99d6cac..eec6282 100644
--- a/zulipterminal/core.py
+++ b/zulipterminal/core.py
@@ -515,10 +515,17 @@ class Controller:
 
     def save_draft_confirmation_popup(self, draft: Composition) -> None:
         question = urwid.Text(
-            "Save this message as a draft? (This will overwrite the existing draft.)"
+            (
+                "bold",
+                "Save this message as a draft? "
+                "(This will overwrite the existing draft.)",
+            ),
+            "center",
         )
         save_draft = partial(self.model.save_draft, draft)
-        self.loop.widget = PopUpConfirmationView(self, question, save_draft)
+        self.loop.widget = PopUpConfirmationView(
+            self, question, save_draft, location="center"
+        )
 
     def stream_muting_confirmation_popup(
         self, stream_id: int, stream_name: str
@@ -532,6 +539,21 @@ class Controller:
         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 fa3dc1f..9856685 100644
--- a/zulipterminal/ui_tools/boxes.py
+++ b/zulipterminal/ui_tools/boxes.py
@@ -10,7 +10,7 @@ from time import sleep
 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.ui_tools.buttons import EditModeButton
 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
@@ -709,14 +714,28 @@ class WriteBox(urwid.Pile):
 
         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)
+        self.view.middle_column.set_focus("body")
+
+    def _set_default_footer_after_autocomplete(self) -> None:
+        self.is_in_typeahead_mode = False
+        self.view.set_footer_text()
+
     def keypress(self, size: urwid_Size, key: str) -> Optional[str]:
         if self.is_in_typeahead_mode and not (
             is_command_key("AUTOCOMPLETE", key)
             or is_command_key("AUTOCOMPLETE_REVERSE", key)
         ):
-            # set default footer when done with autocomplete
-            self.is_in_typeahead_mode = False
-            self.view.set_footer_text()
+            # 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):
             self.send_stop_typing_status()
@@ -799,11 +818,24 @@ class WriteBox(urwid.Pile):
                         "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._set_compose_attributes_to_defaults()
-            self.view.controller.exit_editor_mode()
-            self.main_view(False)
-            self.view.middle_column.set_focus("body")
+
+            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


Test patch:
diff --git a/tests/ui_tools/test_boxes.py b/tests/ui_tools/test_boxes.py
index 4b28a1e..c4e89a2 100644
--- a/tests/ui_tools/test_boxes.py
+++ b/tests/ui_tools/test_boxes.py
@@ -25,7 +25,12 @@ from zulipterminal.config.symbols import (
 )
 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 @@ class TestWriteBox:
         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 @@ class TestWriteBox:
         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 @@ class TestWriteBox:
     ) -> 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"],
         [
@@ -1508,38 +1559,59 @@ class TestWriteBox:
             )
 
     @pytest.mark.parametrize(
-        "key, current_typeahead_mode, expected_typeahead_mode, expect_footer_was_reset",
+        "key, current_typeahead_mode, expected_typeahead_mode",
         [
-            # footer does not reset
-            (primary_key_for_command("AUTOCOMPLETE"), False, False, False),
-            (primary_key_for_command("AUTOCOMPLETE_REVERSE"), False, False, False),
-            (primary_key_for_command("AUTOCOMPLETE"), True, True, False),
-            (primary_key_for_command("AUTOCOMPLETE_REVERSE"), True, True, False),
-            # footer resets
-            (primary_key_for_command("EXIT_COMPOSE"), True, False, True),
-            ("space", True, False, True),
-            ("k", True, False, True),
+            (primary_key_for_command("AUTOCOMPLETE"), False, False),
+            (primary_key_for_command("AUTOCOMPLETE_REVERSE"), False, False),
+            (primary_key_for_command("AUTOCOMPLETE"), True, True),
+            (primary_key_for_command("AUTOCOMPLETE_REVERSE"), True, True),
         ],
     )
-    def test_keypress_typeahead_mode_autocomplete_key(
+    def test__keypress_typeahead_mode_autocomplete_key_footer_no_reset(
         self,
+        mocker: MockerFixture,
         write_box: WriteBox,
         widget_size: Callable[[Widget], urwid_Size],
         current_typeahead_mode: bool,
         expected_typeahead_mode: bool,
-        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)
 
         write_box.keypress(size, 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()
-        else:
-            self.view.set_footer_text.assert_not_called()
+        assert not self.view.set_footer_text.called
+
+    @pytest.mark.parametrize(
+        "key, current_typeahead_mode, expected_typeahead_mode",
+        [
+            (primary_key_for_command("EXIT_COMPOSE"), True, False),
+            ("space", True, False),
+            ("k", True, False),
+        ],
+    )
+    def test__keypress_typeahead_mode_autocomplete_key_footer_reset(
+        self,
+        mocker: MockerFixture,
+        write_box: WriteBox,
+        widget_size: Callable[[Widget], urwid_Size],
+        current_typeahead_mode: bool,
+        expected_typeahead_mode: 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)
+
+        write_box.keypress(size, key)
+
+        assert write_box.is_in_typeahead_mode == expected_typeahead_mode
+
+        # We may prefer called-once in future, but the key part is that we do reset
+        assert self.view.set_footer_text.called
 
     @pytest.mark.parametrize(
         [