Views/BuddyView: Introduce BuddyView to show Recent PMs and Group PMs. #315

amanagr · 2019-02-26T09:23:39Z

No description provided.

amanagr · 2019-02-28T10:44:02Z

This is a preview of how buddy list looks now. This is mostly functional and is ready for a spin if anyone wants to try. I would love any feedback on this.

I will add tests for this soon and then reorder some of the commits.

amanagr · 2019-03-04T11:28:14Z

@neiljp I have added minimal tests for the functions and this could possibly serve as a v1 for BuddyList. Since this PR is already getting too big, I will add some feature improvements I have in mind along with more robust tests in future PRs. Thanks!

neiljp

I don't have much time to do a complete review as this is quite large and mixed, but I've left some thoughts on the first and third, in order to give feedback sooner rather than later.

While I'm unsure right now if the current UI is the approach to go, I do like that this builds UI elements that we could use wherever we want; I hope the inheritance helps?

neiljp · 2019-03-10T01:16:54Z

tests/ui/test_ui_tools.py

@@ -1244,6 +1244,7 @@ def test_text_content(self, mocker,
                count_str)
        assert len(text[0]) == len(expected_text) == (width - 1)
        assert text[0] == expected_text
+        assert top_button._w._original_widget._wrap_mode == 'any'


Why is wrapping on any character better than wrapping on spaces?

Primarily, to keep computation of showing unread count at the right position simple. Otherwise, we will have to take wrapping of words into account which is currently not included in the code.

I can see that this could confuse the placement of the unread counts. That said, the styling of the words looks pretty bad if the text wraps mid-word.

I would put this 'why' into the commit title/summary in any case.

Of course! done 👍

neiljp · 2019-03-10T06:45:54Z

zulipterminal/helper.py

+        user_ids = frozenset(map(int, user_ids))
+        unread_counts['unread_pms'][user_ids] = count
+        unread_counts['all_msg'] += count
+        unread_counts['all_pms'] += count


The rest of this code builds infrastructure for testing classify_unread_counts - which is great :)

However, I would suggest putting the above additions and the changes to the tests required for it from this commit in a separate commit.

Both of those commits would then be fine in another PR?

neiljp · 2019-03-10T06:49:50Z

tests/conftest.py

@@ -350,11 +350,112 @@ def initial_data(logged_on_user):
            'stream_weekly_traffic': 0
        }],
        'unread_msgs': {
-            'pms': [],
-            'count': 0,
+            'pms': [{


I like the addition of this extra data to the tests - but this is a lot. Is this real data you've captured? Can we trim it to base data and then parametrize to look at some cases? This looks like a lot, and while having real data is good, it makes it more difficult to understand?

amanagr · 2019-03-11T09:55:36Z

Thanks @neiljp for the review I will get to them by the end of this week as I am busy with exams.

amanagr · 2019-03-16T12:13:08Z

I have split the commit as you suggested. As for the fixtures, I can curb them down but I feel they can be useful later as they provide the base for all the group_pm and unread_count related fixtures which were previously missing.

amanagr · 2019-03-23T05:07:48Z

@neiljp is this ready to merge?

amanagr · 2019-05-15T11:22:59Z

@punchagan @neiljp I have rebased this PR against master. Let me know your views on this.

Don't space wrap the last word if it doesn't fit in the column. Instead fill part of word in one row and rest in the next row. This is done to keep computation of showing unread count at the right position simple. Otherwise, we will have to take wrapping of words into account which is currently not included in the code.

Store PM recipients in index as dict of frozenset(recipients) and Timestamp to be used in BuddyView.

Add tests and fixtures for testing classify_unread_counts.

Get unread_counts for group pms stored as 'huddles' and store them in unread_msg_counts. Add tests for classify_unread_counts along with appropriate fixtures.

This is based on TopButton except that it has multiple recipients. Add tests to check correctly rendered caption.

This includes following features: * `b` keypress toggling of BuddyView * Shows Recent PMs and Group PMs * Shows corresponding Unread Counts. * Shows Online/Idle/Offline status of users/Group PMs. core/keys: Use 'b' to toggle BuddyView. Fixes zulip#220

Allow caption to overflow to next line if `shrink=False`.

codecov · 2019-07-22T12:52:16Z

Codecov Report

Merging #315 into master will decrease coverage by 1.96%.
The diff coverage is 64.81%.

@@            Coverage Diff             @@
##           master     #315      +/-   ##
==========================================
- Coverage   70.38%   68.41%   -1.97%     
==========================================
  Files          12       12              
  Lines        2195     1979     -216     
  Branches      479      440      -39     
==========================================
- Hits         1545     1354     -191     
  Misses        524      524              
+ Partials      126      101      -25

Impacted Files	Coverage Δ
zulipterminal/config/keys.py	`100% <ø> (ø)`	⬆️
zulipterminal/ui.py	`71.87% <0%> (-3.69%)`	⬇️
zulipterminal/core.py	`49.26% <30%> (-1.21%)`	⬇️
zulipterminal/ui_tools/views.py	`82.37% <66.07%> (-0.4%)`	⬇️
zulipterminal/helper.py	`62.75% <66.66%> (-2.81%)`	⬇️
zulipterminal/ui_tools/buttons.py	`75.22% <85.71%> (-6.3%)`	⬇️
zulipterminal/ui_tools/boxes.py	`53.63% <0%> (-5.73%)`	⬇️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b31c8b...aaf5ee1. Read the comment docs.

zulipbot · 2020-04-17T00:46:22Z

Heads up @amanagr, 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/master branch and resolve your pull request's merge conflicts accordingly.

zulipbot added the size: L [Automatic label added by zulipbot] label Feb 26, 2019

amanagr force-pushed the buddy_list branch from 3744725 to b2d9f74 Compare February 27, 2019 18:27

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

amanagr force-pushed the buddy_list branch 4 times, most recently from 8a633f9 to e266118 Compare February 28, 2019 10:37

amanagr force-pushed the buddy_list branch 4 times, most recently from a178b10 to a9e7f95 Compare March 3, 2019 02:57

amanagr changed the title ~~[WIP] Buddy list~~ Views/BuddyView: Introduce BuddyView to show Recent PMs and Group PMs. Mar 3, 2019

amanagr force-pushed the buddy_list branch 4 times, most recently from 678065f to 2447562 Compare March 3, 2019 04:04

This comment has been minimized.

Sign in to view

amanagr force-pushed the buddy_list branch 2 times, most recently from ea20770 to 5cea361 Compare March 4, 2019 11:09

amanagr requested a review from neiljp March 4, 2019 11:28

amanagr force-pushed the buddy_list branch 2 times, most recently from 0162d7a to dc1a267 Compare March 6, 2019 15:06

neiljp reviewed Mar 10, 2019

View reviewed changes

amanagr force-pushed the buddy_list branch from dc1a267 to 6d3672d Compare March 16, 2019 08:45

amanagr force-pushed the buddy_list branch from 6d3672d to 691399a Compare March 17, 2019 02:40

amanagr requested a review from neiljp April 8, 2019 03:53

amanagr force-pushed the buddy_list branch 2 times, most recently from 4e937a5 to be41830 Compare May 15, 2019 11:14

sumanthvrao mentioned this pull request May 26, 2019

model: Fix append message event for PMs. #365

Merged

amanagr force-pushed the buddy_list branch 3 times, most recently from a71259b to 4a95601 Compare June 6, 2019 04:11

sumanthvrao mentioned this pull request Jun 12, 2019

Add tests for 'classify_unread_count' #383

Closed

amanagr added 8 commits July 19, 2019 17:22

helper/index_messages: Store all the PM recipients in index.

f913b0f

Store PM recipients in index as dict of frozenset(recipients) and Timestamp to be used in BuddyView.

test_classify_unread_counts: Add tests for classify_unread_counts.

c0cf3bc

Add tests and fixtures for testing classify_unread_counts.

classify_unread_counts: Store unread of count of GroupPMs.

afbdd9b

Get unread_counts for group pms stored as 'huddles' and store them in unread_msg_counts. Add tests for classify_unread_counts along with appropriate fixtures.

unread_count: Set correct count for the group_pms.

b832ca1

GroupPMButton: Introduce Button to show GroupPMs.

f457841

This is based on TopButton except that it has multiple recipients. Add tests to check correctly rendered caption.

buttons/TopButton: Don't shrink text in case of group pms.

aaf5ee1

Allow caption to overflow to next line if `shrink=False`.

amanagr force-pushed the buddy_list branch from 4a95601 to aaf5ee1 Compare July 22, 2019 12:52

zulipbot added the has conflicts label Apr 17, 2020

Base automatically changed from master to main January 30, 2021 20:30

neiljp force-pushed the main branch from f9f483a to 793e73d Compare December 15, 2021 21:30

neiljp mentioned this pull request Apr 13, 2022

Add Private Messages side panel #1197

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Views/BuddyView: Introduce BuddyView to show Recent PMs and Group PMs. #315

Views/BuddyView: Introduce BuddyView to show Recent PMs and Group PMs. #315

amanagr commented Feb 26, 2019

amanagr commented Feb 28, 2019

This comment has been minimized.

amanagr commented Mar 4, 2019

neiljp left a comment

neiljp Mar 10, 2019

amanagr Mar 16, 2019

neiljp Mar 16, 2019

amanagr Mar 17, 2019

neiljp Mar 10, 2019

neiljp Mar 10, 2019

amanagr commented Mar 11, 2019

amanagr commented Mar 16, 2019

amanagr commented Mar 23, 2019

amanagr commented May 15, 2019

codecov bot commented Jul 22, 2019

zulipbot commented Apr 17, 2020

Views/BuddyView: Introduce BuddyView to show Recent PMs and Group PMs. #315

Are you sure you want to change the base?

Views/BuddyView: Introduce BuddyView to show Recent PMs and Group PMs. #315

Conversation

amanagr commented Feb 26, 2019

amanagr commented Feb 28, 2019

This comment has been minimized.

amanagr commented Mar 4, 2019

neiljp left a comment

Choose a reason for hiding this comment

neiljp Mar 10, 2019

Choose a reason for hiding this comment

amanagr Mar 16, 2019

Choose a reason for hiding this comment

neiljp Mar 16, 2019

Choose a reason for hiding this comment

amanagr Mar 17, 2019

Choose a reason for hiding this comment

neiljp Mar 10, 2019

Choose a reason for hiding this comment

neiljp Mar 10, 2019

Choose a reason for hiding this comment

amanagr commented Mar 11, 2019

amanagr commented Mar 16, 2019

amanagr commented Mar 23, 2019

amanagr commented May 15, 2019

codecov bot commented Jul 22, 2019

Codecov Report

zulipbot commented Apr 17, 2020