Skip to content
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.

Sign up for GitHub

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

Group message data #1537

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Group message data #1537

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
15a7c86
views: Use `self` to re-use instance variables in MsgInfoView class.
Niloth-p Jul 22, 2024
c734d2a
core/views: Stack inner popups, add support for exiting all popups.
Niloth-p Jul 22, 2024
e316327
views/core: Delete all message info popup parameters from inner popups.
Niloth-p Jul 22, 2024
9e5036e
views: Delete the calling arguments from MsgInfoView's show_* calls.
Niloth-p Jul 22, 2024
9051d4f
refactor: views/helper/core/messages: Group message info in a TypedDict.
Niloth-p Jul 22, 2024
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
18 changes: 0 additions & 18 deletions tests/ui_tools/test_popups.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,19 +503,13 @@ def mock_external_classes(self, mocker: MockerFixture, msg_box: MessageBox) -> N
self.full_rendered_message = FullRenderedMsgView(
controller=self.controller,
message=self.message,
topic_links=OrderedDict(),
message_links=OrderedDict(),
time_mentions=list(),
Comment on lines -506 to -508
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Removing dead/unnecessary code is a refactor - it shouldn't change the behavior :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, thank you!

title="Full Rendered Message",
)

def test_init(self, msg_box: MessageBox) -> None:
assert self.full_rendered_message.title == "Full Rendered Message"
assert self.full_rendered_message.controller == self.controller
assert self.full_rendered_message.message == self.message
assert self.full_rendered_message.topic_links == OrderedDict()
assert self.full_rendered_message.message_links == OrderedDict()
assert self.full_rendered_message.time_mentions == list()
assert self.full_rendered_message.header.widget_list == msg_box.header
assert self.full_rendered_message.footer.widget_list == msg_box.footer

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re the commit text for the 'stack' change, my only nit would be re the end of the 'why stack popups' that, we don't need to pass the entire data around... unless we need it :) None of these now do, but as per discussion there are upcoming PRs that will do so.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, is that so? I can't seem to recall that discussion.
The change to the commit text sounds better anyways.
Glad to know that the rest is fine.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've not rechecked, but was likely referring to Sashank's comment here, where he is expecting this will be necessary for spoilers.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's covered by this PR. Won't need to pass around any data for that.

Expand Down Expand Up @@ -574,19 +568,13 @@ def mock_external_classes(self, mocker: MockerFixture, msg_box: MessageBox) -> N
self.full_raw_message = FullRawMsgView(
controller=self.controller,
message=self.message,
topic_links=OrderedDict(),
message_links=OrderedDict(),
time_mentions=list(),
title="Full Raw Message",
)

def test_init(self, msg_box: MessageBox) -> None:
assert self.full_raw_message.title == "Full Raw Message"
assert self.full_raw_message.controller == self.controller
assert self.full_raw_message.message == self.message
assert self.full_raw_message.topic_links == OrderedDict()
assert self.full_raw_message.message_links == OrderedDict()
assert self.full_raw_message.time_mentions == list()
assert self.full_raw_message.header.widget_list == msg_box.header
assert self.full_raw_message.footer.widget_list == msg_box.footer

Expand Down Expand Up @@ -644,18 +632,12 @@ def mock_external_classes(self, mocker: MockerFixture) -> None:
self.edit_history_view = EditHistoryView(
controller=self.controller,
message=self.message,
topic_links=OrderedDict(),
message_links=OrderedDict(),
time_mentions=list(),
title="Edit History",
)

def test_init(self) -> None:
assert self.edit_history_view.controller == self.controller
assert self.edit_history_view.message == self.message
assert self.edit_history_view.topic_links == OrderedDict()
assert self.edit_history_view.message_links == OrderedDict()
assert self.edit_history_view.time_mentions == list()
self.controller.model.fetch_message_history.assert_called_once_with(
message_id=self.message["id"],
)
Expand Down
50 changes: 6 additions & 44 deletions zulipterminal/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,61 +348,23 @@ def show_msg_sender_info(self, user_id: int) -> None:
"area:user",
)

def show_full_rendered_message(
self,
message: Message,
topic_links: Dict[str, Tuple[str, int, bool]],
message_links: Dict[str, Tuple[str, int, bool]],
time_mentions: List[Tuple[str, str]],
) -> None:
def show_full_rendered_message(self, message: Message) -> None:
self.show_pop_up(
FullRenderedMsgView(
self,
message,
topic_links,
message_links,
time_mentions,
f"Full rendered message {SCROLL_PROMPT}",
self, message, f"Full rendered message {SCROLL_PROMPT}"
),
"area:msg",
)

def show_full_raw_message(
self,
message: Message,
topic_links: Dict[str, Tuple[str, int, bool]],
message_links: Dict[str, Tuple[str, int, bool]],
time_mentions: List[Tuple[str, str]],
) -> None:
def show_full_raw_message(self, message: Message) -> None:
self.show_pop_up(
FullRawMsgView(
self,
message,
topic_links,
message_links,
time_mentions,
f"Full raw message {SCROLL_PROMPT}",
),
FullRawMsgView(self, message, f"Full raw message {SCROLL_PROMPT}"),
"area:msg",
)

def show_edit_history(
self,
message: Message,
topic_links: Dict[str, Tuple[str, int, bool]],
message_links: Dict[str, Tuple[str, int, bool]],
time_mentions: List[Tuple[str, str]],
) -> None:
def show_edit_history(self, message: Message) -> None:
self.show_pop_up(
EditHistoryView(
self,
message,
topic_links,
message_links,
time_mentions,
f"Edit History {SCROLL_PROMPT}",
),
"area:msg",
EditHistoryView(self, message, f"Edit History {SCROLL_PROMPT}"), "area:msg"
)

def open_in_browser(self, url: str) -> None:
Expand Down
18 changes: 0 additions & 18 deletions zulipterminal/ui_tools/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1798,16 +1798,10 @@ def __init__(
self,
controller: Any,
message: Message,
topic_links: Dict[str, Tuple[str, int, bool]],
message_links: Dict[str, Tuple[str, int, bool]],
time_mentions: List[Tuple[str, str]],
title: str,
) -> None:
self.controller = controller
self.message = message
self.topic_links = topic_links
self.message_links = message_links
self.time_mentions = time_mentions
width = 64
widgets: List[Any] = []

Expand Down Expand Up @@ -1912,16 +1906,10 @@ def __init__(
self,
controller: Any,
message: Message,
topic_links: Dict[str, Tuple[str, int, bool]],
message_links: Dict[str, Tuple[str, int, bool]],
time_mentions: List[Tuple[str, str]],
title: str,
) -> None:
self.controller = controller
self.message = message
self.topic_links = topic_links
self.message_links = message_links
self.time_mentions = time_mentions
max_cols, max_rows = controller.maximum_popup_dimensions()

# Get rendered message
Expand All @@ -1944,16 +1932,10 @@ def __init__(
self,
controller: Any,
message: Message,
topic_links: Dict[str, Tuple[str, int, bool]],
message_links: Dict[str, Tuple[str, int, bool]],
time_mentions: List[Tuple[str, str]],
title: str,
) -> None:
self.controller = controller
self.message = message
self.topic_links = topic_links
self.message_links = message_links
self.time_mentions = time_mentions
max_cols, max_rows = controller.maximum_popup_dimensions()

# Get rendered message header and footer
Expand Down