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

[CIS-42] Markdown support for messages #2067

Merged
merged 10 commits into from
Jun 16, 2022
Merged

Conversation

hugobernalstream
Copy link
Contributor

@hugobernalstream hugobernalstream commented Jun 7, 2022

🔗 Issue Links

https://stream-io.atlassian.net/browse/CIS-42

🎯 Goal

Add Markdown support for messages.

📝 Summary

  • Add Markdown formatting for messages with proper Markdown syntax.
  • Add a feature flag to enable/disable Markdown support.

🛠 Implementation

  • Implement an Adapter over the SwiftyMarkdown library for Markdown support.
  • Add a Regex validation to check for Markdown syntax before formatting the text.

🎨 Showcase

Screen Shot 2022-06-06 at 22 26 22

Screen Shot 2022-06-06 at 22 26 40

performance tests for containsMarkdown and format methods:

Screen Shot 2022-06-12 at 19 10 12
Screen Shot 2022-06-12 at 19 11 00

🧪 Manual Testing Notes

Markdown support is enabled by default.

☑️ Contributor Checklist

  • I have signed the Stream CLA (required)
  • Changelog is updated with client-facing changes
  • New code is covered by unit tests
  • This change follows zero ⚠️ policy (required)
  • Comparison screenshots added for visual changes
  • Affected documentation updated (docusaurus, tutorial, CMS)

🎁 Meme

Bold

@hugobernalstream hugobernalstream requested a review from a team as a code owner June 7, 2022 03:46
@hugobernalstream hugobernalstream self-assigned this Jun 7, 2022
@Stream-SDK-Bot
Copy link
Collaborator

Stream-SDK-Bot commented Jun 7, 2022

1 Error
🚫 Please start subject with capital letter.
9551990
1 Warning
⚠️ Big PR

Generated by 🚫 Danger

Copy link
Contributor

@adamrushy adamrushy left a 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

@emerge-tools
Copy link

emerge-tools bot commented Jun 7, 2022

⬆️ This change increases install size by 130.4 kB ⬆️

Image of diff

🗂 See the Emerge size breakdown
Item Install size
➕ StreamChatUI.framework/StreamChatUI.SwiftyScanner.closeTag(withGroupID) ⬆️ 6.3 kB
➕ StreamChatUI.framework/StreamChatUI.SwiftyScanner.scanNonRepeatingTags ⬆️ 5.2 kB
➕ StreamChatUI.framework/StreamChatUI.SwiftyMarkdown.init(string) ⬆️ 4.2 kB
➕ StreamChatUI.framework/StreamChatUI.SwiftyScanner.scanRepeatingTags ⬆️ 3.9 kB
➕ StreamChatUI.framework/StreamChatUI.SwiftyTokeniser.process ⬆️ 3.8 kB
➕ StreamChatUI.framework/StreamChatUI.SwiftyMarkdown.attributedString(from) ⬆️ 3.8 kB
StreamChatUI.framework/Code Signature ⬆️ 3.1 kB
➕ StreamChatUI.framework/StreamChatUI.SwiftyScanner.scan ⬆️ 2.4 kB
➕ StreamChatUI.framework/StreamChatUI.SwiftyLineProcessor.processLineLevelAttributes ⬆️ 2.0 kB
➕ StreamChatUI.framework/StreamChatUI.SwiftyLineProcessor.processFrontMatter ⬆️ 1.8 kB
StreamChatUI.framework/Strings.Unmapped ⬆️ 1.8 kB
➕ StreamChatUI.framework/StreamChatUI.SwiftyMarkdown.font(for,characterOverride) ⬆️ 1.8 kB
➕ StreamChatUI.framework/StreamChatUI.SwiftyLineProcessor.process ⬆️ 1.8 kB
➕ StreamChatUI.framework/__C._dictionaryUpCast ⬆️ 1.7 kB
➕ StreamChatUI.framework/StreamChatUI.SwiftyTokeniser.init(with) ⬆️ 1.7 kB
➕ StreamChatUI.framework/StreamChatUI.SwiftyScanner.range(for) ⬆️ 1.7 kB
➕ StreamChatUI.framework/StreamChatUI.SwiftyMarkdown.Objc Metadata ⬆️ 1.6 kB
StreamChatUI.framework/DYLD.Lazy Bind ⬆️ 1.4 kB
StreamChatUI.framework/DYLD.Exports ⬆️ 1.3 kB
StreamChatUI.framework/Swift._ArrayBufferProtocol.replaceSubrange(with,elementsOf) ⬆️ 1.2 kB

🔎 See the full analysis (557ab61) merging into develop (ace2a6d)

Provide a baseSha and pullRequestNumber with your upload to ensure diffs are correct. Read more in the docs

Copy link
Member

@tbarbugli tbarbugli left a 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 {}

Copy link
Contributor

@evsaev evsaev left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@bielikb bielikb left a 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

docusaurus/docs/iOS/uikit/data-formatting.md Outdated Show resolved Hide resolved
docusaurus/docs/iOS/uikit/data-formatting.md Show resolved Hide resolved
docusaurus/docs/iOS/uikit/data-formatting.md Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Jun 16, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@bielikb bielikb merged commit e635ab7 into develop Jun 16, 2022
@bielikb bielikb deleted the feature/CIS-42-Markdown branch June 16, 2022 20:27
@adolfogarza adolfogarza mentioned this pull request Jun 20, 2022
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.

7 participants