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

Markdown support in notification #184

Merged
merged 5 commits into from
Aug 21, 2021
Merged

Markdown support in notification #184

merged 5 commits into from
Aug 21, 2021

Conversation

Sternagfonkel
Copy link
Contributor

As described in #74 markdown messages should also be renderd inside the android notification if possible.
If not at least the markdown "metadata" should be removed.

Rendering the markdown inside the android notification is basically possible, but tricky. This is because the support for spans in the notification is limited. The spans the Markwon lib generates by default are mostly ignored in notifications, so the span factory must be adapted similar to these examples to fit the android standard:
Markwon Notification Sample
Markwon RemoteView Sample
There's some scope to adapt the rendering to fit the markwon implementation, but basically this means that the rendering result and thus the appearance differ from the current. Also links are not clickable and some other things are ignored, like different text sizes for headings.

And the Markwon library doesn't produce HTML output which could also be used (also limited) in notifications.

So there are three places where the message is shown and markdown could be rendered:

  1. The folded notification: Here also line breaks are ignored, so I show stripped text only here, while limited markdown rendering would also be possible.
    Screenshot_1627843825

  2. The unfolded notification: Line breaks are rendered here, so I render the markdown here, even if it is limited.
    Screenshot_1628092354

  3. The message list inside the app: No changes here. As mentioned above, the rendering result here differs from the limited rendering in the notification.
    Screenshot_1627843876

Is this an acceptable behaviour? As a fallback we can change it and only show the stripped message in the notification, which would look similar to this:
Screenshot_1627843862

Copy link
Member

@jmattheis jmattheis left a comment

Choose a reason for hiding this comment

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

Looks good! Actually better than I imagined.

I've added some sub comments.

Markus added 2 commits August 4, 2021 21:53
Implementation as raw text.
Copy link
Member

@jmattheis jmattheis left a comment

Choose a reason for hiding this comment

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

Great!

@jmattheis jmattheis merged commit 9ba9405 into gotify:master Aug 21, 2021
@Sternagfonkel Sternagfonkel deleted the feature/#74_unrendered-markdown branch August 22, 2021 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants