Group narrowing commands and improve descriptions #1516

Niloth-p · 2024-06-07T01:50:11Z

What does this PR do, and why?

Groups commands that switch narrows
Improves their help descriptions

Outstanding aspect(s)

Feedback on the following are needed:

name of the chosen help category
order of the commands within the narrowing category
help descriptions

External discussion & connections

Discussed in #zulip-terminal in re-categorization
Fully fixes #
Partially fixes issue #
Builds upon previous unmerged work in PR #
Is a follow-up to work in PR #
Requires merge of PR #
Merge will enable work on #

How did you test this?

Manually - Behavioral changes
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

It is a minimal coherent idea
It has a commit summary following the documented style (title & body)
It has a commit summary describing the motivation and reasoning for the change
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

rsashank · 2024-06-16T19:49:04Z

docs/hotkeys.md

 |Narrow to the stream of the current message|<kbd>s</kbd>|
 |Narrow to the topic of the current message|<kbd>S</kbd>|
 |Narrow to compose box message recipient|<kbd>Meta</kbd> + <kbd>.</kbd>|
-|Narrow to a topic/direct-chat, or stream/all-direct-messages|<kbd>z</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>|
+|Zoom in/out the message's conversation context|<kbd>z</kbd>|
+|Go to your combined feed|<kbd>a</kbd> / <kbd>Esc</kbd>|
+|Go to your direct message feed|<kbd>P</kbd>|
+|Go to your starred messages|<kbd>f</kbd>|
+|Go to messages in which you're mentioned|<kbd>#</kbd>|


I'm not entirely sure about the language we should use here, but should we consider using "Go to" instead of "Narrow" for lines 34-36 as well? To me, "Go to" seems clearer than "Narrow." However, this is just my opinion, so we'll need to wait for feedback from Neil and the mentors.

The rewording of the to "Zoom in/out" seems great, though!

Thanks for the review!

Ah, good point! Yes, I was considering the same, but I wasn't sure because those narrowing commands kind of use the current context and don't switch to something completely different or a named narrow, like the menu buttons do. But, it's probably just me.
And that's also why line 41 doesn't feel like a great fit either, because it's not a named narrow.
Thank you for sharing your thoughts!

Zoom in/out credits: Zulip webapp

For context, historically - at least in ZT - we've had confusion over communicating 'a narrow' (message view; noun) and 'narrowing' (switching to a message view; verb).

Most of this likely comes from the phrasing in the code, and also how the API defines a narrow as a filter of the messages you can see: https://zulip.com/api/construct-narrow.
(note we currently use an old API style for narrows, which needs to be refactored to support the documented style which uses a dict)

You can also see this in our user-facing tutorial: https://github.com/zulip/zulip-terminal/blob/main/docs/getting-started.md#Narrowing

So, there's potentially some small follow-up we could do to clarify this for end-users, if we're switching to 'message view' for the noun form.

Regarding the specific verb prefix selection (go-to vs narrow):

ideally it would be as concise as possible, as well as consistent

some of the contextual grouping we have was originally intended to make it easier to understand based on the category titles, but since the descriptions can appear without them, then we can't assume their presence and simplify that much

'go' is used a lot in the web app descriptions for 'narrowing' (https://zulip.com/help/keyboard-shortcuts)

the web app or docs actually don't seem to use narrow for either form? (outside of API)

narrowing can change keyboard focus, but we currently use 'go' for the keyboard movement, so that could be confusing when really it's specifically changing the message view

Re z, this is a simple toggleable hotkey in ZT, which we introduced before the web app switched to using just s for doing almost the same. In web it doesn't apply to DMs, so it's not really a 'safe' toggle between views - and I'm not sure if shift s (S) still works. I'm inclined to suggest Toggle could be a good phrase here.

'Zoom in/out' is definitely cleaner, and relates to why we chose z in the first place, but the challenge with the phrasing here has always been how to cleanly describe the different contexts. 'Conversation' seems to be the term used for 'most zoomed in' view at the moment, but DM feed and channel feeds are not quite as clear cut, but might be useful?

(for the reasons above, z should be right next to s/S in the help)

neiljp

@Niloth-p Thanks for this thoughtful work 👍
(and @rsashank's review :+1)

Following up from the changes to eg. have Help Menu and About Menu capitalized similarly, we could also consider doing similar changes to parts of the UI like All Messages and Direct Messages. These could be simple capitalization, or possibly subsequently styled versions, to highlight that they refer to specific defined items.

As per most of this work, the challenge is picking the correct terms and phrases, and using them consistently. That'll likely be easier as we go on, and we may want to discuss those specific terms, including eg. for referring to popups/menus/...

neiljp · 2024-06-23T02:10:59Z

docs/hotkeys.md

+## Switch message view
+|Command|Key Combination|
+| :--- | :---: |
+|Narrow to the stream of the current message|<kbd>s</kbd>|
+|Narrow to the topic of the current message|<kbd>S</kbd>|
+|Narrow to compose box message recipient|<kbd>Meta</kbd> + <kbd>.</kbd>|
+|Narrow to a topic/direct-chat, or stream/all-direct-messages|<kbd>z</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>|


The web app calls this 'Navigation' and includes the n and p keys here.

While that name for the category is short, it doesn't quite seem correct with us supporting left/right navigation too; maybe 'Switching messages view' would be more consistent, at least for now? The title of the middle section is currently the plural form: messages, and we have other '-ing' forms of category titles.

For n and p these can jump inside and between conversations, which suggests something other than 'Switching' for the title (perhaps something closer to 'Navigating' or 'Jumping'), but this seems minor until we fine-tune the View/Narrow/Navigate nuances.

Re moving the Meta + . command here, it's certainly useful to know that it fits in this category, though it could certainly be useful in the other section too - something which I think you have have explored enabling in #1519? Was it for this purpose?

As per discussion in #1497, keeping 'Enter' in 'Navigation' (ie. scrolling/left/right/...) seems good for now. One wrinkle is that in addition to that general-purpose behavior, it would be useful to also include that 'Enter' activates narrowing buttons in the left panel in general, much like the change you've added in another PR for replying to a user in the right (user) panel.

neiljp · 2024-06-23T02:18:23Z

zulipterminal/config/keys.py

@@ -416,6 +416,7 @@ class KeyBinding(TypedDict):
 HELP_CATEGORIES = {
    "general": "General",
    "navigation": "Navigation",
+    "narrow": "Switch message view",


Nit: This isn't user-facing, but maybe "narrowing" here?

neiljp · 2024-06-23T02:24:58Z

zulipterminal/config/keys.py

@@ -190,22 +190,22 @@ class KeyBinding(TypedDict):
    },
    'ALL_MESSAGES': {
        'keys': ['a', 'esc'],
-        'help_text': 'Narrow to all messages',
+        'help_text': 'Go to your combined feed',


If we're making this change, we should also update the UI to use Combined Feed at the same time; since these are different changes, they should at least be in different commits - ie. I wouldn't expect a UI rename of that type in this commit. That renaming was discussed in #zulip terminal > Combined feed renaming.

neiljp · 2024-06-23T17:06:23Z

docs/hotkeys.md

 |Narrow to the stream of the current message|<kbd>s</kbd>|
 |Narrow to the topic of the current message|<kbd>S</kbd>|
 |Narrow to compose box message recipient|<kbd>Meta</kbd> + <kbd>.</kbd>|
-|Narrow to a topic/direct-chat, or stream/all-direct-messages|<kbd>z</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>|
+|Zoom in/out the message's conversation context|<kbd>z</kbd>|
+|Go to your combined feed|<kbd>a</kbd> / <kbd>Esc</kbd>|
+|Go to your direct message feed|<kbd>P</kbd>|
+|Go to your starred messages|<kbd>f</kbd>|
+|Go to messages in which you're mentioned|<kbd>#</kbd>|


For context, historically - at least in ZT - we've had confusion over communicating 'a narrow' (message view; noun) and 'narrowing' (switching to a message view; verb).

Most of this likely comes from the phrasing in the code, and also how the API defines a narrow as a filter of the messages you can see: https://zulip.com/api/construct-narrow.
(note we currently use an old API style for narrows, which needs to be refactored to support the documented style which uses a dict)

You can also see this in our user-facing tutorial: https://github.com/zulip/zulip-terminal/blob/main/docs/getting-started.md#Narrowing

So, there's potentially some small follow-up we could do to clarify this for end-users, if we're switching to 'message view' for the noun form.

Regarding the specific verb prefix selection (go-to vs narrow):

ideally it would be as concise as possible, as well as consistent

some of the contextual grouping we have was originally intended to make it easier to understand based on the category titles, but since the descriptions can appear without them, then we can't assume their presence and simplify that much

'go' is used a lot in the web app descriptions for 'narrowing' (https://zulip.com/help/keyboard-shortcuts)

the web app or docs actually don't seem to use narrow for either form? (outside of API)

narrowing can change keyboard focus, but we currently use 'go' for the keyboard movement, so that could be confusing when really it's specifically changing the message view

neiljp · 2024-06-23T17:19:02Z

docs/hotkeys.md

 |Narrow to the stream of the current message|<kbd>s</kbd>|
 |Narrow to the topic of the current message|<kbd>S</kbd>|
 |Narrow to compose box message recipient|<kbd>Meta</kbd> + <kbd>.</kbd>|
-|Narrow to a topic/direct-chat, or stream/all-direct-messages|<kbd>z</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>|
+|Zoom in/out the message's conversation context|<kbd>z</kbd>|
+|Go to your combined feed|<kbd>a</kbd> / <kbd>Esc</kbd>|
+|Go to your direct message feed|<kbd>P</kbd>|
+|Go to your starred messages|<kbd>f</kbd>|
+|Go to messages in which you're mentioned|<kbd>#</kbd>|


Re z, this is a simple toggleable hotkey in ZT, which we introduced before the web app switched to using just s for doing almost the same. In web it doesn't apply to DMs, so it's not really a 'safe' toggle between views - and I'm not sure if shift s (S) still works. I'm inclined to suggest Toggle could be a good phrase here.

'Zoom in/out' is definitely cleaner, and relates to why we chose z in the first place, but the challenge with the phrasing here has always been how to cleanly describe the different contexts. 'Conversation' seems to be the term used for 'most zoomed in' view at the moment, but DM feed and channel feeds are not quite as clear cut, but might be useful?

(for the reasons above, z should be right next to s/S in the help)

Niloth-p · 2024-06-24T22:57:24Z

I've made the following changes:

1st commit

Re-ordered to s, S, z, Meta .
User-facing category name: "Switch message view" -> "Switching messages view"
Internal category name: "narrow" -> "narrowing"

2nd commit

The previous PR version used "Go to", this version uses "View" and "View all". Please refer to the commit description for details.

Follow-ups to address in separate commits / PRs:

All Messages -> Combined Feed everywhere
- All DMs -> your DM feed or equivalent
Replace "narrow" with "message view" or others in user-facing documentation like the user tutorial.
Consider casefix changes to parts of the UI like - All Messages and Direct Messages
I didn't do this as part of the 2nd commit because it makes sense grammatically, without marking them as proper nouns. We could do that when we start using "Feed" terminology?
Add a new key binding mentioning that 'Enter' activates narrowing buttons
I don't see this help text properly fitting in any category that we have at the moment, and general is just too broad.
We could instead not add this to a category, but just add it to a context, following the discussions in CZO topics Restructure Key Bindings? and Supporting Help Multi-categories #T1519.
We have the ACTIVATE_BUTTON key binding at the moment.
I've used a mention of 'PM' in the help text. I know that we ideally want to use DMs and key it in with ZFL, but hopefully we can handle that as a follow-up PR too, as we're currently using PM everywhere in the code base.
Return to n & p later after fine-tuning the View/Narrow/Navigate nuances.

Please let me know if I've missed anything or misunderstood something, @neiljp.

zormit

To me this looks very fine. I won't think about wording too much/too deeply, I think it's improving in this PR and that's the most important thing to me :) Neil had already a look, so I'll leave the details to him.

Niloth-p · 2024-07-07T05:08:29Z

Note: The "Narrow to compose box message recipient" command that we just assigned to category "compose_box" in #1520 is now re-assigned to category "narrowing".
Comparing the 2 categories, I think the "narrowing" category is a better fit, as the compose box relation can be covered with the contextual help when added.

neiljp · 2024-07-07T06:42:55Z

@Niloth-p Thanks for the rebase 👍 I agree about the narrowing category, certainly until we consider multi-categories (if we do).

It would seem that n/p would also be best in this same category for now? That is, compared to others that will be present in the short term?

The meta . change has issues:

it changes to any conversation, not just DMs/PMs
if it was DMs/PMs only, the UI itself should all be expressed as 'Direct' by now
'Open PM' reads strangely to me, but given the above it can be changed in any case

I believe those were the two points I had noticed and was starting to work through before you pushed again this evening :)

Hotkeys document regenerated.

To make the 'z' hotkey be listed immediately after the related 's' and 'S' hotkeys in the help menu.

Niloth-p · 2024-07-07T13:28:39Z

Grouped 'p' and 'n' into the 'narrowing' category.
Oops, thank you!
I've rephrased it to "Switch message view to the compose box target". I felt 'recipient' could be made a bit more user-friendly.
I noticed that I had somehow had missed the re-ordering changes while rebasing on merges, at some point of time. So, I created a separate commit for that and kept it in the middle, so that it can be squashed into either the first or the last, if necessary.
Updated the commit descriptions.

Replace "Narrow to": - Use "View" instead of "Narrow to" - Use "View all" for those corresponding to narrow buttons - Use "Switch message view" for the compose box message recipient Rephrasing help texts: - Use "zoom in/out" to simplify and shorten the longest description, while also being clear on the choice of key being 'z' - "compose box message recipient" -> "compose box target" reduces length and user-friendliness, while retaining clarity Hotkeys document regenerated.

neiljp · 2024-07-08T05:01:32Z

@Niloth-p Thanks for the update 👍 I partially reversed the mentions text, but that's a marginal language change, and otherwise I just tweaked the commit text.

The middle commit is small, but helps keep the reordering clear, since the diff is less clear otherwise, so I've left it separate 👍

Merging this now, thanks all! 🎉

neiljp · 2024-07-08T05:16:26Z

I've added the follow-up points you noted above as a summary into the bottom of #1524 to keep them tracked, as well as a few other points.

zulipbot added the size: M [Automatic label added by zulipbot] label Jun 7, 2024

Niloth-p added the PR needs buddy review label Jun 10, 2024

Niloth-p mentioned this pull request Jun 13, 2024

Break down the generalized ENTER command #1497

Merged

18 tasks

rsashank reviewed Jun 16, 2024

View reviewed changes

rsashank removed the PR needs buddy review label Jun 16, 2024

Niloth-p added the PR needs mentor review label Jun 16, 2024

Niloth-p requested a review from zormit June 16, 2024 20:16

neiljp reviewed Jun 23, 2024

View reviewed changes

neiljp added the PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback label Jun 23, 2024

neiljp mentioned this pull request Jun 24, 2024

TRACKING: Improve hotkey descriptions, categories & accuracy #1524

Open

34 tasks

Niloth-p force-pushed the category/1516-narrowing/pr branch from df45f34 to 873cc42 Compare June 24, 2024 22:51

zulipbot added size: L [Automatic label added by zulipbot] and removed size: M [Automatic label added by zulipbot] labels Jun 24, 2024

Niloth-p removed the PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback label Jun 24, 2024

zormit approved these changes Jun 27, 2024

View reviewed changes

zormit added PR needs review PR requires feedback to proceed and removed PR needs mentor review labels Jun 27, 2024

Niloth-p mentioned this pull request Jun 28, 2024

Add title-to-title scroll feature to Help Menu #1527

Open

19 tasks

zulipbot added the has conflicts label Jul 7, 2024

Niloth-p force-pushed the category/1516-narrowing/pr branch from 873cc42 to b1ea99b Compare July 7, 2024 04:58

zulipbot removed the has conflicts label Jul 7, 2024

Niloth-p added 2 commits July 7, 2024 18:52

keys: Add new help category to group narrowing commands.

fa63fa9

Hotkeys document regenerated.

keys: Re-order the 'narrowing' key bindings.

fbaa177

To make the 'z' hotkey be listed immediately after the related 's' and 'S' hotkeys in the help menu.

Niloth-p force-pushed the category/1516-narrowing/pr branch from b1ea99b to 8f3dd49 Compare July 7, 2024 13:25

neiljp removed the PR needs review PR requires feedback to proceed label Jul 8, 2024

neiljp force-pushed the category/1516-narrowing/pr branch from 8f3dd49 to 70032a5 Compare July 8, 2024 04:49

neiljp added this to the Next Release milestone Jul 8, 2024

neiljp added the PR ready to be merged PR has been reviewed & is ready to be merged label Jul 8, 2024

neiljp merged commit 66c74ba into zulip:main Jul 8, 2024
21 checks passed

Niloth-p deleted the category/1516-narrowing/pr branch July 8, 2024 10:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Group narrowing commands and improve descriptions #1516

Group narrowing commands and improve descriptions #1516

Niloth-p commented Jun 7, 2024

rsashank Jun 16, 2024 •

edited

Loading

Niloth-p Jun 16, 2024

neiljp Jun 23, 2024

neiljp Jun 23, 2024

neiljp left a comment

neiljp Jun 23, 2024

neiljp Jun 23, 2024

neiljp Jun 23, 2024

neiljp Jun 23, 2024

neiljp Jun 23, 2024

Niloth-p commented Jun 24, 2024 •

edited

Loading

zormit left a comment •

edited

Loading

Niloth-p commented Jul 7, 2024 •

edited

Loading

neiljp commented Jul 7, 2024

Niloth-p commented Jul 7, 2024

neiljp commented Jul 8, 2024

neiljp commented Jul 8, 2024

Group narrowing commands and improve descriptions #1516

Group narrowing commands and improve descriptions #1516

Conversation

Niloth-p commented Jun 7, 2024

What does this PR do, and why?

Outstanding aspect(s)

External discussion & connections

How did you test this?

Self-review checklist for each commit

Visual changes

rsashank Jun 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neiljp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Niloth-p commented Jun 24, 2024 • edited Loading

1st commit

2nd commit

Follow-ups to address in separate commits / PRs:

zormit left a comment • edited Loading

Choose a reason for hiding this comment

Niloth-p commented Jul 7, 2024 • edited Loading

neiljp commented Jul 7, 2024

Niloth-p commented Jul 7, 2024

neiljp commented Jul 8, 2024

neiljp commented Jul 8, 2024

rsashank Jun 16, 2024 •

edited

Loading

Niloth-p commented Jun 24, 2024 •

edited

Loading

zormit left a comment •

edited

Loading

Niloth-p commented Jul 7, 2024 •

edited

Loading