Add button to view @ mentions. #433

sumanthvrao · 2019-07-16T12:56:38Z

This PR adds support to view @ mentioned messages. Any feedback regarding this is welcomed.

Solves #425

amanagr · 2019-07-16T17:39:59Z

One thing I noticed is that if there are two message of the same narrow (say #zt>some topic), and if I narrow using one of them via S/z from the @mention narrow to some topic and come back to @mention narrow and select the other msg of the same narrow (some topic) and press z, it's focus is still set on the previous message. This might come as a separate PR btw.

sumanthvrao · 2019-07-19T01:10:33Z

@amanagr Not sure why, But I'm not able to reproduce this. How are you coming back to the @mention narrow? (By selecting it from the left sidebar?).

My steps:

Went to @mention narrow and clicked on #checkin>sumanth which has multiple mentioned messages. I clicked on the top and used z, which zoomed into Checkin stream
I again went to @mention narrow choosing from side bar and did the same on the other message. Which took me to the correct focus.

Am I doing anything wrong?

amanagr · 2019-07-19T11:17:05Z

I am also unable to reproduce it now :|

sumanthvrao · 2019-07-20T15:29:29Z

@neiljp FYI. Updated this with tests. Its now ready for a review.

sumanthvrao · 2019-09-06T00:53:02Z

@neiljp Resolved conflicts and rebased this. Do you have time to give it a review?

sumanthvrao · 2020-03-02T02:47:32Z

This PR has be split up as :

The first 3 commits adds the necessary support functions involving the new 'mentioned messages' narrow; These involve - create narrow, get id's in narrow and show all messages in the narrow. Tests have been added in the respective commits as well.
The next 3 commits are UI related. They help register the new hotkey (#) , connect it via keypress and change the search bar text as well.
The 7th and 8th commit - create the UI button for the 'mentioned messages' and connect it via the UI.
The last commit classifies the unread messages so that the UI button can be marked with them.

I have separated the commits which involve the scenario when a mentioned message is received after ZT has started. I have kept these in a separate branch and will work on these shortly.

@neiljp @amanagr

neiljp

@sumanthvrao Thanks for looking at this again - it may just be 3 fewer commits, or the reworking you did, but this feels good 🎉
I think if the first 6 commits (by git, not per github!) can be polished then these will allow a good initial partial merge, allowing the narrowing to work with # (maybe with that squash mentioned). Then the button code and dynamic updates can be integrated separately.

Hopefully the comments are clear, but as always let me know here and/or on czo.

zulipterminal/core.py

tests/ui/test_ui_tools.py

zulipterminal/ui_tools/boxes.py

README.md

zulipterminal/config/keys.py

zulipterminal/ui.py

zulipterminal/ui_tools/buttons.py

zulipterminal/ui_tools/views.py

sumanthvrao · 2020-03-04T01:38:20Z

@neiljp Thanks for the review. I have updated the PR with the changes requested.

neiljp

@sumanthvrao Other than the possible extra comment note and Narrow to in the README/help_text, I'd like to merge the first 5 commits ASAP. You have a different git name for one of those commits though - if you want to maintain the original name, we can do that before merging those 5 too.

I've left a small note re the button spacing; there may be a few other issues, but I've run out of time to look further right now.

zulipterminal/core.py

zulipterminal/ui_tools/buttons.py

sumanthvrao · 2020-03-05T14:49:27Z

~~@neiljp What do you want the README/help text to say. I think Messages in which you're mentioned seems good.~~

The new commit message was because I squashed 2 commits from the previous version. I have updated the commit heading to be the first one of the squashed commits.

~~I didn't exactly get what changes you wanted wrt button spacing.~~

Besides this, if there are no more changes, let's get it merged ASAP :)

neiljp · 2020-03-07T05:06:30Z

@sumanthvrao The first 5 are merged; now onto more UI :) 🎉

I think the other work is updating while ZT is running, and possibly the all-mentions aspect?

sumanthvrao · 2020-03-07T05:16:20Z

Thanks @neiljp !

If user is in 'mentioned' view and a new mentioned message arrives, we need to append it to current view. Tests added.

On startup, we classify any unreads which belong to mentioned stream so that we can mark the MentionedButton with the correct mentioned count. Tests added.

Tests amended.

kaustubh-nair

LGTM!
I've tested this locally and everything works great for me.
Also, the commits look concise and well organized. 👍

sumanthvrao · 2020-03-18T14:39:41Z

Thanks for the review @kaustubh-nair! Glad to see there are no errors. :)

neiljp · 2020-04-09T07:05:58Z

@sumanthvrao I just merged the rest of these commits manually, after slightly adjusting the last one to fit with the recent changes to set_count - working well so far, thanks! 🎉

Some commits are quite small, but we put in a lot of related effort before this - though some extra tests may be warranted?

preetmishra · 2020-04-09T07:21:20Z

@sumanthvrao Awesome work! This looks great. 👍

sumanthvrao · 2020-04-10T00:47:36Z

@neiljp Thanks for taking time to review this and I'm glag to finally get it merged.

Thanks @preetmishra :)

zulipbot added the size: L [Automatic label added by zulipbot] label Jul 16, 2019

sumanthvrao force-pushed the zt-add-mentions-buttons branch from 860611d to 4ceebf5 Compare July 20, 2019 03:47

zulipbot added size: XL [Automatic label added by zulipbot] and removed size: L [Automatic label added by zulipbot] labels Jul 20, 2019

sumanthvrao force-pushed the zt-add-mentions-buttons branch from 4ceebf5 to b732385 Compare July 20, 2019 15:28

sumanthvrao changed the title ~~[WIP] Add button to view @ mentions.~~ Add button to view @ mentions. Jul 20, 2019

sumanthvrao force-pushed the zt-add-mentions-buttons branch from b732385 to 2c51ab0 Compare September 6, 2019 00:52

sumanthvrao requested a review from neiljp September 6, 2019 00:54

sumanthvrao force-pushed the zt-add-mentions-buttons branch from 2c51ab0 to 32cc6e0 Compare January 11, 2020 01:05

sumanthvrao force-pushed the zt-add-mentions-buttons branch 6 times, most recently from 785f430 to f6e5301 Compare March 2, 2020 02:44

neiljp reviewed Mar 3, 2020

View reviewed changes

sumanthvrao force-pushed the zt-add-mentions-buttons branch from f6e5301 to 618b3fb Compare March 4, 2020 01:37

neiljp reviewed Mar 5, 2020

View reviewed changes

zulipterminal/core.py Outdated Show resolved Hide resolved

zulipterminal/ui_tools/buttons.py Outdated Show resolved Hide resolved

sumanthvrao force-pushed the zt-add-mentions-buttons branch 4 times, most recently from 5a52631 to 993cf2a Compare March 7, 2020 04:22

sumanthvrao force-pushed the zt-add-mentions-buttons branch from 993cf2a to eb0a253 Compare March 7, 2020 05:22

zulipbot added size: M [Automatic label added by zulipbot] and removed size: XL [Automatic label added by zulipbot] labels Mar 7, 2020

sumanthvrao added 5 commits March 8, 2020 12:08

model: Append mentioned messages to current view.

04dfaa3

If user is in 'mentioned' view and a new mentioned message arrives, we need to append it to current view. Tests added.

helper: Classify unread counts as belonging to mentioned narrow.

517f12e

On startup, we classify any unreads which belong to mentioned stream so that we can mark the MentionedButton with the correct mentioned count. Tests added.

buttons: Add MentionedButton Class for displaying mentioned messages.

e353b47

views: Display and connect '@' Mentions button in LeftColumnView.

5e921fc

Tests amended.

helper: Update unread counts for mentioned messages in UI and in index.

d839729

sumanthvrao force-pushed the zt-add-mentions-buttons branch from eb0a253 to d839729 Compare March 8, 2020 06:39

zulipbot added size: L [Automatic label added by zulipbot] and removed size: M [Automatic label added by zulipbot] labels Mar 8, 2020

neiljp added the PR needs review PR requires feedback to proceed label Mar 9, 2020

kaustubh-nair reviewed Mar 18, 2020

View reviewed changes

neiljp closed this Apr 9, 2020

neiljp mentioned this pull request Apr 9, 2020

Add button to indicate mentions (@theloggedinuser) #425

Closed

sumanthvrao deleted the zt-add-mentions-buttons branch April 10, 2020 00:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add button to view @ mentions. #433

Add button to view @ mentions. #433

sumanthvrao commented Jul 16, 2019 •

edited

Loading

amanagr commented Jul 16, 2019

sumanthvrao commented Jul 19, 2019

amanagr commented Jul 19, 2019

sumanthvrao commented Jul 20, 2019

sumanthvrao commented Sep 6, 2019

sumanthvrao commented Mar 2, 2020

neiljp left a comment

sumanthvrao commented Mar 4, 2020

neiljp left a comment

sumanthvrao commented Mar 5, 2020 •

edited

Loading

neiljp commented Mar 7, 2020

sumanthvrao commented Mar 7, 2020

kaustubh-nair left a comment

sumanthvrao commented Mar 18, 2020

neiljp commented Apr 9, 2020

preetmishra commented Apr 9, 2020

sumanthvrao commented Apr 10, 2020

Add button to view @ mentions. #433

Add button to view @ mentions. #433

Conversation

sumanthvrao commented Jul 16, 2019 • edited Loading

amanagr commented Jul 16, 2019

sumanthvrao commented Jul 19, 2019

amanagr commented Jul 19, 2019

sumanthvrao commented Jul 20, 2019

sumanthvrao commented Sep 6, 2019

sumanthvrao commented Mar 2, 2020

neiljp left a comment

Choose a reason for hiding this comment

sumanthvrao commented Mar 4, 2020

neiljp left a comment

Choose a reason for hiding this comment

sumanthvrao commented Mar 5, 2020 • edited Loading

neiljp commented Mar 7, 2020

sumanthvrao commented Mar 7, 2020

kaustubh-nair left a comment

Choose a reason for hiding this comment

sumanthvrao commented Mar 18, 2020

neiljp commented Apr 9, 2020

preetmishra commented Apr 9, 2020

sumanthvrao commented Apr 10, 2020

sumanthvrao commented Jul 16, 2019 •

edited

Loading

sumanthvrao commented Mar 5, 2020 •

edited

Loading