search

JonasGessner / JGProgressHUD

An elegant and simple progress HUD for iOS and tvOS, compatible with Swift and ObjC.
MIT License
3.35k stars 336 forks source link

Fixes #127: Implement dismissal completion block #138

Closed aaronpearce closed 4 years ago

aaronpearce commented 4 years ago

Fixes #127.

This adds a basic completion block to be called when dismissal is completed alongside the existing delegates.

Could possibly also add a handler for willDismiss if wanted.

JonasGessner commented 4 years ago

Hey, thanks for the PR. Is there any particular reason for the design choice that the completion block is a property on the HUD? This seems unconventional and will easily lead to retain cycles if people are not careful. I'd opt for a design where the completion block is passed as a parameter to a dismiss function, i.e. -[JGProgressHUD dismiss:withCompletion:].

aaronpearce commented 4 years ago

I believe I implemented it this way to minimise changes within all the dismiss methods. Its also done the same as the other blocks such as tapOutsideBlock and tapOnHUDViewBlock

JonasGessner commented 4 years ago

True about the other blocks, although they are somewhat different in the sense that they do not trigger upon some method call (unlike the dismiss completion block) but happen asynchronously when the user performs some action.

aaronpearce commented 4 years ago

The other reason I did it this was so I did not need to create delegate methods scattered throughout my code unnecessarily. I only needed the one dismissal callback. This way makes my code tidier overall.

PatrikTheDev commented 4 years ago

Personally, I'm for passing the closure into the dismiss(), not as a property. The code may look cleaner but the chance of a retain cycle if someone isn't careful is too high in my opinion.

JonasGessner commented 4 years ago

@aaronpearce I will close it now as I implemented this feature in 8ffc8faba6d31bfd457e1582d24bba5853caa09d where I made the changes that I wanted (i.e. adding a block to the dismiss method directly). But thanks anyway, this was still some good input.