Migrate UI & Docs from Private to Direct phrasing

[theViz343]

What does this PR do?
This PR replaces instances of 'private' with 'direct', in the context of private messages, in the UI and relevant docs.
This is in line with the design decision to rename private message (PM) to direct message (DM).

Fixes #1288

CZO Discussion Link

Tested?

• Manually
• Existing tests (adapted, if necessary)
• New tests added (for any new behavior)
• Passed linting & tests (each commit)

Commit flow

• First, second and fourth commits change UI elements from 'private' to 'direct'.
• Third and fifth commits update the documentation.

Notes & Questions

• Some UI changes like the keybinding help texts in the second commit do not result in test failures. Do we need to add tests for them?

[zulipbot]

Hello @zulip/server-hotkeys members, this pull request was labeled with the "area: hotkeys" label, so you may want to check it out!

[neiljp]

@theViz343 Thanks for looking into this 👍 Is there anything else visible to change?

I let a few comments inline. More generally, your commit message structure looks fine. However, there's no need to reference the issue in every commit, only the last one, and it can be on the final review - see the related notices that it gave in the issue already :)

[mounilKshah]

This PR looks good to me at first glance. Apart from Neil's suggestions, I have left a comment for a minor modification.

As for hotkeys.md and developer-file-overview.md, as Neil has mentioned, it is better to have them in separate commits later, as they are not updated manually, instead, scripts are used for this purpose.

[zulipbot]

Hello @theViz343, it seems like you have referenced #1288 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 #1288..

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.

[theViz343]

I've made the changes requested. I've also added the test for the PM button text. It it looks good, I can add similar tests for the other sidebar buttons.
For the failing linting check, I'm going to squash the docstring change commit and the doc change commit that follows just after it.

[neiljp]

@theViz343 Mainly just feedback on the commit structure:

• Please do squash those two commits together; it's not going to pass CI (or tools/check-branch) otherwise, and one change is directly associated with the other. If it's just the one doc file, you can refer to it more specifically as 'hotkeys' in the commit title prefix.

• For the title shortening in developer-file-overview, that's not a rename, so it can be in a separate commit. "It also" suggests it's worthy of being separate, and it makes it clearer to tell what's happening from the commit titles. It would also be good as a commit directly before the buttons/docs commit, as you have now but more explicit in the title text.

• If we're introducing a new test, it's good to keep it as a standalone commit before the associated code is changed. For your previous iteration of the code change, such a test would have hopefully both passed for the existing code, and then failed with the new version. Ordering the commits like that ensures that's the case.

• Reviewing the commits again, the README change is a change to a 'prose' document (ie. not generated), so since small would fit with the general docs/ commit.

Re the test, this seems like two or more tests which can be split fairly easily:

• The expected text, and maybe hotkey for this button; I say maybe since I'm not sure if we gain from specifying the hotkey, since it comes from the hotkeys dynamically
• Ensuring a style for the buttons generally, in terms of the elements of the regex. While a regex is fine, it doesn't tell you how the text is incorrect, or which one is wrong, if it fails multiple elements. Separate checks for the elements you list in the regex comment would individually fail and show the assert condition, telling you specifically what was wrong.

For this PR let's leave it for PMButton only, to focus on the transition, though based on how the latter tests would be near-identical for other buttons, the code similarity also suggests we could refactor at least some of those buttons into one class.

[theViz343]

The expected text, and maybe hotkey for this button; I say maybe since I'm not sure if we gain from specifying the hotkey, since it comes from the hotkeys dynamically

I agree with the hotkey not being useful to check here. Checking just the text should be fine.

Ensuring a style for the buttons generally, in terms of the elements of the regex. While a regex is fine, it doesn't tell you how the text is incorrect, or which one is wrong, if it fails multiple elements. Separate checks for the elements you list in the regex comment would individually fail and show the assert condition, telling you specifically what was wrong.

So, I think checking the length of the button_text (since it remains the same) should be sufficient, apart from checking the expected text. This will catch the most common errors, I suppose.

I've also moved the commit with the notifications change before any doc change commit to keep UI and doc changes separate.

[neiljp]

@theViz343 Thanks for the slight tidy 👍 I rephrased the commit text slightly, but otherwise this is as you wrote it, and I'll merge it now - thanks! 🎉