Skip to content

Break down the generalized ENTER command #1497

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion docs/hotkeys.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@
|Scroll up|<kbd>PgUp</kbd> / <kbd>K</kbd>|
|Scroll down|<kbd>PgDn</kbd> / <kbd>J</kbd>|
|Go to bottom / Last message|<kbd>End</kbd> / <kbd>G</kbd>|
|Trigger the selected entry|<kbd>Enter</kbd> / <kbd>Space</kbd>|
|Narrow to all messages|<kbd>a</kbd> / <kbd>Esc</kbd>|
|Narrow to all direct messages|<kbd>P</kbd>|
|Narrow to all starred messages|<kbd>f</kbd>|
|Narrow to messages in which you're mentioned|<kbd>#</kbd>|
|Next unread topic|<kbd>n</kbd>|
|Next unread direct message|<kbd>p</kbd>|
|Perform current action|<kbd>Enter</kbd>|

## Searching
|Command|Key Combination|
Expand All @@ -40,6 +40,7 @@
|Search streams|<kbd>q</kbd>|
|Search topics in a stream|<kbd>q</kbd>|
|Search emojis from emoji picker|<kbd>p</kbd>|
|Submit search and browse results|<kbd>Enter</kbd>|

## Message actions
|Command|Key Combination|
Expand Down Expand Up @@ -83,6 +84,7 @@
|Autocomplete @mentions, #stream_names, :emoji: and topics|<kbd>Ctrl</kbd> + <kbd>f</kbd>|
|Cycle through autocomplete suggestions in reverse|<kbd>Ctrl</kbd> + <kbd>r</kbd>|
|Narrow to compose box message recipient|<kbd>Meta</kbd> + <kbd>.</kbd>|
|Insert new line|<kbd>Enter</kbd>|

## Editor: Navigation
|Command|Key Combination|
Expand Down
2 changes: 1 addition & 1 deletion tests/ui_tools/test_boxes.py
Original file line number Diff line number Diff line change
Expand Up @@ -1819,7 +1819,7 @@ def test_valid_char(
@pytest.mark.parametrize(
"log, expect_body_focus_set", [([], False), (["SOMETHING"], True)]
)
@pytest.mark.parametrize("enter_key", keys_for_command("ENTER"))
@pytest.mark.parametrize("enter_key", keys_for_command("EXECUTE_SEARCH"))
def test_keypress_ENTER(
self,
panel_search_box: PanelSearchBox,
Expand Down
4 changes: 2 additions & 2 deletions tests/ui_tools/test_buttons.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ def test_keypress_TOGGLE_MUTE_STREAM(

class TestUserButton:
# FIXME Place this in a general test of a derived class?
@pytest.mark.parametrize("enter_key", keys_for_command("ENTER"))
@pytest.mark.parametrize("enter_key", keys_for_command("ACTIVATE_BUTTON"))
def test_activate_called_once_on_keypress(
self,
mocker: MockerFixture,
Expand Down Expand Up @@ -358,7 +358,7 @@ def test_init_calls_top_button(
assert emoji_button.emoji_name == emoji_unit[0]
assert emoji_button.reaction_count == count

@pytest.mark.parametrize("key", keys_for_command("ENTER"))
@pytest.mark.parametrize("key", keys_for_command("ACTIVATE_BUTTON"))
@pytest.mark.parametrize(
"emoji, has_user_reacted, is_selected_final, expected_reaction_count",
[
Expand Down
9 changes: 6 additions & 3 deletions tests/ui_tools/test_messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from pytest import param as case
from urwid import Columns, Divider, Padding, Text

from zulipterminal.config.keys import keys_for_command
from zulipterminal.config.keys import keys_for_command, primary_key_for_command
from zulipterminal.config.symbols import (
ALL_MESSAGES_MARKER,
DIRECT_MESSAGE_MARKER,
Expand Down Expand Up @@ -1961,11 +1961,14 @@ def test_footlinks_limit(self, maximum_footlinks, expected_instance):
assert isinstance(footlinks, expected_instance)

@pytest.mark.parametrize(
"key", keys_for_command("ENTER"), ids=lambda param: f"left_click-key:{param}"
"key",
keys_for_command("ACTIVATE_BUTTON"),
ids=lambda param: f"left_click-key:{param}",
)
def test_mouse_event_left_click(
self, mocker, msg_box, key, widget_size, compose_box_is_open
):
expected_keypress = primary_key_for_command("ACTIVATE_BUTTON")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed to edit this test as mouse_event always calls the primary_key_for_command("ACTIVATE_BUTTON") and does not use the passed key.

size = widget_size(msg_box)
col = 1
row = 1
Expand All @@ -1979,4 +1982,4 @@ def test_mouse_event_left_click(
if compose_box_is_open:
msg_box.keypress.assert_not_called()
else:
msg_box.keypress.assert_called_once_with(size, key)
msg_box.keypress.assert_called_once_with(size, expected_keypress)
8 changes: 4 additions & 4 deletions tests/ui_tools/test_popups.py
Original file line number Diff line number Diff line change
Expand Up @@ -830,7 +830,7 @@ def test_init(self, edit_mode_view: EditModeView) -> None:
(2, "change_all"),
],
)
@pytest.mark.parametrize("key", keys_for_command("ENTER"))
@pytest.mark.parametrize("key", keys_for_command("ACTIVATE_BUTTON"))
def test_select_edit_mode(
self,
edit_mode_view: EditModeView,
Expand Down Expand Up @@ -1523,7 +1523,7 @@ def test_keypress_exit_popup(
self.stream_info_view.keypress(size, key)
assert self.controller.exit_popup.called

@pytest.mark.parametrize("key", (*keys_for_command("ENTER"), " "))
@pytest.mark.parametrize("key", (*keys_for_command("ACTIVATE_BUTTON"), " "))
def test_checkbox_toggle_mute_stream(
self, key: str, widget_size: Callable[[Widget], urwid_Size]
) -> None:
Expand All @@ -1536,7 +1536,7 @@ def test_checkbox_toggle_mute_stream(

toggle_mute_status.assert_called_once_with(stream_id)

@pytest.mark.parametrize("key", (*keys_for_command("ENTER"), " "))
@pytest.mark.parametrize("key", (*keys_for_command("ACTIVATE_BUTTON"), " "))
def test_checkbox_toggle_pin_stream(
self, key: str, widget_size: Callable[[Widget], urwid_Size]
) -> None:
Expand All @@ -1549,7 +1549,7 @@ def test_checkbox_toggle_pin_stream(

toggle_pin_status.assert_called_once_with(stream_id)

@pytest.mark.parametrize("key", (*keys_for_command("ENTER"), " "))
@pytest.mark.parametrize("key", (*keys_for_command("ACTIVATE_BUTTON"), " "))
def test_checkbox_toggle_visual_notification(
self, key: str, widget_size: Callable[[Widget], urwid_Size]
) -> None:
Expand Down
28 changes: 23 additions & 5 deletions zulipterminal/config/keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from typing_extensions import NotRequired, TypedDict
from urwid.command_map import (
ACTIVATE,
CURSOR_DOWN,
CURSOR_LEFT,
CURSOR_MAX_RIGHT,
Expand Down Expand Up @@ -91,6 +92,11 @@ class KeyBinding(TypedDict):
'help_text': 'Go to bottom / Last message',
'key_category': 'navigation',
},
'ACTIVATE_BUTTON': {
'keys': ['enter', ' '],
'help_text': 'Trigger the selected entry',
'key_category': 'navigation',
},
'REPLY_MESSAGE': {
'keys': ['r', 'enter'],
'help_text': 'Reply to the current message',
Expand Down Expand Up @@ -244,16 +250,16 @@ class KeyBinding(TypedDict):
'excluded_from_random_tips': True,
'key_category': 'searching',
},
'EXECUTE_SEARCH': {
'keys': ['enter'],
'help_text': 'Submit search and browse results',
'key_category': 'searching',
},
'TOGGLE_MUTE_STREAM': {
'keys': ['m'],
'help_text': 'Mute/unmute streams',
'key_category': 'stream_list',
},
'ENTER': {
'keys': ['enter'],
'help_text': 'Perform current action',
'key_category': 'navigation',
},
'THUMBS_UP': {
'keys': ['+'],
'help_text': 'Toggle thumbs-up reaction to the current message',
Expand Down Expand Up @@ -400,6 +406,14 @@ class KeyBinding(TypedDict):
'help_text': 'Swap with previous character',
'key_category': 'editor_text_manipulation',
},
'NEW_LINE': {
# urwid_readline's command
# This obvious hotkey is added to clarify against 'enter' to send
# and to differentiate from other hotkeys using 'enter'.
'keys': ['enter'],
'help_text': 'Insert new line',
'key_category': 'msg_compose',
},
'FULL_RENDERED_MESSAGE': {
'keys': ['f'],
'help_text': 'Show/hide full rendered message (from message information)',
Expand Down Expand Up @@ -432,6 +446,7 @@ class KeyBinding(TypedDict):
"SCROLL_UP": CURSOR_PAGE_UP,
"SCROLL_DOWN": CURSOR_PAGE_DOWN,
"GO_TO_BOTTOM": CURSOR_MAX_RIGHT,
"ACTIVATE_BUTTON": ACTIVATE,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is strictly necessary, since the urwid default map has the values we're setting already, but it's fine to include and a good pointer that we may need to return to it in future.

}


Expand Down Expand Up @@ -477,6 +492,9 @@ def display_key_for_urwid_key(urwid_key: str) -> str:
"""
Returns a displayable user-centric format of the urwid key.
"""
if urwid_key == " ":
return "Space"

for urwid_map_key, display_map_key in URWID_KEY_TO_DISPLAY_KEY_MAPPING.items():
if urwid_map_key in urwid_key:
urwid_key = urwid_key.replace(urwid_map_key, display_map_key)
Expand Down
8 changes: 4 additions & 4 deletions zulipterminal/ui_tools/boxes.py
Original file line number Diff line number Diff line change
Expand Up @@ -947,14 +947,14 @@ def main_view(self) -> Any:

def keypress(self, size: urwid_Size, key: str) -> Optional[str]:
if (
is_command_key("ENTER", key) and self.text_box.edit_text == ""
is_command_key("EXECUTE_SEARCH", key) and self.text_box.edit_text == ""
) or is_command_key("GO_BACK", key):
self.text_box.set_edit_text("")
self.controller.exit_editor_mode()
self.controller.view.middle_column.set_focus("body")
return key

elif is_command_key("ENTER", key):
elif is_command_key("EXECUTE_SEARCH", key):
self.controller.exit_editor_mode()
self.controller.search_messages(self.text_box.edit_text)
self.controller.view.middle_column.set_focus("body")
Expand Down Expand Up @@ -1003,15 +1003,15 @@ def valid_char(self, ch: str) -> bool:

def keypress(self, size: urwid_Size, key: str) -> Optional[str]:
if (
is_command_key("ENTER", key) and self.get_edit_text() == ""
is_command_key("EXECUTE_SEARCH", key) and self.get_edit_text() == ""
) or is_command_key("GO_BACK", key):
self.panel_view.view.controller.exit_editor_mode()
self.reset_search_text()
self.panel_view.set_focus("body")
# Don't call 'Esc' when inside a popup search-box.
if not self.panel_view.view.controller.is_any_popup_open():
self.panel_view.keypress(size, primary_key_for_command("GO_BACK"))
elif is_command_key("ENTER", key) and not self.panel_view.empty_search:
elif is_command_key("EXECUTE_SEARCH", key) and not self.panel_view.empty_search:
self.panel_view.view.controller.exit_editor_mode()
self.set_caption([("filter_results", " Search Results "), " "])
self.panel_view.set_focus("body")
Expand Down
4 changes: 2 additions & 2 deletions zulipterminal/ui_tools/buttons.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ def activate(self, key: Any) -> None:
self.show_function()

def keypress(self, size: urwid_Size, key: str) -> Optional[str]:
if is_command_key("ENTER", key):
if is_command_key("ACTIVATE_BUTTON", key):
self.activate(key)
return None
else: # This is in the else clause, to avoid multiple activation
Expand Down Expand Up @@ -417,7 +417,7 @@ def mouse_event(
self, size: urwid_Size, event: str, button: int, col: int, row: int, focus: int
) -> bool:
if event == "mouse press" and button == 1:
self.keypress(size, primary_key_for_command("ENTER"))
self.keypress(size, primary_key_for_command("ACTIVATE_BUTTON"))
return True
return super().mouse_event(size, event, button, col, row, focus)

Expand Down
2 changes: 1 addition & 1 deletion zulipterminal/ui_tools/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -898,7 +898,7 @@ def mouse_event(
if event == "mouse press" and button == 1:
if self.model.controller.is_in_editor_mode():
return True
self.keypress(size, primary_key_for_command("ENTER"))
self.keypress(size, primary_key_for_command("ACTIVATE_BUTTON"))
return True

return super().mouse_event(size, event, button, col, row, focus)
Expand Down
6 changes: 5 additions & 1 deletion zulipterminal/ui_tools/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1740,7 +1740,11 @@ def __init__(self, controller: Any, button: Any) -> None:
for mode in EDIT_MODE_CAPTIONS:
self.add_radio_button(mode)
super().__init__(
controller, self.widgets, "ENTER", 62, "Topic edit propagation mode"
controller,
self.widgets,
"ACTIVATE_BUTTON",
62,
"Topic edit propagation mode",
)
# Set cursor to marked checkbox.
for i in range(len(self.widgets)):
Expand Down
Loading