search

Juanpe / SkeletonView

☠️ An elegant way to show users that something is happening and also prepare them to which contents they are awaiting
MIT License
12.62k stars 1.12k forks source link

UnSwizzleLayoutSubviews should not have a delay. #490

Open alexookah opened 2 years ago

alexookah commented 2 years ago

Hello, I happened to have another infinite loop issue with swizzle methods. I remember I had fixed this in the past with adding the unSwizzle methods. https://github.com/Juanpe/SkeletonView/pull/381

In order to fix my new issue which was trying to show a floatingKeyboard right after hidingSkeletonView. I think that the unSwizzle method occurs too late.

The swizzle delay 0.01 maybe makes sense to me but not the unSwizzle delay. Also does unSwizzle really need to be in async method?

https://github.com/Juanpe/SkeletonView/blob/main/SkeletonViewCore/Sources/Internal/UIKitExtensions/UIView%2BSwizzling.swift#L44

When receiving the notification with skeletonDidDisappear the swizzle methods are still active (due to the delay 0.01). It should be triggerred after the unswizzle are completed. The sk.isSkeletonActive should return false only when the unSwizzle method is disabled in my opinion.

Should I make a MR that removes the delay for unSwizzle methods?

I was also thinking about some delegate about the didDisappear or some callback that runs after the unswizzle is triggered. Do you like that idea?

alexookah commented 2 years ago

Added a MR to remove the async delay for unSwizzle methods https://github.com/Juanpe/SkeletonView/pull/491

alexookah commented 2 years ago

I was thinking, maybe the async+delay should also be removed from the swizzle methods in case the hideSkeletonView is run immediately after showSkeletonView

artgoncharov commented 2 years ago

Hello, I have a sample to reproduce the issue here: https://github.com/Juanpe/SkeletonView/issues/495 Unfortunately looks like swizzling by itself breaks the internal logic of UIKit

alexookah commented 2 years ago

When Skeleton is active and you try to show a floating keyboard it will produce infinite loop.

In order to avoid this and you want to show keyboard do it after stopping the skeleton Animation with a delay 0.02 seconds. (work-around)

artgoncharov commented 2 years ago

I know that it produce an infinite loop, that's why I've opened the issue :) Unfortunately, yours workaround it's not a workaround, I have a modular app, some part of the screen still loading but another one it ready for the interaction. Why one part should block another one? That's the main reason why we all should avoid swizzling in production