Skip to content
This repository was archived by the owner on Aug 18, 2023. It is now read-only.

Conversation

SuzGupta
Copy link
Contributor

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

  • Launch the application
  • Tap the Settings tab icon
  • Tap What's New
  • Learn all about what's new in your favorite Twitter organizer!

Notes

  • Because Swift's new(ish) AttributedString does not appear to support headers in Markdown, these were reformatted as emboldened text instead.
  • The one unrelated change was to silence a SwiftLint trailing_newline warning.

Screenshot

WhatsNewFromFile

…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
Copy link
Owner

@mikaelacaron mikaelacaron left a comment

Choose a reason for hiding this comment

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

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;
Copy link
Owner

Choose a reason for hiding this comment

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

Why is the deployment target changed to iOS 15.0?

Comment on lines +40 to +48
Section(header: Text("Updates")) {
NavigationLink {
WhatsNewView(viewModel: viewModel)
} label: {
HStack {
Text("What's New")
}
}
}
Copy link
Owner

Choose a reason for hiding this comment

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

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
Copy link
Owner

Choose a reason for hiding this comment

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

This should be an ObservedObject rather than a StateObject

Comment on lines +13 to +19
var body: some View {
let plainString = viewModel.getStringFromBundle()
let attributedString = viewModel.createAttributedString(plainString: plainString)
Text(attributedString)
.padding()
Spacer()
}
Copy link
Owner

Choose a reason for hiding this comment

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

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 {
Copy link
Owner

Choose a reason for hiding this comment

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

Can you also add a navigation title to this screen, just "What's New" is fine!

@@ -0,0 +1,5 @@
**v1.2**
Copy link
Owner

Choose a reason for hiding this comment

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

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 {
Copy link
Owner

Choose a reason for hiding this comment

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

I would rename this to be more specific that this is only for getting the whatsnew file, maybe something like getWhatsNewFromBundle()??

@mikaelacaron mikaelacaron added the hacktoberfest-accepted Accepted PR for Hacktoberfest label Oct 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

hacktoberfest-accepted Accepted PR for Hacktoberfest

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FEATURE - What's New Screen

2 participants