Skip to content

Commit

Permalink
core/boxes: Improve handling of pressing Esc during message compose.
Browse files Browse the repository at this point in the history
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 zulip#1342.
  • Loading branch information
rsashank authored and neiljp committed Jul 14, 2024
1 parent f1e6c6d commit 11f8d9f
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 9 deletions.
68 changes: 61 additions & 7 deletions tests/ui_tools/test_boxes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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"],
[
Expand Down Expand Up @@ -1523,23 +1574,26 @@ 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,
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()
# 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(
[
Expand Down
15 changes: 15 additions & 0 deletions zulipterminal/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
31 changes: 29 additions & 2 deletions zulipterminal/ui_tools/boxes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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):
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 11f8d9f

Please sign in to comment.