Repository: zulip/zulip-terminal
Pull request #1520 (2024-06-17 17:30:56)

Create new help categories related to opening compose box
### What does this PR do, and why?
Groups key bindings from message actions into the new categories:
- Compose: New Message
- Compose: Replies

And in the 2nd commit, suggests renaming the "Composing a message" category to "Writing a message" category, to improve clarity, in contrast with the newly added categories.

### External discussion & connections
- [x] Discussed in **#zulip-terminal** in `Re-Categorization of Hotkeys`
- [ ] Fully fixes #
- [ ] Partially fixes issue #
- [ ] Builds upon previous unmerged work in PR #
- [ ] Is a follow-up to work in PR #
- [ ] Requires merge of PR #
- [x] Merge will enable work on #1484

### How did you test this?
- [ ] Manually - Behavioral changes
- [x] Manually - Visual changes
- [ ] Adapting existing automated tests
- [ ] Adding automated tests for new behavior (or missing tests)
- [ ] Existing automated tests should already cover this (*only a refactor of tested code*)

### Self-review checklist for each commit
- [x] It is a [minimal coherent idea](https://github.com/zulip/zulip-terminal#structuring-commits---speeding-up-reviews-merging--development)
- [x] It has a commit summary following the [documented style](https://github.com/zulip/zulip-terminal#structuring-commits---speeding-up-reviews-merging--development) (title & body)
- [x] It has a commit summary describing the  motivation and reasoning for the change
- [x] It individually passes linting and tests
- [ ] It contains test additions for any new behavior
- [ ] It flows clearly from a previous branch commit, and/or prepares for the next commit

### Visual Changes
![image](https://github.com/zulip/zulip-terminal/assets/20315308/6b38c06e-b926-4d0a-8fa6-29b4bf35de08)


Niloth-p: Rebased, resolving merge conflicts and updating INSERT_NEW_LINE's category along with the rest in the 2nd commit.
zulipbot: Heads up @Niloth-p, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's [recent commits](https://github.com/zulip/zulip-terminal/commits/main) to see where the conflicts occur. Please rebase your feature branch against the `upstream/main` branch and [resolve](https://zulip.readthedocs.io/en/latest/git/troubleshooting.html#recover-from-a-git-rebase-failure) your pull request's merge conflicts accordingly.

<!-- mergeConflictWarning -->

neiljp: @Niloth-p Thanks for sticking with these improvements - I agree they are definitely helping :+1:

Following on from my summary in #1524, my main concern here is that the categories are small (3 or 4 entries), plus the draft option doesn't entirely fit as a 'new message' (which would make it 2, and find somewhere to put the draft entry?). While it potentially makes it easier to find some entries when there are very many categories, the vertical spacing starts to take over. Do you agree?

I think we'll still gain from splitting these commands out into one section, and the renaming of the other section certainly remains worthwhile too. 

Having just merged the GO_BACK splitting PR (#1514), it also prompts me to think whether the exiting of compose could be included here (too), as a 'Starting/Exiting compose' sort of category. However, that introduces multi-category commands again, and could make naming the single category more challenging - this isn't as important as getting the splitting and renaming done and merged.
Niloth-p: Updates:
- Merged the 2 new categories.
- Rebased onto main, and updated the recently newly added EXIT_COMPOSE.

Things to check:
- The naming of the newly formed category - both internally and externally
- Is the ordering of help keys in the new category satisfactory?
Niloth-p: > While it potentially makes it easier to find some entries when there are very many categories, the vertical spacing starts to take over. Do you agree?

Regarding my opinion,
The spacing would definitely dominate in smaller screens. 3 is quite a small category indeed.

But, I like smaller help categories in general for any application, but that's probably because I try to memorize them (and the related commands) instead of just referring to a single command.
So, tighter categories help in that aspect, instead of me having to break them down into further tiny categories in my own mind, it's already done for me. So, the tightness/fit of entries into categories would be more preferred over the size of categories. 
But, that's probably me, I have no idea what users majorly prefer.

Patch:
diff --git a/docs/hotkeys.md b/docs/hotkeys.md
index d1cd722..e984694 100644
--- a/docs/hotkeys.md
+++ b/docs/hotkeys.md
@@ -8,11 +8,11 @@
 |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>|
-|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>|
 |Quit|<kbd>Ctrl</kbd> + <kbd>c</kbd>|
 |Show/hide user information (from users list)|<kbd>i</kbd>|
+|New footer hotkey hint|<kbd>Tab</kbd>|
 
 ## Navigation
 |Command|Key Combination|
@@ -47,13 +47,7 @@
 ## Message actions
 |Command|Key Combination|
 | :--- | :---: |
-|Reply to the current message|<kbd>r</kbd> / <kbd>Enter</kbd>|
-|Reply mentioning the sender of the current message|<kbd>@</kbd>|
-|Reply quoting the current message text|<kbd>></kbd>|
-|Reply directly to the sender of the current message|<kbd>R</kbd>|
 |Edit message's content or topic|<kbd>e</kbd>|
-|New message to a stream|<kbd>c</kbd>|
-|New message to a person or group of people|<kbd>x</kbd>|
 |Show/hide emoji picker for current message|<kbd>:</kbd>|
 |Narrow to the stream of the current message|<kbd>s</kbd>|
 |Narrow to the topic of the current message|<kbd>S</kbd>|
@@ -77,7 +71,18 @@
 |Show/hide stream members (from stream information)|<kbd>m</kbd>|
 |Copy stream email to clipboard (from stream information)|<kbd>c</kbd>|
 
-## Composing a Message
+## Begin composing a message
+|Command|Key Combination|
+| :--- | :---: |
+|Open draft message saved in this session|<kbd>d</kbd>|
+|Reply to the current message|<kbd>r</kbd> / <kbd>Enter</kbd>|
+|Reply mentioning the sender of the current message|<kbd>@</kbd>|
+|Reply quoting the current message text|<kbd>></kbd>|
+|Reply directly to the sender of the current message|<kbd>R</kbd>|
+|New message to a stream|<kbd>c</kbd>|
+|New message to a person or group of people|<kbd>x</kbd>|
+
+## Writing a message
 |Command|Key Combination|
 | :--- | :---: |
 |Cycle through recipient and content boxes|<kbd>Tab</kbd>|
diff --git a/zulipterminal/config/keys.py b/zulipterminal/config/keys.py
index 254778a..f2111c2 100644
--- a/zulipterminal/config/keys.py
+++ b/zulipterminal/config/keys.py
@@ -49,7 +49,7 @@ KEY_BINDINGS: Dict[str, KeyBinding] = {
     'OPEN_DRAFT': {
         'keys': ['d'],
         'help_text': 'Open draft message saved in this session',
-        'key_category': 'general',
+        'key_category': 'open_compose',
     },
     'COPY_ABOUT_INFO': {
         'keys': ['c'],
@@ -104,22 +104,22 @@ KEY_BINDINGS: Dict[str, KeyBinding] = {
     'REPLY_MESSAGE': {
         'keys': ['r', 'enter'],
         'help_text': 'Reply to the current message',
-        'key_category': 'msg_actions',
+        'key_category': 'open_compose',
     },
     'MENTION_REPLY': {
         'keys': ['@'],
         'help_text': 'Reply mentioning the sender of the current message',
-        'key_category': 'msg_actions',
+        'key_category': 'open_compose',
     },
     'QUOTE_REPLY': {
         'keys': ['>'],
         'help_text': 'Reply quoting the current message text',
-        'key_category': 'msg_actions',
+        'key_category': 'open_compose',
     },
     'REPLY_AUTHOR': {
         'keys': ['R'],
         'help_text': 'Reply directly to the sender of the current message',
-        'key_category': 'msg_actions',
+        'key_category': 'open_compose',
     },
     'EDIT_MESSAGE': {
         'keys': ['e'],
@@ -129,38 +129,38 @@ KEY_BINDINGS: Dict[str, KeyBinding] = {
     'STREAM_MESSAGE': {
         'keys': ['c'],
         'help_text': 'New message to a stream',
-        'key_category': 'msg_actions',
+        'key_category': 'open_compose',
     },
     'PRIVATE_MESSAGE': {
         'keys': ['x'],
         'help_text': 'New message to a person or group of people',
-        'key_category': 'msg_actions',
+        'key_category': 'open_compose',
     },
     'CYCLE_COMPOSE_FOCUS': {
         'keys': ['tab'],
         'help_text': 'Cycle through recipient and content boxes',
-        'key_category': 'msg_compose',
+        'key_category': 'compose_box',
     },
     'SEND_MESSAGE': {
         'keys': ['ctrl d', 'meta enter'],
         'help_text': 'Send a message',
-        'key_category': 'msg_compose',
+        'key_category': 'compose_box',
     },
     'SAVE_AS_DRAFT': {
         'keys': ['meta s'],
         'help_text': 'Save current message as a draft',
-        'key_category': 'msg_compose',
+        'key_category': 'compose_box',
     },
     'AUTOCOMPLETE': {
         'keys': ['ctrl f'],
         'help_text': ('Autocomplete @mentions, #stream_names, :emoji:'
                       ' and topics'),
-        'key_category': 'msg_compose',
+        'key_category': 'compose_box',
     },
     'AUTOCOMPLETE_REVERSE': {
         'keys': ['ctrl r'],
         'help_text': 'Cycle through autocomplete suggestions in reverse',
-        'key_category': 'msg_compose',
+        'key_category': 'compose_box',
     },
     'ADD_REACTION': {
         'keys': [':'],
@@ -180,12 +180,12 @@ KEY_BINDINGS: Dict[str, KeyBinding] = {
     'NARROW_MESSAGE_RECIPIENT': {
         'keys': ['meta .'],
         'help_text': 'Narrow to compose box message recipient',
-        'key_category': 'msg_compose',
+        'key_category': 'compose_box',
     },
     'EXIT_COMPOSE': {
         'keys': ['esc'],
         'help_text': 'Exit message compose box',
-        'key_category': 'msg_compose',
+        'key_category': 'compose_box',
     },
     'TOGGLE_NARROW': {
         'keys': ['z'],
@@ -426,7 +426,7 @@ KEY_BINDINGS: Dict[str, KeyBinding] = {
         # and to differentiate from other hotkeys using 'enter'.
         'keys': ['enter'],
         'help_text': 'Insert new line',
-        'key_category': 'msg_compose',
+        'key_category': 'compose_box',
     },
     'FULL_RENDERED_MESSAGE': {
         'keys': ['f'],
@@ -438,6 +438,11 @@ KEY_BINDINGS: Dict[str, KeyBinding] = {
         'help_text': 'Show/hide full raw message (from message information)',
         'key_category': 'msg_actions',
     },
+    'NEW_HINT': {
+        'keys': ['tab'],
+        'help_text': 'New footer hotkey hint',
+        'key_category': 'general',
+    },
 }
 # fmt: on
 
@@ -447,7 +452,8 @@ HELP_CATEGORIES = {
     "searching": "Searching",
     "msg_actions": "Message actions",
     "stream_list": "Stream list actions",
-    "msg_compose": "Composing a Message",
+    "open_compose": "Begin composing a message",
+    "compose_box": "Writing a message",
     "editor_navigation": "Editor: Navigation",
     "editor_text_manipulation": "Editor: Text Manipulation",
 }
diff --git a/zulipterminal/helper.py b/zulipterminal/helper.py
index 76d0a01..a71d055 100644
--- a/zulipterminal/helper.py
+++ b/zulipterminal/helper.py
@@ -784,6 +784,7 @@ def process_media(controller: Any, link: str) -> None:
         controller.view.set_footer_text, "Downloading your media..."
     )
     media_path = download_media(controller, link, show_download_status)
+    media_path = normalized_file_path(media_path)
     tool = ""
 
     # TODO: Add support for other platforms as well.
@@ -825,7 +826,7 @@ def download_media(
                     show_download_status()
 
         controller.report_success([" Downloaded ", ("bold", media_name)])
-        return normalized_file_path(local_path)
+        return local_path
 
 
 @asynch
diff --git a/zulipterminal/ui.py b/zulipterminal/ui.py
index b6a5a3d..d0785bd 100644
--- a/zulipterminal/ui.py
+++ b/zulipterminal/ui.py
@@ -328,6 +328,9 @@ class View(urwid.WidgetWrap):
         elif is_command_key("MARKDOWN_HELP", key):
             self.controller.show_markdown_help()
             return key
+        elif is_command_key("NEW_HINT", key):
+            self.set_footer_text()
+            return key
         return super().keypress(size, key)
 
     def mouse_event(


Test patch:
diff --git a/tests/ui/test_ui.py b/tests/ui/test_ui.py
index 8693869..d017f46 100644
--- a/tests/ui/test_ui.py
+++ b/tests/ui/test_ui.py
@@ -341,6 +341,23 @@ class TestView:
         assert returned_key == key
         assert view.body.focus_col == 1
 
+    @pytest.mark.parametrize("key", keys_for_command("NEW_HINT"))
+    def test_keypress_NEW_HINT(
+        self,
+        view: View,
+        mocker: MockerFixture,
+        key: str,
+        widget_size: Callable[[Widget], urwid_Box],
+    ) -> None:
+        size = widget_size(view)
+        set_footer_text = mocker.patch(VIEW + ".set_footer_text")
+        mocker.patch(CONTROLLER + ".is_in_editor_mode", return_value=False)
+
+        returned_key = view.keypress(size, key)
+
+        set_footer_text.assert_called_once_with()
+        assert returned_key == key
+
     @pytest.mark.parametrize("key", keys_for_command("SEARCH_PEOPLE"))
     @pytest.mark.parametrize("autohide", [True, False], ids=["autohide", "no_autohide"])
     def test_keypress_autohide_users(