zulip
recent_topics: Add Private message to recent_topics.
We add Private message to the Recent topics to make it an all-encompassing overview of everything that's happening for a user.
Fixes #19449
@amanagr I have made most of the changes. I'm facing problem with keyboard navigation.
@amanagr I have update the commit with private tests as well, can you please have a look at it?
@madrix01 did you lose all your changes 😅 ? I only see some relevant code here. git reflog can help you recover those changes.
@amanagr no, they are all present in this commit as time asked me to break the commit into small changes, so it would be easy to review the code and add tests. Discussed here, all the changes you have suggested are present in the above mentioned commit.
@amanagr It was the part of the pr. I rebased this pr, as i was to break single commit into small commits.
So, did you open a PR for the rest of the code? I should be able to test the complete thing. :) It fine to just push remaining commits in this PR too if you haven't done so.
@amanagr @timabbott apologies for the delay. I have update all the code here including the ui.
@madrix01 you'll need to debug the node tests for this before we can merge it.
@timabbott I have tweaked the tests current tests, they work with full coverage, but special tests for private messages are yet to be added.
This needs a rebase for our recent topics PRs merged recently, but should otherwise be good to test-deploy on chat.zulip.org.
Also, is the first commit something we could merge independently of the second one to reduce the size of this PR and thus merge conflict risk? I think we might need some checks to prevent trying to display those without type="stream" in order to do that.
@timabbott
Also, is the first commit something we could merge independently of the second one to reduce the size of this PR and thus merge conflict risk? I think we might need some checks to prevent trying to display those without
type="stream"in order to do that.
Yes this commit will only allow message with all the type to pass in process_messages() in recent_topics_data.js but PMs will not be rendered in the UI, there is a check for filtering out messages with only type === "stream"
I just pulled down the PR, and it's not quite ready for test deployment. I'll post my comments here.
The icons (envelope and people) are too dark in light mode; I think they'd look good matching the background of the unread counts.
In dark mode, let's try having the icons match the colors for the envelope/people icons in the left sidebar.
Otherwise looks good so far; I can test more when the error is no longer popping up. Thanks!
The unread counts need to be aligned properly:
Absence of mark as read and mute topic icon was causing this UI bugs, I have added margin to the right to match the unread in the topics.
I'm also getting this error if I click around:
Can you show me the steps to generate this error?
Hm, I'm not getting the error anymore, so maybe my dev environment somehow got into a weird state. Could you push your other fixes? Thanks!
@alya @timabbott I have pushed the following changes
- made pm row unread counts align with topic row
- fixed bugs with keyboard navigation as there are only 2 columns with PM row and 4 with columns row
- added function to differentiate between Pm and topic row.
Will add more ui changes soon.
Thank you!
Icons in light mode
Icons in dark mode
@alya I have updated the PR with the changes you suggested can you please have a look at it? Thank you !!
@madrix01 Could you please go through the manual review checklist at https://zulip.readthedocs.io/en/latest/contributing/code-reviewing.html#manual-testing, as there are a lot of opportunities for UI glitches on this one?
Ok, I'll fix that, Are you facing any bug or glitches with the keyboard navigation ?