SwiftUIBackports
SwiftUIBackports copied to clipboard
Adds interactiveDismissDisabled backport
Describe your changes
This adds a func interactiveDismissDisabled(_ isDisabled: Bool = true)
backport.
Hey, thank you for the submission.
So I actually missed the official implementation for this, didn’t realise we had an official (iOS 15+) modifier so thanks for bring it to my attention.
However I did already include an implementation of this, just with a different name: https://github.com/shaps80/SwiftUIBackports/blob/main/Sources/SwiftUIBackports/Extras/InteractiveDismiss.swift
Can I suggest you update the naming of my original implementation, keeping any docs you’ve added here as they’re def an improvement.
My implementation has a 2nd (unofficial) implementation that enables the user to get a callback when the dismiss attempt occurs. This enables building ‘confirmation’ style UX, so I’d like to keep that.
Hello! I've updated backport code and now it's using your implementation. Please check this out.
Hey thanks! Actually I wasn't clear sorry about that. What I meant was to rename my implementations. So we'd end up with:
interactiveDismissDisabled(_ disabled: Bool = true) -> some View
interactiveDismissDisabled(_ disabled: Bool = true, onAttempt: @escaping () -> Void) -> some View
Note: the closure here shouldn't be optional since passing nil would be the same as using the first modifier.
Also we should move the first one to its own file so it goes inside the Public
folder. The onAttempt
version should stay in Extras
since it's unofficial API.
Hope that's clearer. Sorry for confusion. I really appreciate the contribution 👍👍
But if rename old implementations, people who already using it will get an error. And it will conflict with apple implementation.
we should move the first one to its own file
Both methods depends on private Representable class, that's why I call in backport func your old func
Good point I missed that.
So i think we implement my suggestion, then just include the current api (calling these new methods) for backward compat.
These should also be marked as deprecated:
@obsoleted(swift, renamed: "interactiveDismissDisabled(_:)")
func presentation(_ isModal: Bool) -> some View
Also we should update the README
to include the new APIs (removing the old one).
I'm on phone atm so code might be wrong haha
Why do you want keep two separate methods? At first I did exactly as you suggest. But then I've merged two into one because using deprecated method in backport results warning in backport's code.
This library should support official API, to simplify migration later on. However I wasn’t previously aware of the official API and so I named it differently.
Implementing both the official and the unofficial methods with the same modifier name, ensures easy/consistent discovery for an API consumer.
I like your contribution, which would bring it inline with the official API. However as you mentioned, we now have users that may be using the existing API. In order for me to remove it in a future version I need users to move away from it. That’s the point of deprecations.
If you’re uncomfortable with this change I can do it myself but I thought since you’ve already started the work, you may want your contribution included 👍🏻
Thanks again.
No, I asking about presentation(isModal: Bool)
and presentation(isModal: Bool = true, _ onAttempt: @escaping () -> Void)
. Why not just use single method which I made presentation(isModal: Bool = true, _ onAttempt: (() -> Void)? = nil)
that covers all cases?
No, I asking about
presentation(isModal: Bool)
andpresentation(isModal: Bool = true, _ onAttempt: @escaping () -> Void)
. Why not just use single method which I madepresentation(isModal: Bool = true, _ onAttempt: (() -> Void)? = nil)
that covers all cases?
To be honest I used to do this but I often found this problematic.
For example the onAttempt
is unofficial and provided as an 'extra' feature that's not part of the official backport.
And in many cases the implementation backing those methods would then be different as well. While that's not the case in this instance, I still prefer (for consistency) to define the methods separately. This also mirrors how themselves tend to vend API.
IMO it also simplifies documentation, deprecations and code change over time.
Hey looks like this wasn't moving so I pulled the changes manually into my code.
I've also decided on some other minor changes based on another PR, so its all going in but from my end sorry about that 👍