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.

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
1 change: 1 addition & 0 deletions tests/ui_tools/test_messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -726,6 +726,7 @@ def test_soup2markup(self, content, expected_markup, mocker):
server_url=SERVER_URL,
message_links=OrderedDict(),
time_mentions=list(),
spoilers=list(),
bq_len=0,
)

Expand Down
60 changes: 52 additions & 8 deletions zulipterminal/ui_tools/messages.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ def __init__(self, message: Message, model: "Model", last_message: Any) -> None:
self.message_links: Dict[str, Tuple[str, int, bool]] = dict()
self.topic_links: Dict[str, Tuple[str, int, 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
# if this is the first message
if self.last_message is None:
Expand Down Expand Up @@ -371,12 +372,22 @@ def footlinks_view(
@classmethod
def soup2markup(
cls, soup: Any, metadata: Dict[str, Any], **state: Any
) -> Tuple[List[Any], Dict[str, Tuple[str, int, bool]], List[Tuple[str, str]]]:
) -> Tuple[
List[Any],
Dict[str, Tuple[str, int, bool]],
List[Tuple[str, str]],
List[Tuple[int, List[Any], List[Any]]],
]:
# Ensure a string is provided, in case the soup finds none
# This could occur if eg. an image is removed or not shown
markup: List[Union[str, Tuple[Optional[str], Any]]] = [""]
if soup is None: # This is not iterable, so return promptly
return markup, metadata["message_links"], metadata["time_mentions"]
return (
markup,
metadata["message_links"],
metadata["time_mentions"],
metadata["spoilers"],
)
unrendered_tags = { # In pairs of 'tag_name': 'text'
# TODO: Some of these could be implemented
"br": "", # No indicator of absence
Expand Down Expand Up @@ -647,6 +658,10 @@ def soup2markup(
part[1] if isinstance(part, tuple) else part
for part in processed_header
)
header_len = sum(
len(part[1]) if isinstance(part, tuple) else len(part)
for part in processed_header
)

# Limit to the first 10 characters and append "..."
if len(processed_header_text) > 10:
Expand All @@ -671,9 +686,33 @@ def soup2markup(
]
)
markup.extend(bottom_border)
# Spoiler content
content = element.find(class_="spoiler-content")

# Remove surrounding newlines.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's change the variable name?
This code seems to make several non-trivial and effective assumptions about the data being passed to it through spoiler-content. Personally, I think improving the readability of this block can help.

  • Spoiler contents without headers - use the default heading 'Spoiler' which looks fine.
  • Spoiler headers without content - show empty spoiler content - should these be considered spoilers?
    But that's also in the same ballpark as allowing empty spoiler messages, so it's consistent.

content_contents = content.contents
if len(content_contents) > 2:
if content_contents[-1] == "\n":
content.contents.pop(-1)
if content_contents[0] == "\n":
content.contents.pop(0)
if len(content_contents) == 1 and content_contents[0] == "\n":
content.contents.pop(0)

# FIXME: Do not soup2markup content in the MessageBox as it
Copy link
Collaborator

Choose a reason for hiding this comment

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

The comment seems outdated.

# will render 'sensitive' spoiler anchor tags in the footlinks.
processed_content = cls.soup2markup(content, metadata)[0]
metadata["spoilers"].append(
(header_len, processed_header, processed_content)
)
else:
markup.extend(cls.soup2markup(element, metadata)[0])
return markup, metadata["message_links"], metadata["time_mentions"]
return (
markup,
metadata["message_links"],
metadata["time_mentions"],
metadata["spoilers"],
)

def main_view(self) -> List[Any]:
# Recipient Header
Expand Down Expand Up @@ -769,9 +808,12 @@ def main_view(self) -> List[Any]:
)

# Transform raw message content into markup (As needed by urwid.Text)
content, self.message_links, self.time_mentions = self.transform_content(
self.message["content"], self.model.server_url
)
(
content,
self.message_links,
self.time_mentions,
self.spoilers,
) = self.transform_content(self.message["content"], self.model.server_url)
self.content.set_text(content)

if self.message["id"] in self.model.index["edited_messages"]:
Expand Down Expand Up @@ -858,6 +900,7 @@ def transform_content(
Tuple[None, Any],
Dict[str, Tuple[str, int, bool]],
List[Tuple[str, str]],
List[Tuple[int, List[Any], List[Any]]],
]:
soup = BeautifulSoup(content, "lxml")
body = soup.find(name="body")
Expand All @@ -866,13 +909,14 @@ def transform_content(
server_url=server_url,
message_links=dict(),
time_mentions=list(),
spoilers=list(),
) # type: Dict[str, Any]

if isinstance(body, Tag) and body.find(name="blockquote"):
metadata["bq_len"] = cls.indent_quoted_content(soup, QUOTED_TEXT_MARKER)

markup, message_links, time_mentions = cls.soup2markup(body, metadata)
return (None, markup), message_links, time_mentions
markup, message_links, time_mentions, spoilers = cls.soup2markup(body, metadata)
return (None, markup), message_links, time_mentions, spoilers

@staticmethod
def indent_quoted_content(soup: Any, padding_char: str) -> int:
Expand Down
2 changes: 1 addition & 1 deletion zulipterminal/ui_tools/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1421,7 +1421,7 @@ def __init__(self, controller: Any, stream_id: int) -> None:

title = f"{stream_marker} {stream['name']}"
rendered_desc = stream["rendered_description"]
self.markup_desc, message_links, _ = MessageBox.transform_content(
self.markup_desc, message_links, *_ = MessageBox.transform_content(
rendered_desc,
self.controller.model.server_url,
)
Expand Down