zulip-terminal
zulip-terminal copied to clipboard
zulip
Displays unread message counts of muted streams and topics.
This PR displays unread message count of muted streams and topics. Names of muted streams and topics are displayed in a different color instead of 'M'.
Fixes #209
I have to do 2 separate commits for dimming muted streams and displaying counts right?
@manojkestur That would be preferable, as we can always then pick just one commit to apply easily, separate from the other. If commits are earlier in a branch then they are less likely to have dependencies upon your other commits and easier to 'cherry pick' separately.
@manojkestur I'm not sure if you're ready for a review of this yet, but from a quick scan I can see you have a few unrelated changes in commits, and you replaced the M in one commit by an empty string only to replace that in the next commit (it'd be cleaner to leave it as 'M' and change it in the second).
Re the mute symbol, it looks a little strange here, particularly with a smaller font, though I've not found a better unicode symbol. Did you look at many? Are you available to discuss on chat.zulip.org some time?
@neiljp I thought only dimming of muted streams and topic without M was needed. Now i will change that to M. I saw same unicode symbols with different styling but when i pasted it, it's taking basic unicode symbol only. Yes, i am available to discuss on chat.zulip.org.
@neiljp I have done the suggested changes.Let me know if any changes required.
@neiljp I have resolved the conflicts and have done the required changes.
@manojkestur Thanks for rebasing and updating the last commit :+1:
I do like the possibility of this remaining change, though as I indicated when we discussed it in the stream, I also like the idea of not being distracted by whatever the counts are in those muted streams. Having the option of being like the webapp does appear useful in any case, though I've started a discussion in #general to better understand the motivation in the webapp.
This commit does seem to render OK at first glance, but doesn't respond to muting events properly? I also don't understand at first reading why you are adjusting the helper code. The internal data should be being updated properly, but please let us know if you've found a bug in the logic there - though we should keep that different from changing how the UI is styled.
The key methods - at least for streams - are mark_muted and mark_unmuted. Can we just update those to use the update_coiunt method, perhaps with an optional parameter for the muted style, the same as we have for the text color?
@neiljp I have checked the muted events multiple times it is working fine.
When topic is muted and stream is not muted, unread messages were not added to all messages, but when we mute the stream where muted topic resides, all messages count was reducing. To fix this bug I have done little changes in model.py and helper.py.
Unread count was not updated when stream was muted so I changed that in helper.py.
I have kept unread_count_color as parameter in update_count
Heads up @manojkestur, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.
