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

Render Spoilers in ZT #1529

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

Render Spoilers in ZT #1529

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7d0b85c
boxes/messages: Add support for spoiler header markup in MessageBox.
rsashank Jun 28, 2024
555dfdd
messages/views: Extract spoiler content in a variable in MessageBox.
rsashank Jun 28, 2024
6d11f72
core/views: Add SpoilerView class and corresponding show_* function.
rsashank Jun 30, 2024
a577cef
core/messages/views: Pass spoiler data to MsgInfoView popup helpers.
rsashank Jul 1, 2024
5ebab61
buttons: Add SpoilerButton class showing spoiler header.
rsashank Jul 1, 2024
7469e69
views: Show SpoilerButton in message info view if present.
rsashank Jul 1, 2024
909f042
buttons/core/views: Pass message info data to spoiler classes.
rsashank Jul 2, 2024
8c13265
messages: Hide footlink for links in spoiler blocks.
rsashank Jul 11, 2024
6f7fbdf
buttons/core/messages/view: Add spoiler_link to metadata.
rsashank Jul 11, 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
4 changes: 2 additions & 2 deletions tests/ui_tools/test_buttons.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,8 @@ def spoiler_button(
header: List[Any] = [""],
content: List[Any] = [""],
message: Message = {},
topic_links: Dict[str, Tuple[str, int, bool]] = {},
message_links: Dict[str, Tuple[str, int, bool]] = {},
topic_links: Dict[str, Tuple[str, int, bool, bool]] = {},
message_links: Dict[str, Tuple[str, int, bool, bool]] = {},
time_mentions: List[Tuple[str, str]] = [],
spoilers: List[Tuple[int, List[Any], List[Any]]] = [],
display_attr: Optional[str] = None,
Expand Down
25 changes: 14 additions & 11 deletions tests/ui_tools/test_messages.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1881,7 +1881,7 @@ def test_reactions_view(
[
(
"https://github.com/zulip/zulip-terminal/pull/1",
("#T1", 1, True),
("#T1", 1, True, False),
),
]
),
Expand All @@ -1893,8 +1893,8 @@ def test_reactions_view(
case(
OrderedDict(
[
("https://foo.com", ("Foo!", 1, True)),
("https://bar.com", ("Bar!", 2, True)),
("https://foo.com", ("Foo!", 1, True, False)),
("https://bar.com", ("Bar!", 2, True, False)),
]
),
"1: https://foo.com\n2: https://bar.com",
Expand All @@ -1913,8 +1913,11 @@ def test_reactions_view(
case(
OrderedDict(
[
("https://example.com", ("https://example.com", 1, False)),
("http://example.com", ("http://example.com", 2, False)),
(
"https://example.com",
("https://example.com", 1, False, False),
),
("http://example.com", ("http://example.com", 2, False, False)),
]
),
None,
Expand All @@ -1925,8 +1928,8 @@ def test_reactions_view(
case(
OrderedDict(
[
("https://foo.com", ("https://foo.com, Text", 1, True)),
("https://bar.com", ("Text, https://bar.com", 2, True)),
("https://foo.com", ("https://foo.com, Text", 1, True, False)),
("https://bar.com", ("Text, https://bar.com", 2, True, False)),
]
),
"1: https://foo.com\n2: https://bar.com",
Expand All @@ -1945,9 +1948,9 @@ def test_reactions_view(
case(
OrderedDict(
[
("https://foo.com", ("Foo!", 1, True)),
("http://example.com", ("example.com", 2, False)),
("https://bar.com", ("Bar!", 3, True)),
("https://foo.com", ("Foo!", 1, True, False)),
("http://example.com", ("example.com", 2, False, False)),
("https://bar.com", ("Bar!", 3, True, False)),
]
),
"1: https://foo.com\n3: https://bar.com",
Expand Down Expand Up @@ -1994,7 +1997,7 @@ def test_footlinks_view(
def test_footlinks_limit(self, maximum_footlinks, expected_instance):
message_links = OrderedDict(
[
("https://github.com/zulip/zulip-terminal", ("ZT", 1, True)),
("https://github.com/zulip/zulip-terminal", ("ZT", 1, True, False)),
]
)

Expand Down
18 changes: 9 additions & 9 deletions tests/ui_tools/test_popups.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,9 +1054,9 @@ def test_init(self, message_fixture: Message) -> None:
assert self.msg_info_view.time_mentions == list()
assert self.msg_info_view.spoilers == list()

def test_pop_up_info_order(self, message_fixture: Message) -> None:
topic_links = OrderedDict([("https://bar.com", ("topic", 1, True))])
message_links = OrderedDict([("image.jpg", ("image", 1, True))])
def test_popup_info_order(self, message_fixture: Message) -> None:
topic_links = OrderedDict([("https://bar.com", ("topic", 1, True, False))])
message_links = OrderedDict([("image.jpg", ("image", 1, True, False))])
msg_info_view = MsgInfoView(
self.controller,
message_fixture,
Expand Down Expand Up @@ -1302,14 +1302,14 @@ def test_height_reactions(
],
[
(
OrderedDict([("https://bar.com", ("Foo", 1, True))]),
OrderedDict([("https://bar.com", ("Foo", 1, True, False))]),
"1: Foo\nhttps://bar.com",
{None: "popup_contrast"},
{None: "selected"},
15,
),
(
OrderedDict([("https://foo.com", ("", 1, True))]),
OrderedDict([("https://foo.com", ("", 1, True, False))]),
"1: https://foo.com",
{None: "popup_contrast"},
{None: "selected"},
Expand All @@ -1323,7 +1323,7 @@ def test_height_reactions(
)
def test_create_link_buttons(
self,
initial_link: "OrderedDict[str, Tuple[str, int, bool]]",
initial_link: "OrderedDict[str, Tuple[str, int, bool, bool]]",
expected_text: str,
expected_attr_map: Dict[None, str],
expected_focus_map: Dict[None, str],
Expand Down Expand Up @@ -1588,8 +1588,8 @@ def test_markup_description(
(
OrderedDict(
[
("https://example.com", ("Example", 1, True)),
("https://generic.com", ("Generic", 2, True)),
("https://example.com", ("Example", 1, True, False)),
("https://generic.com", ("Generic", 2, True, False)),
]
),
"1: https://example.com\n2: https://generic.com",
Expand All @@ -1608,7 +1608,7 @@ def test_markup_description(
)
def test_footlinks(
self,
message_links: "OrderedDict[str, Tuple[str, int, bool]]",
message_links: "OrderedDict[str, Tuple[str, int, bool, bool]]",
expected_text: str,
expected_attrib: List[Tuple[Optional[str], int]],
expected_footlinks_width: int,
Expand Down
20 changes: 10 additions & 10 deletions zulipterminal/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,8 @@ def show_topic_edit_mode(self, button: Any) -> None:
def show_msg_info(
self,
msg: Message,
topic_links: Dict[str, Tuple[str, int, bool]],
message_links: Dict[str, Tuple[str, int, bool]],
topic_links: Dict[str, Tuple[str, int, bool, bool]],
message_links: Dict[str, Tuple[str, int, bool, bool]],
time_mentions: List[Tuple[str, str]],
spoilers: List[Tuple[int, List[Any], List[Any]]],
) -> None:
Expand Down Expand Up @@ -342,8 +342,8 @@ def show_msg_sender_info(self, user_id: int) -> None:
def show_full_rendered_message(
self,
message: Message,
topic_links: Dict[str, Tuple[str, int, bool]],
message_links: Dict[str, Tuple[str, int, bool]],
topic_links: Dict[str, Tuple[str, int, bool, bool]],
message_links: Dict[str, Tuple[str, int, bool, bool]],
time_mentions: List[Tuple[str, str]],
spoilers: List[Tuple[int, List[Any], List[Any]]],
) -> None:
Expand All @@ -363,8 +363,8 @@ def show_full_rendered_message(
def show_full_raw_message(
self,
message: Message,
topic_links: Dict[str, Tuple[str, int, bool]],
message_links: Dict[str, Tuple[str, int, bool]],
topic_links: Dict[str, Tuple[str, int, bool, bool]],
message_links: Dict[str, Tuple[str, int, bool, bool]],
time_mentions: List[Tuple[str, str]],
spoilers: List[Tuple[int, List[Any], List[Any]]],
) -> None:
Expand All @@ -384,8 +384,8 @@ def show_full_raw_message(
def show_edit_history(
self,
message: Message,
topic_links: Dict[str, Tuple[str, int, bool]],
message_links: Dict[str, Tuple[str, int, bool]],
topic_links: Dict[str, Tuple[str, int, bool, bool]],
message_links: Dict[str, Tuple[str, int, bool, bool]],
time_mentions: List[Tuple[str, str]],
spoilers: List[Tuple[int, List[Any], List[Any]]],
) -> None:
Expand Down Expand Up @@ -494,8 +494,8 @@ def show_spoiler(
self,
content: str,
message: Message,
topic_links: Dict[str, Tuple[str, int, bool]],
message_links: Dict[str, Tuple[str, int, bool]],
topic_links: Dict[str, Tuple[str, int, bool, bool]],
message_links: Dict[str, Tuple[str, int, bool, bool]],
time_mentions: List[Tuple[str, str]],
spoilers: List[Tuple[int, List[Any], List[Any]]],
) -> None:
Expand Down
4 changes: 2 additions & 2 deletions zulipterminal/ui_tools/buttons.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,8 @@ def __init__(
header: List[Any],
content: List[Any],
message: Message,
topic_links: Dict[str, Tuple[str, int, bool]],
message_links: Dict[str, Tuple[str, int, bool]],
topic_links: Dict[str, Tuple[str, int, bool, bool]],
message_links: Dict[str, Tuple[str, int, bool, bool]],
time_mentions: List[Tuple[str, str]],
spoilers: List[Tuple[int, List[Any], List[Any]]],
display_attr: Optional[str],
Expand Down
27 changes: 18 additions & 9 deletions zulipterminal/ui_tools/messages.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ def __init__(self, message: Message, model: "Model", last_message: Any) -> None:
self.topic_name = ""
self.email = "" # FIXME: Can we remove this?
self.user_id: Optional[int] = None
self.message_links: Dict[str, Tuple[str, int, bool]] = dict()
self.topic_links: Dict[str, Tuple[str, int, bool]] = dict()
self.message_links: Dict[str, Tuple[str, int, bool, bool]] = dict()
self.topic_links: Dict[str, Tuple[str, int, bool, bool]] = dict()
self.time_mentions: List[Tuple[str, str]] = list()
self.spoilers: List[Tuple[int, List[Any], List[Any]]] = list()
self.last_message = last_message
Expand All @@ -77,6 +77,7 @@ def __init__(self, message: Message, model: "Model", last_message: Any) -> None:
link["text"],
len(self.topic_links) + 1,
True,
False,
)

self.stream_name = self.message["display_recipient"]
Expand Down Expand Up @@ -314,7 +315,7 @@ def reactions_view(

@staticmethod
def footlinks_view(
message_links: Dict[str, Tuple[str, int, bool]],
message_links: Dict[str, Tuple[str, int, bool, bool]],
*,
maximum_footlinks: int,
padded: bool,
Expand All @@ -331,7 +332,7 @@ def footlinks_view(
footlinks = []
counter = 0
footlinks_width = 0
for link, (text, index, show_footlink) in message_links.items():
for link, (text, index, show_footlink, spoiler_link) in message_links.items():
if counter == maximum_footlinks:
break
if not show_footlink:
Expand Down Expand Up @@ -374,7 +375,7 @@ def soup2markup(
cls, soup: Any, metadata: Dict[str, Any], **state: Any
) -> Tuple[
List[Any],
Dict[str, Tuple[str, int, bool]],
Dict[str, Tuple[str, int, bool, bool]],
List[Tuple[str, str]],
List[Tuple[int, List[Any], List[Any]]],
]:
Expand Down Expand Up @@ -503,27 +504,35 @@ def soup2markup(
# Do not show as a footlink as the text is sufficient
# to represent the link.
show_footlink = False

spoiler_link = False
if element.find_parent("div", class_="spoiler-block"):
show_footlink = False
spoiler_link = True

# Detect duplicate links to save screen real estate.
if link not in metadata["message_links"]:
metadata["message_links"][link] = (
text,
len(metadata["message_links"]) + 1,
show_footlink,
spoiler_link,
)
else:
# Append the text if its link already exist with a
# different text.
saved_text, saved_link_index, saved_footlink_status = metadata[
"message_links"
][link]
(
saved_text,
saved_link_index,
saved_footlink_status,
spoiler_link,
) = metadata["message_links"][link]
if saved_text != text:
metadata["message_links"][link] = (
f"{saved_text}, {text}",
saved_link_index,
show_footlink or saved_footlink_status,
spoiler_link,
)

markup.extend(
Expand Down Expand Up @@ -900,7 +909,7 @@ def transform_content(
cls, content: Any, server_url: str
) -> Tuple[
Tuple[None, Any],
Dict[str, Tuple[str, int, bool]],
Dict[str, Tuple[str, int, bool, bool]],
List[Tuple[str, str]],
List[Tuple[int, List[Any], List[Any]]],
]:
Expand Down
28 changes: 16 additions & 12 deletions zulipterminal/ui_tools/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1084,8 +1084,8 @@ def __init__(
title: str,
content: str,
message: Message,
topic_links: Dict[str, Tuple[str, int, bool]],
message_links: Dict[str, Tuple[str, int, bool]],
topic_links: Dict[str, Tuple[str, int, bool, bool]],
message_links: Dict[str, Tuple[str, int, bool, bool]],
time_mentions: List[Tuple[str, str]],
spoilers: List[Tuple[int, List[Any], List[Any]]],
) -> None:
Expand Down Expand Up @@ -1609,8 +1609,8 @@ def __init__(
controller: Any,
msg: Message,
title: str,
topic_links: Dict[str, Tuple[str, int, bool]],
message_links: Dict[str, Tuple[str, int, bool]],
topic_links: Dict[str, Tuple[str, int, bool, bool]],
message_links: Dict[str, Tuple[str, int, bool, bool]],
time_mentions: List[Tuple[str, str]],
spoilers: List[Tuple[int, List[Any], List[Any]]],
) -> None:
Expand Down Expand Up @@ -1748,17 +1748,21 @@ def __init__(

@staticmethod
def create_link_buttons(
controller: Any, links: Dict[str, Tuple[str, int, bool]]
controller: Any, links: Dict[str, Tuple[str, int, bool, bool]]
) -> Tuple[List[MessageLinkButton], int]:
link_widgets = []
link_width = 0

for index, link in enumerate(links):
text, link_index, _ = links[link]
text, link_index, _, spoiler_link = links[link]
if text:
caption = f"{link_index}: {text}\n{link}"
if spoiler_link:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was thinking of several different approaches like prepending the spoiler keyword to the text, then removing it later from text, or using negative integers to communicate spoiler links or such, but yeah, they're all worse solutions than passing around an extra bool value.

With #1537, this should become simplified, as you won't have to add the 'bool' type in so many different places. Even temporarily, you could just create a type and update it in one place.

Also, I only noticed later that most of the bulk comes from the 'bool' type and not the bool value. So, this should be fine after a bit of refactoring.

I didn't look deeply into this, hoping you can just tell me, do both message_links and topic_links necessarily need to be of the same type?

caption = f"{link_index} [spoiler]: {text}\n{link}"
else:
caption = f"{link_index}: {link}"
if spoiler_link:
caption = f"{link_index} [spoiler]: {link}"
link_width = max(link_width, len(max(caption.split("\n"), key=len)))

display_attr = None if index % 2 else "popup_contrast"
Expand Down Expand Up @@ -1884,8 +1888,8 @@ def __init__(
self,
controller: Any,
message: Message,
topic_links: Dict[str, Tuple[str, int, bool]],
message_links: Dict[str, Tuple[str, int, bool]],
topic_links: Dict[str, Tuple[str, int, bool, bool]],
message_links: Dict[str, Tuple[str, int, bool, bool]],
time_mentions: List[Tuple[str, str]],
spoilers: List[Tuple[int, List[Any], List[Any]]],
title: str,
Expand Down Expand Up @@ -2005,8 +2009,8 @@ def __init__(
self,
controller: Any,
message: Message,
topic_links: Dict[str, Tuple[str, int, bool]],
message_links: Dict[str, Tuple[str, int, bool]],
topic_links: Dict[str, Tuple[str, int, bool, bool]],
message_links: Dict[str, Tuple[str, int, bool, bool]],
time_mentions: List[Tuple[str, str]],
spoilers: List[Tuple[int, List[Any], List[Any]]],
title: str,
Expand Down Expand Up @@ -2052,8 +2056,8 @@ def __init__(
self,
controller: Any,
message: Message,
topic_links: Dict[str, Tuple[str, int, bool]],
message_links: Dict[str, Tuple[str, int, bool]],
topic_links: Dict[str, Tuple[str, int, bool, bool]],
message_links: Dict[str, Tuple[str, int, bool, bool]],
time_mentions: List[Tuple[str, str]],
spoilers: List[Tuple[int, List[Any], List[Any]]],
title: str,
Expand Down
Loading