Skip to content

Break down the GO_BACK command into separate commands #1514

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.

Sign up for GitHub

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 3 commits into from
Jun 28, 2024
Merged

Break down the GO_BACK command into separate commands #1514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6823e5c
keys/boxes/views: Extract CLEAR_SEARCH from GO_BACK command.
Niloth-p Jun 6, 2024
03cb3d3
keys/model/views/lint-hotkeys: Extract EXIT_POPUP from GO_BACK command.
Niloth-p Jun 6, 2024
97e8e20
keys/boxes: Convert remaining GO_BACK command use into EXIT_COMPOSE.
Niloth-p Jun 6, 2024
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
|Show/hide Help Menu|<kbd>?</kbd>|
|Show/hide Markdown Help Menu|<kbd>Meta</kbd> + <kbd>m</kbd>|
|Show/hide About Menu|<kbd>Meta</kbd> + <kbd>?</kbd>|
|Go back|<kbd>Esc</kbd>|
|Open draft message saved in this session|<kbd>d</kbd>|
|Copy information from About Menu to clipboard|<kbd>c</kbd>|
|Redraw screen|<kbd>Ctrl</kbd> + <kbd>l</kbd>|
Expand All @@ -18,6 +17,7 @@
## Navigation
|Command|Key Combination|
| :--- | :---: |
|Close popup|<kbd>Esc</kbd>|
|Go up / Previous message|<kbd>Up</kbd> / <kbd>k</kbd>|
|Go down / Next message|<kbd>Down</kbd> / <kbd>j</kbd>|
|Go left|<kbd>Left</kbd> / <kbd>h</kbd>|
Expand All @@ -42,6 +42,7 @@
|Search topics in a stream|<kbd>q</kbd>|
|Search emojis from emoji picker|<kbd>p</kbd>|
|Submit search and browse results|<kbd>Enter</kbd>|
|Clear search in current panel|<kbd>Esc</kbd>|

## Message actions
|Command|Key Combination|
Expand Down Expand Up @@ -85,6 +86,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>|
|Exit message compose box|<kbd>Esc</kbd>|
|Insert new line|<kbd>Enter</kbd>|

## Editor: Navigation
Expand Down
12 changes: 6 additions & 6 deletions tests/ui/test_ui_tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -559,8 +559,8 @@ def test_keypress_SEARCH_STREAMS(self, mocker, stream_view, key, widget_size):
stream_view.stream_search_box
)

@pytest.mark.parametrize("key", keys_for_command("GO_BACK"))
def test_keypress_GO_BACK(self, mocker, stream_view, key, widget_size):
@pytest.mark.parametrize("key", keys_for_command("CLEAR_SEARCH"))
def test_keypress_CLEAR_SEARCH(self, mocker, stream_view, key, widget_size):
size = widget_size(stream_view)
mocker.patch.object(stream_view, "set_focus")
mocker.patch(VIEWS + ".urwid.Frame.keypress")
Expand Down Expand Up @@ -725,8 +725,8 @@ def test_keypress_SEARCH_TOPICS(self, mocker, topic_view, key, widget_size):
topic_view.topic_search_box
)

@pytest.mark.parametrize("key", keys_for_command("GO_BACK"))
def test_keypress_GO_BACK(self, mocker, topic_view, key, widget_size):
@pytest.mark.parametrize("key", keys_for_command("CLEAR_SEARCH"))
def test_keypress_CLEAR_SEARCH(self, mocker, topic_view, key, widget_size):
size = widget_size(topic_view)
mocker.patch(VIEWS + ".TopicsView.set_focus")
mocker.patch(VIEWS + ".urwid.Frame.keypress")
Expand Down Expand Up @@ -1105,8 +1105,8 @@ def test_keypress_SEARCH_PEOPLE(self, right_col_view, mocker, key, widget_size):
right_col_view.user_search
)

@pytest.mark.parametrize("key", keys_for_command("GO_BACK"))
def test_keypress_GO_BACK(self, right_col_view, mocker, key, widget_size):
@pytest.mark.parametrize("key", keys_for_command("CLEAR_SEARCH"))
def test_keypress_CLEAR_SEARCH(self, right_col_view, mocker, key, widget_size):
size = widget_size(right_col_view)
mocker.patch(VIEWS + ".UsersView")
mocker.patch(VIEWS + ".RightColumnView.set_focus")
Expand Down
10 changes: 5 additions & 5 deletions tests/ui_tools/test_boxes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,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("GO_BACK"))
@pytest.mark.parametrize("key", keys_for_command("EXIT_COMPOSE"))
def test__compose_attributes_reset_for_private_compose(
self,
key: str,
Expand All @@ -256,7 +256,7 @@ def test__compose_attributes_reset_for_private_compose(
assert write_box.msg_write_box.edit_text == ""
assert write_box.compose_box_status == "closed"

@pytest.mark.parametrize("key", keys_for_command("GO_BACK"))
@pytest.mark.parametrize("key", keys_for_command("EXIT_COMPOSE"))
def test__compose_attributes_reset_for_stream_compose(
self,
key: str,
Expand Down Expand Up @@ -1516,7 +1516,7 @@ def test_keypress_SEND_MESSAGE_no_topic(
(primary_key_for_command("AUTOCOMPLETE"), True, True, False),
(primary_key_for_command("AUTOCOMPLETE_REVERSE"), True, True, False),
# footer resets
(primary_key_for_command("GO_BACK"), True, False, True),
(primary_key_for_command("EXIT_COMPOSE"), True, False, True),
("space", True, False, True),
("k", True, False, True),
],
Expand Down Expand Up @@ -1858,8 +1858,8 @@ def test_keypress_ENTER(
panel_view.set_focus.assert_not_called()
panel_view.body.set_focus.assert_not_called()

@pytest.mark.parametrize("back_key", keys_for_command("GO_BACK"))
def test_keypress_GO_BACK(
@pytest.mark.parametrize("back_key", keys_for_command("CLEAR_SEARCH"))
def test_keypress_CLEAR_SEARCH(
self,
panel_search_box: PanelSearchBox,
back_key: str,
Expand Down
30 changes: 15 additions & 15 deletions tests/ui_tools/test_popups.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ def test_exit_popup_no(
self.callback.assert_not_called()
assert self.controller.exit_popup.called

@pytest.mark.parametrize("key", keys_for_command("GO_BACK"))
def test_exit_popup_GO_BACK(
@pytest.mark.parametrize("key", keys_for_command("EXIT_POPUP"))
def test_exit_popup_EXIT_POPUP(
self,
popup_view: PopUpConfirmationView,
key: str,
Expand Down Expand Up @@ -133,8 +133,8 @@ def test_init(self, mocker: MockerFixture) -> None:
self.pop_up_view.body, header=mocker.ANY, footer=mocker.ANY
)

@pytest.mark.parametrize("key", keys_for_command("GO_BACK"))
def test_keypress_GO_BACK(
@pytest.mark.parametrize("key", keys_for_command("EXIT_POPUP"))
def test_keypress_EXIT_POPUP(
self,
key: str,
widget_size: Callable[[Widget], urwid_Size],
Expand Down Expand Up @@ -210,7 +210,7 @@ def mock_external_classes(self, mocker: MockerFixture) -> None:
)

@pytest.mark.parametrize(
"key", {*keys_for_command("GO_BACK"), *keys_for_command("ABOUT")}
"key", {*keys_for_command("EXIT_POPUP"), *keys_for_command("ABOUT")}
)
def test_keypress_exit_popup(
self, key: str, widget_size: Callable[[Widget], urwid_Size]
Expand Down Expand Up @@ -467,7 +467,7 @@ def test__fetch_user_data_USER_NOT_FOUND(self, mocker: MockerFixture) -> None:
assert custom_profile_data == {}

@pytest.mark.parametrize(
"key", {*keys_for_command("GO_BACK"), *keys_for_command("USER_INFO")}
"key", {*keys_for_command("EXIT_POPUP"), *keys_for_command("USER_INFO")}
)
def test_keypress_exit_popup(
self, key: str, widget_size: Callable[[Widget], urwid_Size]
Expand Down Expand Up @@ -540,7 +540,7 @@ def test_keypress_exit_popup_invalid_key(
"key",
{
*keys_for_command("FULL_RENDERED_MESSAGE"),
*keys_for_command("GO_BACK"),
*keys_for_command("EXIT_POPUP"),
},
)
def test_keypress_show_msg_info(
Expand Down Expand Up @@ -616,7 +616,7 @@ def test_keypress_exit_popup_invalid_key(
"key",
{
*keys_for_command("FULL_RAW_MESSAGE"),
*keys_for_command("GO_BACK"),
*keys_for_command("EXIT_POPUP"),
},
)
def test_keypress_show_msg_info(
Expand Down Expand Up @@ -688,7 +688,7 @@ def test_keypress_exit_popup_invalid_key(
assert not self.controller.exit_popup.called

@pytest.mark.parametrize(
"key", {*keys_for_command("EDIT_HISTORY"), *keys_for_command("GO_BACK")}
"key", {*keys_for_command("EDIT_HISTORY"), *keys_for_command("EXIT_POPUP")}
)
def test_keypress_show_msg_info(
self, key: str, widget_size: Callable[[Widget], urwid_Size]
Expand Down Expand Up @@ -911,7 +911,7 @@ def test_keypress_any_key(
assert not self.controller.exit_popup.called

@pytest.mark.parametrize(
"key", {*keys_for_command("GO_BACK"), *keys_for_command("MARKDOWN_HELP")}
"key", {*keys_for_command("EXIT_POPUP"), *keys_for_command("MARKDOWN_HELP")}
)
def test_keypress_exit_popup(
self, key: str, widget_size: Callable[[Widget], urwid_Size]
Expand Down Expand Up @@ -942,7 +942,7 @@ def test_keypress_any_key(
assert not self.controller.exit_popup.called

@pytest.mark.parametrize(
"key", {*keys_for_command("GO_BACK"), *keys_for_command("HELP")}
"key", {*keys_for_command("EXIT_POPUP"), *keys_for_command("HELP")}
)
def test_keypress_exit_popup(
self, key: str, widget_size: Callable[[Widget], urwid_Size]
Expand Down Expand Up @@ -1113,7 +1113,7 @@ def test_keypress_full_raw_message(
)

@pytest.mark.parametrize(
"key", {*keys_for_command("GO_BACK"), *keys_for_command("MSG_INFO")}
"key", {*keys_for_command("EXIT_POPUP"), *keys_for_command("MSG_INFO")}
)
def test_keypress_exit_popup(
self, key: str, widget_size: Callable[[Widget], urwid_Size]
Expand Down Expand Up @@ -1550,7 +1550,7 @@ def test_footlinks(
assert footlinks_width == expected_footlinks_width

@pytest.mark.parametrize(
"key", {*keys_for_command("GO_BACK"), *keys_for_command("STREAM_INFO")}
"key", {*keys_for_command("EXIT_POPUP"), *keys_for_command("STREAM_INFO")}
)
def test_keypress_exit_popup(
self, key: str, widget_size: Callable[[Widget], urwid_Size]
Expand Down Expand Up @@ -1615,7 +1615,7 @@ def mock_external_classes(self, mocker: MockerFixture) -> None:
self.stream_members_view = StreamMembersView(self.controller, stream_id)

@pytest.mark.parametrize(
"key", {*keys_for_command("GO_BACK"), *keys_for_command("STREAM_MEMBERS")}
"key", {*keys_for_command("EXIT_POPUP"), *keys_for_command("STREAM_MEMBERS")}
)
def test_keypress_exit_popup(
self, key: str, widget_size: Callable[[Widget], urwid_Size]
Expand Down Expand Up @@ -1730,7 +1730,7 @@ def test_keypress_search_emoji(
assert self.emoji_picker_view.get_focus() == "header"

@pytest.mark.parametrize(
"key", {*keys_for_command("GO_BACK"), *keys_for_command("ADD_REACTION")}
"key", {*keys_for_command("EXIT_POPUP"), *keys_for_command("ADD_REACTION")}
)
def test_keypress_exit_called(
self, key: str, widget_size: Callable[[Widget], urwid_Size]
Expand Down
2 changes: 1 addition & 1 deletion tools/lint-hotkeys
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ SCRIPT_NAME = PurePath(__file__).name
HELP_TEXT_STYLE = re.compile(r"^[a-zA-Z /()',&@#:_-]*$")

# Exclude keys from duplicate keys checking
KEYS_TO_EXCLUDE = ["q", "e", "m", "r"]
KEYS_TO_EXCLUDE = ["q", "e", "m", "r", "Esc"]


def main(fix: bool) -> None:
Expand Down
21 changes: 15 additions & 6 deletions zulipterminal/config/keys.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,6 @@ class KeyBinding(TypedDict):
'help_text': 'Show/hide About Menu',
'key_category': 'general',
},
'GO_BACK': {
'keys': ['esc'],
'help_text': 'Go back',
'excluded_from_random_tips': False,
'key_category': 'general',
},
'OPEN_DRAFT': {
'keys': ['d'],
'help_text': 'Open draft message saved in this session',
Expand All @@ -62,6 +56,11 @@ class KeyBinding(TypedDict):
'help_text': 'Copy information from About Menu to clipboard',
'key_category': 'general',
},
'EXIT_POPUP': {
'keys': ['esc'],
'help_text': 'Close popup',
'key_category': 'navigation',
},
'GO_UP': {
'keys': ['up', 'k'],
'help_text': 'Go up / Previous message',
Expand Down Expand Up @@ -183,6 +182,11 @@ class KeyBinding(TypedDict):
'help_text': 'Narrow to compose box message recipient',
'key_category': 'msg_compose',
},
'EXIT_COMPOSE': {
'keys': ['esc'],
'help_text': 'Exit message compose box',
'key_category': 'msg_compose',
},
'TOGGLE_NARROW': {
'keys': ['z'],
'help_text':
Expand Down Expand Up @@ -260,6 +264,11 @@ class KeyBinding(TypedDict):
'help_text': 'Submit search and browse results',
'key_category': 'searching',
},
'CLEAR_SEARCH': {
'keys': ['esc'],
'help_text': 'Clear search in current panel',
'key_category': 'searching',
},
Comment on lines +267 to +271
Copy link
Member

Choose a reason for hiding this comment

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

Since we're aiming at contextual help messages, should we be using the word panel? It feels more like code terminology and might not be useful as a help message.

Copy link
Collaborator Author

@Niloth-p Niloth-p Jun 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm.
Found mentions of 'panel' in the user facing documentation.
What other word do you think would fit better in this context?
Edit: As discussed, it's hard to come up with suitable alternatives, so I've left it as 'panel'.

'TOGGLE_MUTE_STREAM': {
'keys': ['m'],
'help_text': 'Mute/unmute streams',
Expand Down
2 changes: 1 addition & 1 deletion zulipterminal/model.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1714,7 +1714,7 @@ def _handle_message_event(self, event: Event) -> None:
"Press '{}' to close this window."
)
notice = notice_template.format(
failed_command, primary_display_key_for_command("GO_BACK")
failed_command, primary_display_key_for_command("EXIT_POPUP")
)
self.controller.popup_with_message(notice, width=50)
self.controller.update_screen()
Expand Down
10 changes: 5 additions & 5 deletions zulipterminal/ui_tools/boxes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -775,7 +775,7 @@ def keypress(self, size: urwid_Size, key: str) -> Optional[str]:
if success:
self.msg_write_box.edit_text = ""
if self.msg_edit_state is not None:
self.keypress(size, primary_key_for_command("GO_BACK"))
self.keypress(size, primary_key_for_command("EXIT_COMPOSE"))
assert self.msg_edit_state is None
elif is_command_key("NARROW_MESSAGE_RECIPIENT", key):
if self.compose_box_status == "open_with_stream":
Expand All @@ -798,7 +798,7 @@ def keypress(self, size: urwid_Size, key: str) -> Optional[str]:
self.view.controller.report_error(
"Cannot narrow to message without specifying recipients."
)
elif is_command_key("GO_BACK", key):
elif is_command_key("EXIT_COMPOSE", key):
self.send_stop_typing_status()
self._set_compose_attributes_to_defaults()
self.view.controller.exit_editor_mode()
Expand Down Expand Up @@ -948,7 +948,7 @@ def main_view(self) -> Any:
def keypress(self, size: urwid_Size, key: str) -> Optional[str]:
if (
is_command_key("EXECUTE_SEARCH", key) and self.text_box.edit_text == ""
) or is_command_key("GO_BACK", key):
) or is_command_key("CLEAR_SEARCH", key):
self.text_box.set_edit_text("")
self.controller.exit_editor_mode()
self.controller.view.middle_column.set_focus("body")
Expand Down Expand Up @@ -1004,13 +1004,13 @@ def valid_char(self, ch: str) -> bool:
def keypress(self, size: urwid_Size, key: str) -> Optional[str]:
if (
is_command_key("EXECUTE_SEARCH", key) and self.get_edit_text() == ""
) or is_command_key("GO_BACK", key):
) or is_command_key("CLEAR_SEARCH", 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"))
self.panel_view.keypress(size, primary_key_for_command("CLEAR_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 "), " "])
Expand Down
Loading