search

JamesSedlacek / Routing

SwiftUI library for abstracting navigation logic from views
MIT License
353 stars 19 forks source link

fix onDismiss Handler for Sheets & FullScreenCovers 🚀🚀 #13

Closed afathe7090 closed 10 months ago

afathe7090 commented 11 months ago
JamesSedlacek commented 11 months ago

Hey, so I guess I should have added links to the documentation for what needs to be added.

https://developer.apple.com/documentation/swiftui/view/sheet(ispresented:ondismiss:content:)

The sheet view modifier has an optional closure for onDismiss. This is what we want to add functionality for. Not onAppear or onDisappear.

afathe7090 commented 11 months ago

you can check now @JamesSedlacek

JamesSedlacek commented 11 months ago

Overall this is pretty good! Thanks for taking the time to work on this issue.

JamesSedlacek commented 11 months ago

Can we add tests so that we know for sure that when an onDismiss is set in a presentSheet call and then another presentSheet is called without an onDismiss defined, that the onDismiss is set to nil

afathe7090 commented 11 months ago

Sorry, it was closed by mistake @JamesSedlacek 🥲

afathe7090 commented 11 months ago

Can we add tests so that we know for sure that when an onDismiss is set in a presentSheet call and then another presentSheet is called without an onDismiss defined, that the onDismiss is set to nil

we can do that, but we need to remove public func presentSheet(_ destination: Destination) method and use public func presentSheet(_ destination: Destination, onDismiss: DismissAction? = nil) without duplicate code. then, we can test it to present two sheets first one have an onDismiss action and call-dismiss and the other will present it without add onDismiss action then Asset that onDismiss will be nil.

JamesSedlacek commented 10 months ago

@afathe7090 I'm moving all of this stuff over to another repo: https://github.com/JamesSedlacek/Presenting

You can open up another PR in the repo if you want.