Implemented small change to markdown formatting, and added docs #34
Conversation
One of our markdown consent documents were not rendering properly, hence this. Not sure why Spezi team restricted to inline only - open to feedback!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #34 +/- ##
=======================================
Coverage 79.72% 79.72%
=======================================
Files 41 41
Lines 1242 1242
=======================================
Hits 990 990
Misses 252 252
Continue to review full report in Codecov by Sentry.
|
| /// Example: Data( | ||
| /// " # This is a markdown example | ||
| /// *This should be italiced* and **this bolded**").utf8 |
There was a problem hiding this comment.
If this is a code example, make sure to wrap them into markdown swift code block, as you did for the `TextContent´ type as well.
This makes sure that inline code appears with proper syntax highlighting. Make sure to properly format the code according to your swiftlint code style rules.
|
made some changes based on feedback and after talking to Spezi staff! |
There was a problem hiding this comment.
@auddithio Thank you for the PR; I had a few further comments and suggestions that would need to be addressed before we merge the PR 👍
| @@ -66,6 +66,7 @@ public struct MarkdownView: View { | |||
|
|
|||
|
|
|||
| /// Creates a ``MarkdownView`` that displays the content of a markdown file as an utf8 representation that is loaded asynchronously. | |||
|
|
|||
There was a problem hiding this comment.
Please remove the empty line added here, it breaks the documentation block:
| /// ```swift | ||
| /// // Example | ||
| /// Data(" # This is a markdown example | ||
| /// *This should be italiced* and **this bolded**").utf8 | ||
| /// ``` |
There was a problem hiding this comment.
This code example does not compile, if you want to create a String with line breaks you will have to use a multi-line string interval: https://docs.swift.org/swift-book/documentation/the-swift-programming-language/stringsandcharacters/#Multiline-String-Literals
In addition, the code example does not call the API.
Comments like //Example should be removed and I would suggest to rather add an explanation around the example block.
Last but not least, I would move the example above the parameters section of the code documentation.
| /// - Returns: Parsed Markdown as an `AttributedString` | ||
| @MainActor private func parse(markdown: Data) -> AttributedString { | ||
| state = .processing | ||
|
|
||
| guard let markdownString = try? AttributedString( | ||
| markdown: markdown, | ||
| options: .init(interpretedSyntax: .inlineOnlyPreservingWhitespace) | ||
| options: .init(interpretedSyntax: .inlineOnly) |
There was a problem hiding this comment.
What is the reason for this change? Wouldn't it make sense to switch to .full?
| /// ```swift | ||
| /// // Example usage: | ||
| /// let content: TextContent = .localized(LocalizedStringResource("HELLO_SPEZI", bundle: .main)) | ||
| /// let localizedString = content.localizedString(for: .current) | ||
| /// print(localizedString) // Prints the localized string for the current locale. | ||
| /// ``` |
There was a problem hiding this comment.
Thank you for the added documentation!
The same comments about the example also apply here as well.
Markdown formatting + documentation
♻️ Current situation & Problem
The current SpeziViews module were not all well documented equally, as per the documentation requirements set forth in the Stanford Spezi Documentation Guide.. Also, the markdown formatting was restricted to
inlineOnlychanges, which would not render formatting for larger changes like titles, which was the issue our app's consent doc ran into.⚙️ Release Notes
.fullto incorporate the full breadth of document markdowns📚 Documentation
✅ Testing
AttributedStringframework to render markdown documents locally, and it seemed to render properly. Not sure what edge cases the Spezi team wanted to account for by setting it toinline, so further testing for the full template application is needed.📝 Code of Conduct & Contributing Guidelines
By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines: