-
Notifications
You must be signed in to change notification settings - Fork 204
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
[CIS-42] Markdown support for messages #2067
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love the tests 😍 just a few questions
Sources/StreamChatUI/ChatMessageList/ChatMessage/ChatMessageContentView.swift
Outdated
Show resolved
Hide resolved
Sources/StreamChatUI/ChatMessageList/ChatMessage/ChatMessageContentView.swift
Outdated
Show resolved
Hide resolved
Sources/StreamChatUI/ChatMessageList/ChatMessage/ChatMessageContentView.swift
Outdated
Show resolved
Hide resolved
⬆️ This change increases install size by 130.4 kB ⬆️🗂 See the Emerge size breakdown
🔎 See the full analysis (557ab61) merging into develop (ace2a6d)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is good to include documentation for new features. This is needed for customers + it makes development easier in general (easier to review and develop + when we release docs are already on the site).
Good to run (or mention in the PR that you did) UI performance tests on this feature. Even a manual test on large channels and/or channel list would be great.
Some stuff is actually very easy to test in code, you can bench tests containsMarkdown
and format
functions very easily with self.measure {}
Sources/StreamChatUI/StreamSwiftyMarkdown/SwiftyMarkdown/CharacterRule.swift
Show resolved
Hide resolved
Tests/StreamChatUITests/Utils/DefaultMarkdownFormatter_Tests.swift
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few comments related to docs + API
Kudos, SonarCloud Quality Gate passed! |
🔗 Issue Links
https://stream-io.atlassian.net/browse/CIS-42
🎯 Goal
Add Markdown support for messages.
📝 Summary
🛠 Implementation
🎨 Showcase
performance tests for
containsMarkdown
andformat
methods:🧪 Manual Testing Notes
Markdown support is enabled by default.
☑️ Contributor Checklist
🎁 Meme