-
-
Notifications
You must be signed in to change notification settings - Fork 259
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
Create new help categories related to opening compose box #1520
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 Thanks for making this PR!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is nice, thank you. I'm looking forward to having a reduced list of "message actions" (which was too long).
a2cc3e5
to
dd94301
Compare
Rebased, resolving merge conflicts and updating INSERT_NEW_LINE's category along with the rest in the 2nd commit. |
@Niloth-p Thanks for sticking with these improvements - I agree they are definitely helping 👍 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. |
Regroup key bindings that were previously categorized under message actions and the general category. Hotkeys document regenerated.
dd94301
to
b4f977f
Compare
The category name in the code: 'msg_compose' -> 'compose_box' The user visible name: 'Composing a message' -> 'Writing a message' This is to emphasize its difference from the other newly created category for actions that open the compose box, to both users and developers. Hotkeys document regenerated.
b4f977f
to
5b6f02e
Compare
Updates:
Things to check:
|
Regarding my opinion, 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. |
I think small categories can be nice as long as the titles are really really clear. My reason would be a bit different than yours: If I'm looking for a specific command I can just skim the headlines and hopefully quickly find the right category. In that category it then should also be quite fast to identify the command I'd be looking for, because there are not that many present. |
@Niloth-p Thanks for this and your slight update 👍 It's definitely nuanced between the two but let's move on and get this merged for now, and we can always return to examine the category splits later, particularly if we have the section jump keys :) |
What does this PR do, and why?
Groups key bindings from message actions into the new categories:
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
Re-Categorization of Hotkeys
How did you test this?
Self-review checklist for each commit
Visual Changes