zulip-terminal
zulip-terminal copied to clipboard
zulip
Simplify Topbutton attributes
What does this PR do?
This PR creates TopButton attributes prefix_markup,
label_markup and suffix_markup with Type urwidMarkupTuple.
All of the other attributes can be written in terms of these
which simplifies the logic in various methods ad reads better.
This PR formed after splitting of the last commit of #1064 and is a follow-up to it.
Tested?
- [x] Manually
- [x] Existing tests (adapted, if necessary)
- [x] New tests added (for any new behavior)
- [x] Passed linting & tests (each commit)
Commit flow
Commit 1-3: refactors TopButton attributes. Commit 4: button: Add muted style to stream prefix.
Notes & Questions
The old attributes are kept and deleted in the following commits to keep the diffs cleaner and have separate commits.
I haven't handled the count Exception that happens because of self.count. I realize I shouldn't have removed it.
Visual changes
There is a minor issue I missed. I shouldn't have removed self.count I'll add it back soon.
Hello @zulip/server-refactoring members, this pull request was labeled with the "area: refactoring" label, so you may want to check it out!
@neiljp I've split up the commits even further as suggested. Hope all your review points were addressed as well.
@Rohitth007 I pushed the first few commits independently yesterday as those ending https://github.com/zulip/zulip-terminal/commit/7eb3ca4e2a17a11a8cd8a7267b38c0de9192679c
After pushing an intermediate commit to migrate some obvious urwid_MarkupTuple candidates, I've now looked at the rest of the PR and will push back here to merge, except for the prefix length restriction - we can discuss this later. The muted stream prefix style is a small enough feature that I'm happy to include it here.
Thanks for working on this - this makes it much clearer and cleaner :+1: The structure of changing one interface at a time made it much simpler to review.
