Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
robb opened this issue Mar 23, 2023 · 3 comments
Open

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

robb opened this issue Mar 23, 2023 · 3 comments

Comments

@robb
Copy link
Contributor

robb commented Mar 23, 2023

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 #18 (comment)

@mergesort
Copy link
Collaborator

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
Copy link
Contributor Author

robb commented Apr 11, 2023

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

  • Unavailable API that we own exclusively, e.g. sound effects.
  • Unavailable API that uses Apple-owned types, e.g. haptics.

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

mergesort commented Apr 21, 2023

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. 🙇🏻‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants