[WIP] Add DM panel to the left side view. #1416

theViz343 · 2023-07-07T13:48:43Z

What does this PR do, and why?

This PR is currently preparation for the addition of a collapsible DM panel in the left side UI view.

Outstanding aspect(s)

[ ]

External discussion & connections

Discussed in #zulip-terminal in Private messages sidepanel
Fully fixes Add Private Messages side panel #1197
Partially fixes issue #
Builds upon previous unmerged work in PR #
Is a follow-up to work in PR #
Requires merge of PR #
Merge will enable work on #

How did you test this?

Manually - Behavioral changes
Manually - Visual changes
Adapting existing automated tests
Adding automated tests for new behavior (or missing tests)
Existing automated tests should already cover this (only a refactor of tested code)

Self-review checklist for each commit

It is a minimal coherent idea
It has a commit summary following the documented style (title & body)
It has a commit summary describing the motivation and reasoning for the change
It individually passes linting and tests
It contains test additions for any new behavior
It flows clearly from a previous branch commit, and/or prepares for the next commit

Visual changes

zulipbot · 2023-09-29T01:46:19Z

Heads up @theViz343, we just merged some commits that conflict with the changes you 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.

neiljp

@theViz343 I'd tested this previously for UX feedback, but not looked at the code in depth, until now :)

The DM Panel logic isn't quite complete functionally from what I remember, so I've focused on seeing if we can get some early commits from this PR which prepare for that work merged sooner, which is the first 3 commits.

Running the PR at that point appears to work broadly fine, with the new separate count and the stream and topic lists working as expected.

As per a comment, I'd drop the shortcut key text until the later DM-related commits, otherwise if we do a partial merge it's confusing - S is still set up to do a contextual change of narrow at that point.

Related to that, functionally there is a nit that we shouldn't be narrowing to all messages from the stream messages button - if anything, we should be narrowing to all non-DMs, but that's not implemented. At this point, you could make the button non-selectable instead. That may be relevant for the later commits, since we've not discussed the distinction between using the hotkeys (P,S) to switch panels vs ones to go to those narrows themselves - like P currently does.

In general the commits I looked at are good, but as per some comments, they might benefit from splitting/reordering. I did manage to trigger an exception when running at the first commit, since the button didn't yet exist.

neiljp · 2023-09-29T05:08:30Z

zulipterminal/helper.py

@@ -225,6 +226,7 @@ def _set_count_in_view(
    user_buttons_list = controller.view.user_w.users_btn_list
    all_msg = controller.view.home_button
    all_pm = controller.view.pm_button
+    all_stream = controller.view.stream_button


At this commit, this stream_button doesn't exist.

This set_count-related code likely wants to go after a skeleton UI with no count, so the button is there, and then once hooked up the count can be visible.

neiljp · 2023-09-29T05:30:05Z

tests/helper/test_helper.py

    ],
    ids=[
        "mute_private_stream_mute_general_stream_topic",
        "mute_general_stream_mute_private_stream_topic",
        "no_mute_some_other_stream_muted",
+        "mute_general_stream_topic",


More of a note for later, but this test is not the easiest to validate - but that's historical rather than this commit. I can see this being refactored, but unreads are a complex aspect as it is.

neiljp · 2023-09-29T05:39:22Z

zulipterminal/ui_tools/buttons.py

@@ -142,6 +142,18 @@ def __init__(self, *, controller: Any, count: int) -> None:
        )


+class StreamPanelButton(TopButton):
+    def __init__(self, *, controller: Any, count: int) -> None:
+        button_text = "Stream messages  [S]"


The [S] shouldn't be advertised until that hotkey is available in its new role, which will be much later, when we enable the swapping between the DM and stream panels.

neiljp · 2023-09-29T05:48:32Z

zulipterminal/ui_tools/views.py

+class StreamPanel(urwid.Pile):
+    def __init__(self, submenu_view: List[Any], view: Any) -> None:
+        self.view = view
+        count = self.view.model.unread_counts.get("all_stream_msg", 0)


As per another comment, you need the stream_button first, to handle updates.

So, until the unread count tracking is working, the count can be left at zero; in doing that, the Stream messages will look like a title.

When the tracking is complete, you can then do a small commit, or part of another, which hooks the parts together.

neiljp · 2023-09-29T05:52:00Z

zulipterminal/ui_tools/views.py

+        w = urwid.LineBox(
+            self.view.stream_p,
+            tlcorner=COLUMN_TITLE_BAR_LINE,
+            tline=COLUMN_TITLE_BAR_LINE,
+            trcorner=COLUMN_TITLE_BAR_LINE,


Would it be simpler to make this a Divider in the left column?

neiljp · 2023-09-29T06:03:22Z

zulipterminal/ui_tools/views.py

-            title="Topics",
-            title_attr="column_title",
-            tlcorner=COLUMN_TITLE_BAR_LINE,
-            tline=COLUMN_TITLE_BAR_LINE,
-            trcorner=COLUMN_TITLE_BAR_LINE,


This looks independent of this particular commit, and maybe belongs in the previous commit?

Also, this change essentially sets all the borders to be empty?

This commit reworks the stream view section in the left sidebar by replacing it with the new Stream Panel widget. The stream panel consists of the newly added stream messages button (StreamPanelButton), and the existing stream/topic view section. Test added, updated and renamed.

This commit adds a label parameter to the PanelSearchBox to provide context to the user which searching the streams/topics list. Tests updated.

This commit adds an all_streams key to the UnreadCounts dict to store the unread stream messages count. This is subsequently used by the StreamPanelButton in the stream panel. Test case added and tests updated.

This commit adds a list of recent direct messages for the user to the ZT model (recent_dms). It also adds a _sort_recent_dms function to keep the list in sorted order according to the max_message_id of the dm. Fixture added and test updated.

This commit adds an _update_recent_dms method which updates the recent_dms list when a message event is received. It keeps the list in sorted order as well. Tests modified.

This commit reworks the direct messages button by moving it to a different section between the menu and the streams panel. It also adds a direct messages panel which shows all DM recipients.

theViz343 · 2024-02-09T00:14:11Z

The recent update to this PR addresses most of the earlier feedback.

The commits are now reordered so that the unread count functionality for the Stream Panel Button is added after the button's introduction.

I've removed the S hotkey from the early commits and only introduced it for the Stream Panel Button in the last commit which adds panel folding/unfolding. This might also benefit from some discussion on what to do for the current function of the S key (narrow to stream of the current message).

I've also addressed the focus position issue. Now, the earlier focused position should be focused on when folding/unfolding panels.

I've also added the user presence status to the DMButtons. A thing to note is that a DMButton is quite close in UI and functionality, to the UserButton class, with some differences such as the UserInfoView on pressing i and narrow to user vs opening compose box for user. This may warrant a parent class (still a child class of TopButton ) from which these two classes inherit. This may simplify the code a bit.

There are still a few tests to be added to cover the new code, but any initial testing is welcome :)

zulipbot · 2024-04-17T23:34:50Z

Heads up @theViz343, we just merged some commits that conflict with the changes you 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.

zulipbot added the size: L [Automatic label added by zulipbot] label Jul 7, 2023

theViz343 force-pushed the dm-side-panel-1197 branch from 679c692 to 748cb4a Compare July 13, 2023 07:42

neiljp added enhancement New feature or request area: UI General user interface update labels Jul 24, 2023

theViz343 force-pushed the dm-side-panel-1197 branch 4 times, most recently from 1d41edd to 0739bc6 Compare August 16, 2023 17:55

theViz343 force-pushed the dm-side-panel-1197 branch from 0739bc6 to 8d75162 Compare September 4, 2023 21:10

zulipbot added size: XL [Automatic label added by zulipbot] and removed size: L [Automatic label added by zulipbot] labels Sep 4, 2023

theViz343 force-pushed the dm-side-panel-1197 branch 2 times, most recently from 63f5180 to e36508e Compare September 9, 2023 03:08

zulipbot added the has conflicts label Sep 29, 2023

neiljp reviewed Sep 29, 2023

View reviewed changes

neiljp added the PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback label Sep 29, 2023

theViz343 added 3 commits January 28, 2024 12:58

boxes/views: Add context label parameter to PanelSearchBox.

a5de278

This commit adds a label parameter to the PanelSearchBox to provide context to the user which searching the streams/topics list. Tests updated.

helper/views: Add all_streams unread count to the UnreadCounts dict.

1de25b5

This commit adds an all_streams key to the UnreadCounts dict to store the unread stream messages count. This is subsequently used by the StreamPanelButton in the stream panel. Test case added and tests updated.

theViz343 force-pushed the dm-side-panel-1197 branch 2 times, most recently from 19ee960 to 1f00420 Compare January 31, 2024 21:33

theViz343 added 6 commits February 3, 2024 19:58

model: Update recent_dms while handling a message_event.

5e615c9

This commit adds an _update_recent_dms method which updates the recent_dms list when a message event is received. It keeps the list in sorted order as well. Tests modified.

buttons/views:Add DMButton, DMPanel and DMView classes.

b011799

views: Rework direct messages UI.

9406878

This commit reworks the direct messages button by moving it to a different section between the menu and the streams panel. It also adds a direct messages panel which shows all DM recipients.

buttons/views: Rename PMButton to DMPanelButton.

f090ebf

buttons/views: Add panel folding for DM and stream panels.

76ca074

theViz343 force-pushed the dm-side-panel-1197 branch from 1f00420 to 76ca074 Compare February 5, 2024 03:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Add DM panel to the left side view. #1416

[WIP] Add DM panel to the left side view. #1416

theViz343 commented Jul 7, 2023 •

edited

zulipbot commented Sep 29, 2023

neiljp left a comment

neiljp Sep 29, 2023

neiljp Sep 29, 2023

neiljp Sep 29, 2023

neiljp Sep 29, 2023

neiljp Sep 29, 2023

neiljp Sep 29, 2023

theViz343 commented Feb 9, 2024

zulipbot commented Apr 17, 2024

[WIP] Add DM panel to the left side view. #1416

Are you sure you want to change the base?

[WIP] Add DM panel to the left side view. #1416

Conversation

theViz343 commented Jul 7, 2023 • edited

What does this PR do, and why?

Outstanding aspect(s)

External discussion & connections

How did you test this?

Self-review checklist for each commit

Visual changes

zulipbot commented Sep 29, 2023

neiljp left a comment

Choose a reason for hiding this comment

neiljp Sep 29, 2023

Choose a reason for hiding this comment

neiljp Sep 29, 2023

Choose a reason for hiding this comment

neiljp Sep 29, 2023

Choose a reason for hiding this comment

neiljp Sep 29, 2023

Choose a reason for hiding this comment

neiljp Sep 29, 2023

Choose a reason for hiding this comment

neiljp Sep 29, 2023

Choose a reason for hiding this comment

theViz343 commented Feb 9, 2024

zulipbot commented Apr 17, 2024

theViz343 commented Jul 7, 2023 •

edited