Adds WhatsNew to Settings, displaying content from a Markdown file in project#172
Adds WhatsNew to Settings, displaying content from a Markdown file in project#172SuzGupta wants to merge 1 commit into
Conversation
…xed a SwiftLint trailing_newline warning in AWSTweet+Extension. Added function to SettingsViewModel to get unformatted string from bundle. This now displays in WhatsNewView. Added second function to create AttributedString, changed Markdown file formatting, now creating and using AttributedString in WhatsNewView
mikaelacaron
left a comment
mikaelacaron
left a comment
There was a problem hiding this comment.
Good work! I have several changes that I'd like, thanks!
| ENABLE_PREVIEWS = YES; | ||
| INFOPLIST_FILE = "brain-marks/Info.plist"; | ||
| IPHONEOS_DEPLOYMENT_TARGET = 14.0; | ||
| IPHONEOS_DEPLOYMENT_TARGET = 15.0; |
There was a problem hiding this comment.
Why is the deployment target changed to iOS 15.0?
| Section(header: Text("Updates")) { | ||
| NavigationLink { | ||
| WhatsNewView(viewModel: viewModel) | ||
| } label: { | ||
| HStack { | ||
| Text("What's New") | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Can you fix the indentation here?
The NavigationLink shouldn't be indented this far
Also the HStack isn't needed
| import SwiftUI | ||
|
|
||
| struct WhatsNewView: View { | ||
| @StateObject var viewModel: SettingsViewModel |
There was a problem hiding this comment.
This should be an ObservedObject rather than a StateObject
| var body: some View { | ||
| let plainString = viewModel.getStringFromBundle() | ||
| let attributedString = viewModel.createAttributedString(plainString: plainString) | ||
| Text(attributedString) | ||
| .padding() | ||
| Spacer() | ||
| } |
There was a problem hiding this comment.
Can you put this into a ScrollView? That will help accommodate as the file gets bigger and with dynamic type. Here's an interesting article!
A better way to this would be to have a function called createWhatsNewAttributedString that uses the function getStringFromBundle rather than needing 2 variables in the View
This also makes it so you can make getStringFromBundle a private function
Lastly instead of returning a blank string, you could return an error message saying like "Problem with file" idk haha something useful, otherwise an empty string is weird
| struct WhatsNewView: View { | ||
| @StateObject var viewModel: SettingsViewModel | ||
|
|
||
| var body: some View { |
There was a problem hiding this comment.
Can you also add a navigation title to this screen, just "What's New" is fine!
| @@ -0,0 +1,5 @@ | |||
| **v1.2** | |||
There was a problem hiding this comment.
This is fine, if you don't want to change it, but I'd prefer for the version numbers to be like this # v1.2, which would show them as a heading rather than just bold
| .resume() | ||
| } | ||
|
|
||
| func getStringFromBundle() -> String { |
There was a problem hiding this comment.
I would rename this to be more specific that this is only for getting the whatsnew file, maybe something like getWhatsNewFromBundle()??
2 participants
Closes #163
What it Does
Adds a new UPDATES section to the Settings view, with so far just a single row, What's New. When tapped, this leads to a view that displays a Markdown file showing the app changelog.
How I Tested
Notes
AttributedStringdoes not appear to support headers in Markdown, these were reformatted as emboldened text instead.trailing_newlinewarning.Screenshot