Skip to content

Commit

Permalink
boxes: Disable cycling using arrow keys if recipient is invalid.
Browse files Browse the repository at this point in the history
This commit disables cycling from the recipient box using arrow keys
if recipient is invalid, similar to Tab. Cycling through the
MessageBox now looks for arrow key events and checks for recipient
name's validity if losing focus from the recipient box. This should
prevent sending of messages to incorrect recipients.

Currently, only RIGHT and DOWN keypress is monitored since these
are the only keys that can cycle focus from recipients box to
some other box, according to our current MessageBox layout.

Tests amended.
Fixes #779.
  • Loading branch information
zee-bit committed Jun 27, 2021
1 parent 571390b commit f55257f
Show file tree
Hide file tree
Showing 2 changed files with 191 additions and 49 deletions.
145 changes: 129 additions & 16 deletions tests/ui_tools/test_boxes.py
Original file line number Diff line number Diff line change
Expand Up @@ -925,6 +925,7 @@ def test_keypress_typeahead_mode_autocomplete_key(
"box_type",
"msg_body_edit_enabled",
"message_being_edited",
"key",
"expected_focus_name",
"expected_focus_col_name",
],
Expand All @@ -935,130 +936,241 @@ def test_keypress_typeahead_mode_autocomplete_key(
"stream",
True,
False,
"tab_key",
"CONTAINER_HEADER",
"HEADER_BOX_TOPIC",
id="stream_name_to_topic_box",
id="stream_name_to_topic_box_tab",
),
case(
"CONTAINER_HEADER",
"HEADER_BOX_TOPIC",
"stream",
True,
False,
"tab_key",
"CONTAINER_MESSAGE",
"MESSAGE_BOX_BODY",
id="topic_to_message_box",
id="topic_to_message_box_tab",
),
case(
"CONTAINER_HEADER",
"HEADER_BOX_TOPIC",
"stream",
False,
True,
"tab_key",
"CONTAINER_HEADER",
"HEADER_BOX_EDIT",
id="topic_edit_only-topic_to_edit_mode_box",
id="topic_edit_only-topic_to_edit_mode_box_tab",
),
case(
"CONTAINER_HEADER",
"HEADER_BOX_EDIT",
"stream",
False,
True,
"tab_key",
"CONTAINER_HEADER",
"HEADER_BOX_TOPIC",
id="topic_edit_only-edit_mode_to_topic_box",
id="topic_edit_only-edit_mode_to_topic_box_tab",
),
case(
"CONTAINER_MESSAGE",
"MESSAGE_BOX_BODY",
"stream",
True,
False,
"tab_key",
"CONTAINER_HEADER",
"HEADER_BOX_STREAM",
id="message_to_stream_name_box",
id="message_to_stream_name_box_tab",
),
case(
"CONTAINER_HEADER",
"HEADER_BOX_STREAM",
"stream",
True,
True,
"tab_key",
"CONTAINER_HEADER",
"HEADER_BOX_TOPIC",
id="edit_box-stream_name_to_topic_box",
id="edit_box-stream_name_to_topic_box_tab",
),
case(
"CONTAINER_HEADER",
"HEADER_BOX_TOPIC",
"stream",
True,
True,
"tab_key",
"CONTAINER_HEADER",
"HEADER_BOX_EDIT",
id="edit_box-topic_to_edit_mode_box",
id="edit_box-topic_to_edit_mode_box_tab",
),
case(
"CONTAINER_HEADER",
"HEADER_BOX_EDIT",
"stream",
True,
True,
"tab_key",
"CONTAINER_MESSAGE",
"MESSAGE_BOX_BODY",
id="edit_box-edit_mode_to_message_box",
id="edit_box-edit_mode_to_message_box_tab",
),
case(
"CONTAINER_MESSAGE",
"MESSAGE_BOX_BODY",
"stream",
True,
True,
"tab_key",
"CONTAINER_HEADER",
"HEADER_BOX_STREAM",
id="edit_box-message_to_stream_name_box",
id="edit_box-message_to_stream_name_box_tab",
),
case(
"CONTAINER_HEADER",
"HEADER_BOX_RECIPIENT",
"private",
True,
False,
"tab_key",
"CONTAINER_MESSAGE",
"MESSAGE_BOX_BODY",
id="recipient_to_message_box",
id="recipient_to_message_box_tab",
),
case(
"CONTAINER_MESSAGE",
"MESSAGE_BOX_BODY",
"private",
True,
False,
"tab_key",
"CONTAINER_HEADER",
"HEADER_BOX_RECIPIENT",
id="message_to_recipient_box",
id="message_to_recipient_box_tab",
),
case(
"CONTAINER_HEADER",
"HEADER_BOX_STREAM",
"stream",
True,
False,
"right",
"CONTAINER_HEADER",
"HEADER_BOX_TOPIC",
id="stream_name_to_topic_box_right",
),
case(
"CONTAINER_HEADER",
"HEADER_BOX_TOPIC",
"stream",
True,
True,
"right",
"CONTAINER_HEADER",
"HEADER_BOX_EDIT",
id="topic_edit_only-topic_to_edit_mode_box_right",
),
case(
"CONTAINER_HEADER",
"HEADER_BOX_EDIT",
"stream",
False,
True,
"up",
"CONTAINER_HEADER",
"HEADER_BOX_EDIT",
id="topic_edit_only-edit_mode_to_edit_box_up",
),
case(
"CONTAINER_HEADER",
"HEADER_BOX_STREAM",
"stream",
True,
True,
"left",
"CONTAINER_HEADER",
"HEADER_BOX_STREAM",
id="edit_box-stream_name_to_stream_box_left",
),
case(
"CONTAINER_HEADER",
"HEADER_BOX_TOPIC",
"stream",
True,
True,
"right",
"CONTAINER_HEADER",
"HEADER_BOX_EDIT",
id="edit_box-topic_to_edit_mode_box_right",
),
case(
"CONTAINER_HEADER",
"HEADER_BOX_TOPIC",
"stream",
True,
False,
"down",
"CONTAINER_MESSAGE",
"MESSAGE_BOX_BODY",
id="topic_box_to_message_box_down",
),
case(
"CONTAINER_HEADER",
"HEADER_BOX_RECIPIENT",
"private",
True,
False,
"right",
"CONTAINER_HEADER",
"HEADER_BOX_RECIPIENT",
id="recipient_to_recipient_box_right",
),
case(
"CONTAINER_HEADER",
"HEADER_BOX_RECIPIENT",
"private",
True,
False,
"down",
"CONTAINER_MESSAGE",
"MESSAGE_BOX_BODY",
id="recipient_to_message_box_down",
),
case(
"CONTAINER_MESSAGE",
"MESSAGE_BOX_BODY",
"private",
True,
False,
"right",
"CONTAINER_MESSAGE",
"MESSAGE_BOX_BODY",
id="message_to_message_box_private_right",
),
],
)
@pytest.mark.parametrize("tab_key", keys_for_command("CYCLE_COMPOSE_FOCUS"))
def test_keypress_CYCLE_COMPOSE_FOCUS(
def test_keypress_CHANGE_COMPOSE_FOCUS(
self,
write_box,
tab_key,
initial_focus_name,
expected_focus_name,
initial_focus_col_name,
expected_focus_col_name,
box_type,
msg_body_edit_enabled,
message_being_edited,
key,
widget_size,
mocker,
stream_id=10,
):
mocker.patch(BOXES + ".WriteBox._set_stream_write_box_style")

if key == "tab_key":
key = primary_key_for_command("CYCLE_COMPOSE_FOCUS")
if box_type == "stream":
if message_being_edited:
mocker.patch(BOXES + ".EditModeButton")
Expand All @@ -1068,7 +1180,8 @@ def test_keypress_CYCLE_COMPOSE_FOCUS(
write_box.stream_box_view(stream_id)
else:
write_box.private_box_view()
size = widget_size(write_box)
# FIXME: Use widget_size(), instead of hard-coded size.
size = (20,)

def focus_val(x: str) -> int:
return getattr(write_box, "FOCUS_" + x)
Expand All @@ -1080,7 +1193,7 @@ def focus_val(x: str) -> int:
write_box.model.get_invalid_recipient_emails.return_value = []
write_box.model.user_dict = mocker.MagicMock()

write_box.keypress(size, tab_key)
write_box.keypress(size, key)

assert write_box.focus_position == focus_val(expected_focus_name)
# FIXME: Needs refactoring?
Expand Down
Loading

0 comments on commit f55257f

Please sign in to comment.