Repository: zulip/zulip-terminal

Issue:
Message information popup: Tidy shortcut key text & place into a group
The current shortcut/hotkey list in the message information popup (`i` on a message) could be improved by:
- moving the current hotkey lines into a separate display-group, maybe titled 'Viewing Actions'
- simplifying the text in each entry, including replacing the 'Press <key> to...' text by `[<key>]` (like we use in other parts of the UI)

All 'popups' are currently defined in `views.py`, though may be split out in future since their tests are already extracted into `test_popups.py`. See `MsgInfoView` for the relevant class.

There are currently existing display-groups for links in messages, links in topics that a message is in, time mentions, and reactions - though these are conditional upon those elements being present. Explore those in existing messages, or create some in **#test here** on chat.zulip.org (or your own Zulip server), which should give a guide of how groups are shown.

Discussion:
zulipbot: Hello @axrav, it seems like you have referenced #1339 in your pull request description, but you have not referenced them in your commit message description(s). Referencing an issue in a commit message automatically closes the corresponding issue when the commit is merged, which makes the issue tracker easier to manage.

Please run `git commit --amend` in your command line client to amend your commit message description with `Fixes #1339.`.

An example of a correctly-formatted commit:

```
commit fabd5e450374c8dde65ec35f02140383940fe146
Author: zulipbot
Date:   Sat Mar 18 13:42:40 2017 -0700

    pull requests: Check PR commits reference when issue is referenced.

    Fixes #51.
```

To learn how to write a great commit message, please refer to our [guide](https://zulip.readthedocs.io/en/latest/contributing/version-control.html#commit-messages).

<!-- fixCommitWarning -->

axrav: @neiljp I have made the changes you requested, one of the workflows job failed, but the lint issues have already been fixed, can you have a look onto it.
neiljp: @axrav Thanks for the update; the visual change looks better :+1:

As per the inline comment, we can likely combine lines; black treats trailing commas as an instruction to not combine lines in some cases, but since the code is now shorter, it may be able to do so.

The failing test is since each commit does not pass linting and tests individually, even though the overall PR does. That occurs if you fix a linting issue in a separate commit to the commit where the change was made, for example. You can check this locally using `tools/check-branch`.

So you'll want to squash the changes into the original commit. As it stands, these commits look like local commits, rather than those easier to read and review. Please read the guidelines in the README, in particular the Structuring Commits section. Note that not all changes are always squashed into a single commit, as per those notes. Please also update the commit text for the final commit into the preferred style.
axrav: @neiljp if everything looks good, can you merge this up, so I ll be able to assign myself new issues. 
neiljp: @axrav Thanks for working on this - This does look much better :+1:

I wanted to correct the commit title/body first, hence the minor delay, but I'm merging now 🎉 

Please take a look at the merged commit text for how it differs from your pushed one - the less tidying at merging, the faster everything is :)

Patch:
diff --git a/zulipterminal/ui_tools/views.py b/zulipterminal/ui_tools/views.py
index 1ae51ec..8f6ad15 100644
--- a/zulipterminal/ui_tools/views.py
+++ b/zulipterminal/ui_tools/views.py
@@ -1520,13 +1520,15 @@ class MsgInfoView(PopUpView):
         date_and_time = controller.model.formatted_local_time(
             msg["timestamp"], show_seconds=True, show_year=True
         )
-        view_in_browser_keys = ", ".join(map(repr, keys_for_command("VIEW_IN_BROWSER")))
+        view_in_browser_keys = "[{}]".format(
+            ", ".join(map(str, keys_for_command("VIEW_IN_BROWSER")))
+        )
 
-        full_rendered_message_keys = ", ".join(
-            map(repr, keys_for_command("FULL_RENDERED_MESSAGE"))
+        full_rendered_message_keys = "[{}]".format(
+            ", ".join(map(str, keys_for_command("FULL_RENDERED_MESSAGE")))
         )
-        full_raw_message_keys = ", ".join(
-            map(repr, keys_for_command("FULL_RAW_MESSAGE"))
+        full_raw_message_keys = "[{}]".format(
+            ", ".join(map(str, keys_for_command("FULL_RAW_MESSAGE")))
         )
         msg_info = [
             (
@@ -1535,22 +1537,22 @@ class MsgInfoView(PopUpView):
                     ("Date & Time", date_and_time),
                     ("Sender", msg["sender_full_name"]),
                     ("Sender's Email ID", msg["sender_email"]),
-                    (
-                        "View message in browser",
-                        f"Press {view_in_browser_keys} to view message in browser",
-                    ),
-                    (
-                        "Full rendered message",
-                        f"Press {full_rendered_message_keys} to view",
-                    ),
-                    (
-                        "Full raw message",
-                        f"Press {full_raw_message_keys} to view",
-                    ),
                 ],
-            ),
+            )
         ]
+
+        # actions for message info popup
+        viewing_actions = (
+            "Viewing Actions",
+            [
+                ("Open in web browser", view_in_browser_keys),
+                ("Full rendered message", full_rendered_message_keys),
+                ("Full raw message", full_raw_message_keys),
+            ],
+        )
+        msg_info.append(viewing_actions)
         # Only show the 'Edit History' label for edited messages.
+
         self.show_edit_history_label = (
             self.msg["id"] in controller.model.index["edited_messages"]
             and controller.model.initial_data["realm_allow_edit_history"]
@@ -1558,8 +1560,8 @@ class MsgInfoView(PopUpView):
         if self.show_edit_history_label:
             msg_info[0][1][0] = ("Date & Time (Original)", date_and_time)
 
-            keys = ", ".join(map(repr, keys_for_command("EDIT_HISTORY")))
-            msg_info[0][1].append(("Edit History", f"Press {keys} to view"))
+            keys = "[{}]".format(", ".join(map(str, keys_for_command("EDIT_HISTORY"))))
+            msg_info[1][1].append(("Edit History", keys))
         # Render the category using the existing table methods if links exist.
         if message_links:
             msg_info.append(("Message Links", []))
@@ -1594,7 +1596,9 @@ class MsgInfoView(PopUpView):
 
             # slice_index = Number of labels before message links + 1 newline
             #               + 1 'Message Links' category label.
-            slice_index = len(msg_info[0][1]) + 2
+            #               + 2 for Viewing Actions category label and its newline
+            slice_index = len(msg_info[0][1]) + len(msg_info[1][1]) + 2 + 2
+
             slice_index += sum([len(w) + 2 for w in self.button_widgets])
             self.button_widgets.append(message_links)
 
@@ -1610,7 +1614,8 @@ class MsgInfoView(PopUpView):
 
             # slice_index = Number of labels before topic links + 1 newline
             #               + 1 'Topic Links' category label.
-            slice_index = len(msg_info[0][1]) + 2
+            #               + 2 for Viewing Actions category label and its newline
+            slice_index = len(msg_info[0][1]) + len(msg_info[1][1]) + 2 + 2
             slice_index += sum([len(w) + 2 for w in self.button_widgets])
             self.button_widgets.append(topic_links)
 


Test patch:
diff --git a/tests/ui_tools/test_popups.py b/tests/ui_tools/test_popups.py
index a1eb9fe..3205a13 100644
--- a/tests/ui_tools/test_popups.py
+++ b/tests/ui_tools/test_popups.py
@@ -1027,8 +1027,10 @@ class TestMsgInfoView:
         assert self.controller.open_in_browser.called
 
     def test_height_noreactions(self) -> None:
-        expected_height = 6
+        expected_height = 8
         # 6 = 1 (date & time) +1 (sender's name) +1 (sender's email)
+        # +1 (display group header)
+        # +1 (whitespace column)
         # +1 (view message in browser)
         # +1 (full rendered message)
         # +1 (full raw message)
@@ -1099,9 +1101,9 @@ class TestMsgInfoView:
             OrderedDict(),
             list(),
         )
-        # 12 = 6 labels + 1 blank line + 1 'Reactions' (category)
+        # 12 = 7 labels + 2 blank lines + 1 'Reactions' (category)
         # + 4 reactions (excluding 'Message Links').
-        expected_height = 12
+        expected_height = 14
         assert self.msg_info_view.height == expected_height
 
     @pytest.mark.parametrize(