EmergeTools / Pow

Delightful SwiftUI effects for your app
https://movingparts.io/pow
MIT License
3.64k stars 153 forks source link

Make iOS-only API a no-op on other platforms #22

Open robb opened 1 year ago

robb commented 1 year ago

I figured it would be better to add to this issue rather than create a new issue. I was wondering if it makes more sense in a cross-platform codebase to make code like this no-op rather than throwing an error, Type 'AnyChangeEffect' has no member 'feedbackHapticSelection'.

.changeEffect(.feedbackHapticSelection, value: self.isShuffling, isEnabled: self.isShuffling)

I'm porting an over to the Mac and while it's possible for me to add conditional compilation flags to solve this, there are a lot of call-sites (as I love Pow) and I figured it might be worth asking if the better approach was to have the API work but just not do anything on the Mac.

Originally posted by @mergesort in https://github.com/movingparts-io/Pow/issues/18#issuecomment-1479868278

mergesort commented 1 year ago

Hey @robb, no rush but I wanted to know if you happened to have an estimate for this issue? I'm beginning the next phase of porting my iOS app to the Mac and am running into these errors so I wanted to see if Pow was planning on adding this functionality soon or if I should figure out a good way to implement these no-ops. (I'm also open to advice on the best approach.)

Thanks a lot as always!

robb commented 1 year ago

So I looked into this and there are basically two cases to consider:

In the latter case, we could do something like

public extension AnyChangeEffect {
    enum FeedbackType {
        case success
        case error
        case warning

        #if os(iOS)
        var uiKitFeedbackType: UINotificationFeedbackGenerator.FeedbackType { /* … */ }
        #endif
    }

    static func feedback(hapticNotification type: FeedbackType) -> AnyChangeEffect {
        #if os(iOS)
        // Do something
        #else
        // Do nothing
        #endif
    }
}

and then replicate all future API on FeedbackType, not great but fine I guess.

That said, in general, the APIs could make it to the new platforms over time. Effectively turning

myView.changeEffect(feedback(.hapticNotification: .success), value: value)

from a noop into an op – also not great. From a support perspective, this looks a little too much like a footgun for my taste.

So currently my feeling is that Pow should at least see if Apple addresses this problem somehow in iOS 17 to deal with navigationBarTitleDisplayMode and the like. Not great but at this point relative to dubdub… 😬

if I should figure out a good way to implement these no-ops. (I'm also open to advice on the best approach.)

I think you'll likely to run into this problem to some extend with system API as well, so I think it's worth considering your options, generally.

One would be to write these noop-overlays yourself, that's what I've done in the past for missing navigation view related API.

Another would be to push the #if os(iOS) shenanigans into separate, higher-level ViewModifiers that you reuse for common effects. So instead of

myView
    .tint(.orange)
#if os(iOS)
    .changeEffect(.feedback(hapticNotification: .success), value: value)
#else
    .changeEffect(.ping(shape: Capsule(), count: 1), value: value)
#endif
    .buttonStyle(.bordered)

you'd do something like

myView
    .tint(.orange)
    .modifier(SuccessEffectModifier(value))
    .buttonStyle(.bordered)

and

struct SuccessEffectModifier<T: Equatable>: ViewModifier {
    var value: T

    init(_ value: T) {
        self.value = value
    }

    func body(content: Content) -> some View {
        #if os(iOS)
        content.changeEffect(.feedback(hapticNotification: .success), value: value)
        #else
        content.changeEffect(.ping(shape: Capsule(), count: 1), value: value)
        #endif
    }
}

This approach would give you full access to the Environment inside your ViewModifier's body(content:), in case you want to do more than just chose a Pow effect.

If all you want is to trigger a Pow effect, you could do something like this:

myView
    .tint(.orange)
    .changeEffect(.success, value: value)
    .buttonStyle(.bordered)
extension AnyChangeEffect {
    static var success: Self {
        #if os(iOS)
        .feedback(hapticNotification: .success)
        #else
        .ping(shape: Capsule(), count: 1)
        #endif
    }
}

Hope this helps in terms of giving you some options. I think it's a bit annoying how SwiftUI's chaning modifier syntax clashes with availability checks, so hopefully that pain is felt inside Apple as well.

mergesort commented 1 year ago

Hey @robb, sorry it took me a bit to get to this I had an unexpected amount of work over the last couple of weeks. Thank you for an incredibly thorough response!

Option 1 is approximately what I'm doing now, but I hadn't considered the ViewModifier approach which I really like so I'm glad I asked.

For the last option below is there something I can do to return the identity or "no change effect"?

extension AnyChangeEffect {
    static var success: Self {
        #if os(iOS)
        .feedback(hapticNotification: .success)
        #else
        .ping(shape: Capsule(), count: 1)
        #endif
    }
}

I think I should be able to call .ping(shape: Capsule(), count: 0) in the Mac branch with no issue, but I wanted to see if you had a best practice in mind.

Since this is adding haptic effects I'd like to do nothing on the Mac in certain circumstances, and I'm not sure what AnyChangeEffect could come closest to replicating that.

Thanks again, I'm already integrating these into my code base and feeling a lot better about how this could all work cross-platform. 🙇🏻‍♂️