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

Make notifications spoiler aware #4426

Conversation

OpaqueReptile
Copy link
Contributor

@OpaqueReptile OpaqueReptile commented Nov 7, 2021

An attempt to implement matrix-org/matrix-spec-proposals#3124, and address #3477

Signed-off-by: Preston Frazier @preston:inferiorlattice.com

Pull Request Checklist

Screenshot_1636317629 Screenshot_1636317604
image Screenshot_1636383586

@bmarty
Copy link
Member

bmarty commented Nov 8, 2021

Hello @paftree, thanks for the PR.
Can you add a screenshot of a notification with a spoiler to demo your change?

@OpaqueReptile
Copy link
Contributor Author

Added more screenshots above.

@ouchadam
Copy link
Contributor

ouchadam commented Nov 9, 2021

for my own understanding, does this change only hide spoilers in the direct message/room list and notifications? (all the places that use getLastMessageBody)

The spoilers will still be visible within the message timeline?

@OpaqueReptile
Copy link
Contributor Author

Right, the current behavior is using the unformatted message body in the room list/notifications where most clients will just put the spoiler verbatim... which somewhat defeats the point of having spoiler text at all.

This change attempts to replace any spoiler text with fullblock characters in non-html-formatted places. The actual message in the room will still be hidden/viewable by the data-mx-spoiler attribute.

@bmarty bmarty changed the base branch from develop to feature/bma/markdown_roomlist November 16, 2021 16:28
@bmarty bmarty merged commit 4ce361e into element-hq:feature/bma/markdown_roomlist Nov 16, 2021
@bmarty
Copy link
Member

bmarty commented Nov 16, 2021

Merge on one of my branch, will create a new PR soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants