-
Notifications
You must be signed in to change notification settings - Fork 153
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
Comments
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! |
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 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
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 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 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. |
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 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 Since this is adding haptic effects I'd like to do nothing on the Mac in certain circumstances, and I'm not sure what Thanks again, I'm already integrating these into my code base and feeling a lot better about how this could all work cross-platform. 🙇🏻♂️ |
Originally posted by @mergesort in #18 (comment)
The text was updated successfully, but these errors were encountered: