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 diables 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 loosing 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
zee-bit committed Mar 23, 2021
1 parent d81c96f commit e32c9d1
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 66 deletions.
99 changes: 64 additions & 35 deletions tests/ui_tools/test_boxes.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -650,56 +650,84 @@ def test_keypress_typeahead_mode_autocomplete_key(self, mocker, write_box,
"box_type",
"msg_body_edit_enabled",
"message_being_edited",
"key",
"expected_focus_name",
"expected_focus_col_name"], [
('CONTAINER_HEADER', 'HEADER_BOX_STREAM', "stream", True, False,
'CONTAINER_HEADER', 'HEADER_BOX_TOPIC'),
'tab_key', 'CONTAINER_HEADER', 'HEADER_BOX_TOPIC'),
('CONTAINER_HEADER', 'HEADER_BOX_TOPIC', "stream", True, False,
'CONTAINER_MESSAGE', 'MESSAGE_BOX_BODY'),
'tab_key', 'CONTAINER_MESSAGE', 'MESSAGE_BOX_BODY'),
('CONTAINER_HEADER', 'HEADER_BOX_TOPIC', "stream", False, True,
'CONTAINER_HEADER', 'HEADER_BOX_EDIT'),
'tab_key', 'CONTAINER_HEADER', 'HEADER_BOX_EDIT'),
('CONTAINER_HEADER', 'HEADER_BOX_EDIT', "stream", False, True,
'CONTAINER_HEADER', 'HEADER_BOX_TOPIC'),
'tab_key', 'CONTAINER_HEADER', 'HEADER_BOX_TOPIC'),
('CONTAINER_MESSAGE', 'MESSAGE_BOX_BODY', "stream", True, False,
'CONTAINER_HEADER', 'HEADER_BOX_STREAM'),
'tab_key', 'CONTAINER_HEADER', 'HEADER_BOX_STREAM'),
('CONTAINER_HEADER', 'HEADER_BOX_STREAM', "stream", True, True,
'CONTAINER_HEADER', 'HEADER_BOX_TOPIC'),
'tab_key', 'CONTAINER_HEADER', 'HEADER_BOX_TOPIC'),
('CONTAINER_HEADER', 'HEADER_BOX_TOPIC', "stream", True, True,
'CONTAINER_HEADER', 'HEADER_BOX_EDIT'),
'tab_key', 'CONTAINER_HEADER', 'HEADER_BOX_EDIT'),
('CONTAINER_HEADER', 'HEADER_BOX_EDIT', "stream", True, True,
'CONTAINER_MESSAGE', 'MESSAGE_BOX_BODY'),
'tab_key', 'CONTAINER_MESSAGE', 'MESSAGE_BOX_BODY'),
('CONTAINER_MESSAGE', 'MESSAGE_BOX_BODY', "stream", True, True,
'CONTAINER_HEADER', 'HEADER_BOX_STREAM'),
'tab_key', 'CONTAINER_HEADER', 'HEADER_BOX_STREAM'),
('CONTAINER_HEADER', 'HEADER_BOX_RECIPIENT', "private", True, False,
'CONTAINER_MESSAGE', 'MESSAGE_BOX_BODY'),
'tab_key', 'CONTAINER_MESSAGE', 'MESSAGE_BOX_BODY'),
('CONTAINER_MESSAGE', 'MESSAGE_BOX_BODY', "private", True, False,
'CONTAINER_HEADER', 'HEADER_BOX_RECIPIENT'),
'tab_key', 'CONTAINER_HEADER', 'HEADER_BOX_RECIPIENT'),
('CONTAINER_HEADER', 'HEADER_BOX_STREAM', "stream", True, False,
"right", 'CONTAINER_HEADER', 'HEADER_BOX_TOPIC'),
('CONTAINER_HEADER', 'HEADER_BOX_TOPIC', "stream", True, True,
"right", 'CONTAINER_HEADER', 'HEADER_BOX_EDIT'),
('CONTAINER_HEADER', 'HEADER_BOX_EDIT', "stream", False, True,
"up", 'CONTAINER_HEADER', 'HEADER_BOX_EDIT'),
('CONTAINER_HEADER', 'HEADER_BOX_STREAM', "stream", True, True,
"left", 'CONTAINER_HEADER', 'HEADER_BOX_STREAM'),
('CONTAINER_HEADER', 'HEADER_BOX_TOPIC', "stream", True, True,
"right", 'CONTAINER_HEADER', 'HEADER_BOX_EDIT'),
('CONTAINER_HEADER', 'HEADER_BOX_TOPIC', "stream", True, False,
'down', 'CONTAINER_MESSAGE', 'MESSAGE_BOX_BODY'),
('CONTAINER_HEADER', 'HEADER_BOX_RECIPIENT', "private", True, False,
"right", 'CONTAINER_HEADER', 'HEADER_BOX_RECIPIENT'),
('CONTAINER_HEADER', 'HEADER_BOX_RECIPIENT', "private", True, False,
"down", 'CONTAINER_MESSAGE', 'MESSAGE_BOX_BODY'),
('CONTAINER_MESSAGE', 'MESSAGE_BOX_BODY', "private", True, False,
"right", 'CONTAINER_MESSAGE', 'MESSAGE_BOX_BODY'),
], ids=[
'stream_name_to_topic_box',
'topic_to_message_box',
'topic_edit_only-topic_to_edit_mode_box',
'topic_edit_only-edit_mode_to_topic_box',
'message_to_stream_name_box',
'edit_box-stream_name_to_topic_box',
'edit_box-topic_to_edit_mode_box',
'edit_box-edit_mode_to_message_box',
'edit_box-message_to_stream_name_box',
'recipient_to_message_box',
'message_to_recipient_box',
'stream_name_to_topic_box_tab',
'topic_to_message_box_tab',
'topic_edit_only-topic_to_edit_mode_box_tab',
'topic_edit_only-edit_mode_to_topic_box_tab',
'message_to_stream_name_box_tab',
'edit_box-stream_name_to_topic_box_tab',
'edit_box-topic_to_edit_mode_box_tab',
'edit_box-edit_mode_to_message_box_tab',
'edit_box-message_to_stream_name_box_tab',
'recipient_to_message_box_tab',
'message_to_recipient_box_tab',
'stream_name_to_topic_box_right',
'topic_edit_only-topic_to_edit_mode_box_right',
'topic_edit_only-edit_mode_to_edit_box_up',
'edit_box-stream_name_to_stream_box_left',
'edit_box-topic_to_edit_mode_box_right',
'topic_box_to_message_box_down',
'recipient_to_recipient_box_right',
'recipient_to_message_box_down',
'message_to_message_box_private_right',
])
@pytest.mark.parametrize("tab_key",
keys_for_command("CYCLE_COMPOSE_FOCUS"))
def test_keypress_CYCLE_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,
widget_size,
mocker, stream_id=10):
def test_keypress_CHANGE_COMPOSE_FOCUS(self, write_box,
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 @@ -709,7 +737,8 @@ def test_keypress_CYCLE_COMPOSE_FOCUS(self, write_box, tab_key,
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 @@ -721,7 +750,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
81 changes: 50 additions & 31 deletions zulipterminal/ui_tools/boxes.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
)
from zulipterminal.config.keys import (
is_command_key,
is_primary_direction_key,
keys_for_command,
primary_key_for_command,
)
Expand Down Expand Up @@ -500,6 +501,16 @@ def footer_notify_invalid_recipient(self, error_msg: str) -> None:
]
self.view.set_footer_text(footer_message, 3)

def loosing_focus_from_stream_box(self, key: str) -> bool:
stream_header = self.header_write_box[self.FOCUS_HEADER_BOX_STREAM]
stream_name_len = len(stream_header.edit_text)
cursor_pos = stream_header.edit_pos
if (is_command_key('CYCLE_COMPOSE_FOCUS', key)
or key in ['right', 'down']):
if key != 'right' or stream_name_len == cursor_pos:
return True
return False

def keypress(self, size: urwid_Size, key: str) -> Optional[str]:
if self.is_in_typeahead_mode:
if not (is_command_key('AUTOCOMPLETE', key)
Expand Down Expand Up @@ -582,7 +593,8 @@ def keypress(self, size: urwid_Size, key: str) -> Optional[str]:
self.view.controller.save_draft_confirmation_popup(
this_draft,
)
elif is_command_key('CYCLE_COMPOSE_FOCUS', key):
elif (is_command_key('CYCLE_COMPOSE_FOCUS', key)
or is_primary_direction_key(key)):
if len(self.contents) == 0:
return key
header = self.header_write_box
Expand All @@ -592,32 +604,37 @@ def keypress(self, size: urwid_Size, key: str) -> Optional[str]:
if header.focus_col == self.FOCUS_HEADER_BOX_STREAM:
stream_name = (header[self.FOCUS_HEADER_BOX_STREAM]
.edit_text)
if not self.model.is_valid_stream(stream_name):
if (not self.model.is_valid_stream(stream_name)
and self.loosing_focus_from_stream_box(key)):
self.footer_notify_invalid_recipient(
'Invalid stream name.')
return key
user_ids = self.model.get_other_subscribers_in_stream(
stream_name=stream_name)
self.recipient_user_ids = user_ids
self.stream_id = self.model.stream_id_from_name(
stream_name)
if not is_primary_direction_key(key):
user_ids = (
self.model.get_other_subscribers_in_stream(
stream_name=stream_name))
self.recipient_user_ids = user_ids
self.stream_id = self.model.stream_id_from_name(
stream_name)

header.focus_col = self.FOCUS_HEADER_BOX_TOPIC
return key
elif (header.focus_col == self.FOCUS_HEADER_BOX_TOPIC
and self.msg_edit_id):
header.focus_col = self.FOCUS_HEADER_BOX_EDIT
return key
elif header.focus_col == self.FOCUS_HEADER_BOX_EDIT:
if self.msg_body_edit_enabled:
header.focus_col = self.FOCUS_HEADER_BOX_STREAM
self.focus_position = self.FOCUS_CONTAINER_MESSAGE
else:
header.focus_col = self.FOCUS_HEADER_BOX_TOPIC
return key
else:
header.focus_col = self.FOCUS_HEADER_BOX_STREAM
else:
return key
elif not is_primary_direction_key(key):
if (header.focus_col == self.FOCUS_HEADER_BOX_TOPIC
and self.msg_edit_id):
header.focus_col = self.FOCUS_HEADER_BOX_EDIT
return key
elif header.focus_col == self.FOCUS_HEADER_BOX_EDIT:
if self.msg_body_edit_enabled:
header.focus_col = self.FOCUS_HEADER_BOX_STREAM
self.focus_position = (
self.FOCUS_CONTAINER_MESSAGE)
else:
header.focus_col = self.FOCUS_HEADER_BOX_TOPIC
return key
else:
header.focus_col = self.FOCUS_HEADER_BOX_STREAM
elif not is_primary_direction_key(key) or key == 'down':
self.update_recipient_emails(self.to_write_box)
invalid_emails = self.model.get_invalid_recipient_emails(
self.recipient_emails
Expand All @@ -632,16 +649,18 @@ def keypress(self, size: urwid_Size, key: str) -> Optional[str]:
for email in
self.recipient_emails]

if not self.msg_body_edit_enabled:
if (not self.msg_body_edit_enabled
and (not is_primary_direction_key(key) or key == 'down')):
return key
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.to_write_box is None:
header.focus_col = self.FOCUS_HEADER_BOX_STREAM
else:
header.focus_col = self.FOCUS_HEADER_BOX_RECIPIENT
if not is_primary_direction_key(key):
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.to_write_box is None:
header.focus_col = self.FOCUS_HEADER_BOX_STREAM
else:
header.focus_col = self.FOCUS_HEADER_BOX_RECIPIENT

key = super().keypress(size, key)
return key
Expand Down

0 comments on commit e32c9d1

Please sign in to comment.