Skip to content

Fix #1506 : Improve handling of narrows (views) with no messages #1580

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 22 additions & 38 deletions zulipterminal/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
from zulipterminal.platform_code import notify
from zulipterminal.ui_tools.utils import create_msg_box_list

from zulipterminal.ui_tools.messages import PlaceholderMessageBox

class ServerConnectionFailure(Exception):
pass
Expand Down Expand Up @@ -1665,39 +1666,20 @@ def notify_user(self, message: Message) -> str:
text,
)
return ""

def _handle_message_event(self, event: Event) -> None:
"""
Handle new messages (eg. add message to the end of the view)
"""
assert event["type"] == "message"
message = self.modernize_message_response(event["message"])
# sometimes `flags` are missing in `event` so initialize
# an empty list of flags in that case.
message["flags"] = event.get("flags", [])
# We need to update the topic order in index, unconditionally.
if message["type"] == "stream":
# NOTE: The subsequent helper only updates the topic index based
# on the message event not the UI (the UI is updated in a
# consecutive block independently). However, it is critical to keep
# the topics index synchronized as it used whenever the topics list
# view is reconstructed later.
self._update_topic_index(message["stream_id"], message["subject"])
# If the topic view is toggled for incoming message's
# recipient stream, then we re-arrange topic buttons
# with most recent at the top.
if hasattr(self.controller, "view"):
view = self.controller.view
if view.left_panel.is_in_topic_view_with_stream_id(
message["stream_id"]
):
if view.left_panel.is_in_topic_view_with_stream_id(message["stream_id"]):
view.topic_w.update_topics_list(
message["stream_id"], message["subject"], message["sender_id"]
)
self.controller.update_screen()

# We can notify user regardless of whether UI is rendered or not,
# but depend upon the UI to indicate failures.
failed_command = self.notify_user(message)
if (
failed_command
Expand All @@ -1720,31 +1702,33 @@ def _handle_message_event(self, event: Event) -> None:
self.controller.update_screen()
self._notified_user_of_notification_failure = True

# Index messages before calling set_count.
self.index = index_messages([message], self, self.index)
if "read" not in message["flags"]:
set_count([message["id"]], self.controller, 1)

if hasattr(self.controller, "view") and self._have_last_message.get(
repr(self.narrow), False
):
if hasattr(self.controller, "view"):
msg_log = self.controller.view.message_view.log
if msg_log:
last_message = msg_log[-1].original_widget.message
else:
last_message = None
msg_w_list = create_msg_box_list(
self, [message["id"]], last_message=last_message
)
if not msg_w_list:
return
else:
msg_w = msg_w_list[0]

# Assume we have the latest message if this is a new message we sent
narrow_str = repr(self.narrow)
if self.current_narrow_contains_message(message):
msg_log.append(msg_w)
self._have_last_message[narrow_str] = True

self.controller.update_screen()
if self._have_last_message.get(narrow_str, False):
last_message = msg_log[-1].original_widget.message if msg_log else None
msg_w_list = create_msg_box_list(
self, [message["id"]], last_message=last_message
)
if not msg_w_list:
return
msg_w = msg_w_list[0]

if self.current_narrow_contains_message(message):
# Clear placeholder if present
if msg_log and isinstance(msg_log[0].original_widget, PlaceholderMessageBox):
msg_log.clear()
msg_log.append(msg_w)
self.controller.view.message_view.set_focus(len(msg_log) - 1)
self.controller.update_screen()

def _update_topic_index(self, stream_id: int, topic_name: str) -> None:
"""
Expand Down
32 changes: 31 additions & 1 deletion zulipterminal/ui_tools/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ class _MessageEditState(NamedTuple):
message_id: int
old_topic: str


class MessageBox(urwid.Pile):
# type of last_message is Optional[Message], but needs refactoring
def __init__(self, message: Message, model: "Model", last_message: Any) -> None:
Expand Down Expand Up @@ -1189,3 +1188,34 @@ def keypress(self, size: urwid_Size, key: str) -> Optional[str]:
elif is_command_key("MSG_SENDER_INFO", key):
self.model.controller.show_msg_sender_info(self.message["sender_id"])
return key

class PlaceholderMessageBox(urwid.WidgetWrap):
def __init__(self, text: str) -> None:
self.message = {"id": -1} # So it doesn’t explode when accessed
text_widget = urwid.Text(text)
self.original_widget = self # Trickery: mimic MessageBox

super().__init__(text_widget)

def recipient_header(self) -> Any:
return urwid.Text("") # Blank for empty narrow

def top_search_bar(self) -> Any:
return urwid.Text("") # Stub for compatibility

def update_message_author_status(self) -> None:
pass # Stub for compatibility

def keypress(self, size: Tuple[int, int], key: str) -> None:
if is_command_key("GO_DOWN", key):
return None
if is_command_key("GO_UP", key):
return None

elif is_command_key("GO_LEFT", key):
self.view.show_left_panel(visible=True)
return None

elif is_command_key("GO_RIGHT", key):
self.view.show_right_panel(visible=True)
return None
24 changes: 15 additions & 9 deletions zulipterminal/ui_tools/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
import urwid

from zulipterminal.api_types import Message
from zulipterminal.ui_tools.messages import MessageBox

from zulipterminal.ui_tools.messages import MessageBox,PlaceholderMessageBox
from typing import List, Any

def create_msg_box_list(
model: Any,
Expand All @@ -23,7 +23,14 @@ def create_msg_box_list(
if not model.narrow and messages is None:
messages = list(model.index["all_msg_ids"])
if messages is not None:
message_list = [model.index["messages"][id] for id in messages]
message_list = [model.index["messages"][id] for id in messages if id in model.index["messages"]]
else:
message_list = []
if not message_list:
placeholder = urwid.AttrMap(PlaceholderMessageBox("No messages here"), None, "msg_selected")
model.set_focus_in_current_narrow(0)
return [placeholder]

message_list.sort(key=lambda msg: msg["timestamp"])
w_list = []
focus_msg = None
Expand All @@ -32,30 +39,29 @@ def create_msg_box_list(
for msg in message_list:
if is_unsubscribed_message(msg, model):
continue
# Remove messages of muted topics / streams.
if is_muted(msg, model):
muted_msgs += 1
if model.narrow == []: # Don't show in 'All messages'.
continue
msg_flag: Optional[str] = "unread"
flags = msg.get("flags")
# update_messages sends messages with no flags
# but flags are set to [] when fetching old messages.
if flags and ("read" in flags):
msg_flag = None
elif focus_msg is None:
elif (focus_msg is None) and (last_msg is None): # type: ignore[redundant-expr]
focus_msg = message_list.index(msg) - muted_msgs
if msg["id"] == focus_msg_id:
focus_msg = message_list.index(msg) - muted_msgs
# Skip invalid last_msg from placeholder
if last_msg and "type" not in last_msg:
last_msg = None
w_list.append(
urwid.AttrMap(MessageBox(msg, model, last_msg), msg_flag, "msg_selected")
)
last_msg = msg
if focus_msg is not None:
model.set_focus_in_current_narrow(focus_msg)
return w_list


return w_list
# The SIM114 warnings are ignored here since combining the branches would be less clear
def is_muted(msg: Message, model: Any) -> bool:
# PMs cannot be muted
Expand Down
Loading
Loading