about_popup: Terminal size added in about popup.

[Swarnim114]

closes #1536

What does this PR do, and why?

Adds a Terminal size label under the "Detected Environment " in About popup

External discussion & connections

• Discussed in #zulip-terminal in topic
• Fully fixes Add current terminal window size to About popup #1536
• 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

[Swarnim114]

Hi @neiljp
Can you please review this PR?

[neiljp]

@Swarnim114 Thanks for working on this! Welcome to the project!

Visually this looks broadly like I was expecting, though I'd maybe add spaces around the the x to make it easier to read the numbers. If you saw this popup, do you think you'd want to know which was columns vs rows?

In terms of the code, it would be cleaner to pass the actual size directly, to avoid the view (and readers of the code) needing to know about the screen. The popup contents don't update dynamically while it is open right now.

Thanks for looking at tests, and your current way of keeping them as a separate commit is great for now, though we generally squash minimal code and test changes into one commit in a branch before merging. Note that I don't mean all commits, rather that code additions/changes are kept together with matching tests :)

For the next iteration it would be good to get all the checks passing.

[Swarnim114]

@neiljp

I apologize for the issues with my recent pull request.

1. I was unable to get the "Current terminal size update dynamically" feature to work as intended.
2. One of the checks failed due to a small mistake on my part.

Please let me know if there are any further changes you would like me to make.

[neiljp]

@Swarnim114 Thanks for making the updates 👍

To be clear: I was not expecting a dynamic update of size from you, rather meaning that the lack of that meant that the window size could be passed in :)

The current one remaining failing test refers to isolated commits, which is happening since in at least some cases, one of your commits is fixing a problem in an earlier one. See the README about this; you can use check-branch to see what is happening locally.

For merging it would be useful to have

• all the commits squashed together (simplifying your test issue above)
• fixing the points I raised inline
• write a good single commit title and text, matching the Zulip-Terminal project style.

[neiljp]

Please check your PRs for these unrelated changes.

[neiljp]

This new code is not necessary:

• please look up what mocker/mock/Mock() is used for, if you intend to use it :)
• self.about_view is set automatically in the first function (fixture) of this test class, which is used automatically every time, so this is duplicated each time.

[neiljp]

@Swarnim114 If you intend to contribute further to Zulip, note that you can use @zulipbot to set/unset the 'PR awaiting update' and 'PR needs review' tags, which can help us with identifying the state of PRs :)