[TASK] Coding Guideline Update for SwiftUI

[vincode-io]

We definitely need this. Let's use this ticket to keep ideas about what should be included. We are all learning about SwiftUI and best practices. Best practices as they apply to NNW are very much in flux right now.

Everyone, please add to this list as you find something you feel like you would have benefited from by knowing it in advance.

[vincode-io]

I posted this in Slack and we should adapt it to the coding guidelines since it seems to be working out:

This hasn’t been communicated out to the developers yet and we will probably end up writing a technote to formalize it. I was waiting until we had more experience with implementing it as a pattern before doing so. But, it’s come up, so I’ll address it briefly here. We want to put any system state changing logic inside of the ViewModel classes. That would mean moving AddWebFeedView.addWebFeed to AddWebFeedViewModel. I recommend everyone review AddWebFeedViewto see what I mean. If you have to respond to system events, like a Notification, it should also be placed inside the ViewModel. The goal here is to use ViewModels (as ObservableObjects) as an abstraction that gives the impression to SwiftUI that NNW is built as a reactive system. Inside your ViewModel’s you can mix and match Combine or imperative code, which ever works best. Views should never subscribe to Notifications or other events. These should always go through a ViewModel which translates them into published events. I’d also like to put the ViewModels in separate swift files from the View itself. Lots of times this wouldn’t be necessary, but I think we should do it for consistency sake.

Edit: We should not use notifications with selectors in the ViewModels. This can lead to hard to find bugs when mixed with Combine code. We should always use Combine when we need to access the Notification Center.

[vincode-io]

We don’t suffix our view models with ViewModel, we just use Model.

[vincode-io]

We don't have any current plans to add Combine support to the Framework projects or Submodules. If you feel that it would be helpful for you, please bring it up in the #work channel. Instead, you should create a View Model class that subscribes to existing Notifications and implements itself as an ObservableObject.

[stuartbreckenridge]

I was about to write about notifications.

As a guideline, should notifications be subscribed to in the View:

.onReceive(NotificationCenter.default.publisher(for: .AccountRefreshDidBegin)) { _ in
    viewModel.doSomething()
}

Or in the View Model:

NotificationCenter.default.addObserver(self, selector: #selector(doSomething(_:)), name: .AccountRefreshDidBegin, object: nil)

[vincode-io]

@stuartbreckenridge Definitely in the View Model.

The goal is to make NNW look more like a reactive architecture without actually changing it by using View Models as a layer of abstraction.

[vincode-io]

For each scene (window) we could have an SceneModel. This would provide and have access to three additional models. The SidebarModel, TimelineModel, and ArticleModel. The submodels would create delegates that SceneModel can implement to all us to call up into SceneModel when necesssary. For example if SidebarModel needs to do a “Mark All as Read” it would call a delegate method that SceneModel would implement. SceneModel would then forward the request to TimelineModel which would mark the Timeline as read.

This is most similar to how the Mac app is implemented today. MainWindowController == SceneModel, SidebarController == SidebarModel, etc… If you squint real hard it is also how iOS is implemented. SceneCoordinator == SceneModel, MasterFeedViewController == SidebarModel, etc…

By more tracking how the Mac app is done by delegating work to the submodels, I think we can avoid having a super large class like SceneCoordinator turned into.

[stuartbreckenridge]

• [ ] Do use self
• [ ] Don't use self SE-0269

[stuartbreckenridge]

Reference: Managing Model Data in Your App

@StateObject Use @StateObject when the model object is owned by the view in which is created in or will be passed to child views as a Binding.

struct ScoreboardView {

    @StateObject private var model = ScoreboardModel()

    var body: some View {
        ScoreView(model: model)
    }

}

Why?: A state object behaves like an observed object, except that SwiftUI knows to create and manage a single object instance for a given view instance, regardless of how many times it recreates the view. You can use the object locally, or pass the state object into another view’s observed object property.

@ObservedObject Use @ObservedObject to observe changes in a model object passed down the hierarchy. Do not create an @ObservedObject in a View.

struct ScoreView {

    @ObservedObject private var model: ScoreboardModel

    var body: some View {
        Text(model.teamName)
        Text(model.currentScore)
    }

}

Why?: SwiftUI might create or recreate a view at any time, so it’s important that initializing a view with a given set of inputs always results in the same view. As a result, it’s unsafe to create an observed object inside a view.

@EnvironmentObject: NetNewsWire has several model objects—e.g. SceneModel—that are shared throughout the app. To subscribe to changes in these objects use @EnvironmentObject in the specific view:

struct SidebarContainerView: View {

    @EnvironmentObject private var sceneModel: SceneModel
    @StateObject private var sidebarModel = SidebarModel()

    @State private var showSettings: Bool = false

        @ViewBuilder var body: some View {
        SidebarView()
            .modifier(SidebarToolbarModifier())
            .modifier(SidebarListStyleModifier())
            .environmentObject(sidebarModel)
            .navigationTitle(Text("Feeds"))
            .onAppear {
                sceneModel.sidebarModel = sidebarModel
                sidebarModel.delegate = sceneModel
                sidebarModel.rebuildSidebarItems()
            }
       }

}

If you use an environment object, you might add it to the view at the top of your app’s hierarchy, as shown above. Alternatively, you might add it to the root view of a sub-tree in your view hierarchy. Either way, remember to also add it to the preview provider of any view that uses the object, or that has a descendant that uses the object

[rizwankce]

• Swiftui views are cheap. So it’s okay to create as much views as we want • always indent with what Xcode suggests. I make sure Ctrl+I to auto indent

[stuartbreckenridge]

Accessibility:

On user interactive controls—e.g. Button, add help tags using the .help("Add Feed") modifier. On macOS, this will appear as a help tag; on iOS this provides an Accessibility Hint for VoiceOver.

[vincode-io]

If a we need to gate code that has both macOS and iOS then it should be macOS first:

#if os(macOS)
    // AppKit code
#endif

#if os(iOS)
    // UIKit code
#endif

[Wevah]

There's also canImport() which might be more semantically appropriate, depending:

#if canImport(AppKit)
    // AppKit code
#elseif canImport(UIKit)
    // UIKit code
#endif

It probably doesn't matter that much as long as the intent is clear, though.

[stuartbreckenridge]

Related to notifications, subscribing via .sink or stick with #selector?

class MyModel: ObservableObject {

    // Subscriptions
    var notificationSubscriptions = Set<AnyCancellable>()

    init() {
        NotificationCenter.default.publisher(for: .UserDidAddAccount).sink(receiveValue: {  _ in
        self.sortedAccounts = AccountManager.shared.sortedAccounts
    }).store(in: &notificationSubscriptions)
    }
}

[vincode-io]

We should use the new trailing syntax as much as possible to make SwiftUI code more readable.

Button {
    sceneModel.shareArticle()
} label: {
    AppAssets.shareImage
}

[stuartbreckenridge]

When we are dipping into AppKit or UIKit in SwiftUI code, should we annotate it with #warning so we know we have to go back to it to clean it up?

Example here.

[vincode-io]

I don't think the warning is necessary. We'll remember to clear out the hacks when we get SwiftUI solutions for them.

[stuartbreckenridge]

Ok. I’ve removed it where it was used. 

[vincode-io]

Closing this because we don't plan on doing SwiftUI in the immediate future.