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

Hide spoiler content in notifications #1173

Merged
merged 4 commits into from
Mar 21, 2022

Conversation

neiljp
Copy link
Collaborator

@neiljp neiljp commented Mar 20, 2022

What does this PR do?

This is complementary to #1061, which is aimed at the message content and accessing the hidden content. #688 doesn't discuss the specifics, but hiding in notifications is one part of this.

This PR migrates the html processing from the platform code into the model, where beautiful soup is used to translate and extract content accordingly.

Prior to this PR, spoiler text in a message gives rise to extra lines of text and doesn't hide the spoiler content. The updated output is intended to match the style from the web app, which translates a spoiler with a title and content into title (...) in a notification.

Tested?

  • Manually
  • Existing tests (adapted, if necessary)
  • New tests added (for any new behavior)
  • Passed linting & tests (each commit)

Notes & Questions

This provides good infrastructure for a follow-up set of work to explore how different markdown is rendered in notifications and improve it if necessary.

@neiljp neiljp added this to the Next Release milestone Mar 20, 2022
@zulipbot zulipbot added the size: L [Automatic label added by zulipbot] label Mar 20, 2022
@neiljp neiljp force-pushed the 2022-03-18-hide-spoilers-in-notifications branch from c48b596 to b2f7d74 Compare March 21, 2022 00:55
@neiljp neiljp force-pushed the 2022-03-18-hide-spoilers-in-notifications branch from b2f7d74 to 244ca3a Compare March 21, 2022 01:47
@neiljp neiljp merged commit cdbff55 into zulip:main Mar 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: L [Automatic label added by zulipbot]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants