From c709ef41cac4dc1dd07204810fdc00eed3707d6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Pacheco=20Neves?= Date: Mon, 14 Nov 2022 18:53:35 +0000 Subject: [PATCH 1/2] Fix `UIButton` prepare/recover and improve `.none` transition ## Motivation The current `RecoverableButtonViewState` model only works for very simple buttons where just the title is defined. This means that on more complex scenarios buttons aren't properly "prepared" to enter skeleton mode and remain visible for a while until they are covered with the skeleton layer, where the remaining elements are already in skeleton mode (e.g. labels). Furthermore, when configuring styling on buttons they reorder subviews/sublayers so that the skeleton layer didn't always show on top. Same thing is true if `layer.borderColor` is non-nil - the border can be seen for a while until the skeleton layer covers it. Additionally, using `SkeletonTransitionStyle.none` would still allow animations to be performed if any change done in `prepareViewForSkeleton` triggers a layout pass. This is especially noticeable when pushing a screen in a navigation controller and enabling skeleton mode in `viewDidLoad`. ## Changes - Add `state`, `attributedTitle`, `titleColor`, `image` and `backgroundImage` to `RecoverableButtonViewState`, clear them on `prepareViewForSkeleton` and recover them on `recoverViewState` (according to `state`). - Set the `SkeletonLayer.maskLayer`'s `zPosition` to `Float.greatestFiniteMagnitude` so that it always sits on top of the view's content. - Add `borderColor` to `RecoverableViewState`, clear it on `prepareViewForSkeleton` and recover it in `recoverViewState`. - Wrap `startTransition` block in a `UIView.performWithoutAnimation` when `SkeletonTransitionStyle.none`. - Fix `DispatchQueue.main` usage in `CALayer` animation extensions. --- .../Models/RecoverableViewState.swift | 14 +++++++- .../Internal/Models/SkeletonLayer.swift | 1 + .../PrepareViewForSkeleton.swift | 9 +++++- .../SkeletonExtensions/Recoverable.swift | 23 +++++++++++-- .../UIKitExtensions/CALayer+Extensions.swift | 32 +++++++++++-------- .../UIKitExtensions/UIView+Transitions.swift | 2 +- 6 files changed, 62 insertions(+), 19 deletions(-) diff --git a/SkeletonViewCore/Sources/Internal/Models/RecoverableViewState.swift b/SkeletonViewCore/Sources/Internal/Models/RecoverableViewState.swift index 69d2e0ec..d7348285 100644 --- a/SkeletonViewCore/Sources/Internal/Models/RecoverableViewState.swift +++ b/SkeletonViewCore/Sources/Internal/Models/RecoverableViewState.swift @@ -13,12 +13,14 @@ struct RecoverableViewState { var backgroundColor: UIColor? var cornerRadius: CGFloat var clipToBounds: Bool + var borderColor: CGColor? var isUserInteractionsEnabled: Bool init(view: UIView) { self.backgroundColor = view.backgroundColor self.clipToBounds = view.layer.masksToBounds self.cornerRadius = view.layer.cornerRadius + self.borderColor = view.layer.borderColor self.isUserInteractionsEnabled = view.isUserInteractionEnabled } @@ -70,10 +72,20 @@ struct RecoverableImageViewState { } struct RecoverableButtonViewState { + var state: UIControl.State + var attributedTitle: NSAttributedString? var title: String? + var titleColor: UIColor? + var image: UIImage? + var backgroundImage: UIImage? init(view: UIButton) { - self.title = view.titleLabel?.text + self.state = view.state + self.attributedTitle = view.attributedTitle(for: state) + self.title = view.title(for: state) + self.titleColor = view.titleColor(for: state) + self.image = view.image(for: state) + self.backgroundImage = view.backgroundImage(for: state) } } diff --git a/SkeletonViewCore/Sources/Internal/Models/SkeletonLayer.swift b/SkeletonViewCore/Sources/Internal/Models/SkeletonLayer.swift index e5c5b146..03b14b84 100755 --- a/SkeletonViewCore/Sources/Internal/Models/SkeletonLayer.swift +++ b/SkeletonViewCore/Sources/Internal/Models/SkeletonLayer.swift @@ -27,6 +27,7 @@ struct SkeletonLayer { self.maskLayer.anchorPoint = .zero self.maskLayer.bounds = holder.definedMaxBounds self.maskLayer.cornerRadius = CGFloat(holder.skeletonCornerRadius) + self.maskLayer.zPosition = CGFloat(Float.greatestFiniteMagnitude) // CoreAnimation complains if CGFloat is used addTextLinesIfNeeded() self.maskLayer.tint(withColors: colors, traitCollection: holder.traitCollection) } diff --git a/SkeletonViewCore/Sources/Internal/SkeletonExtensions/PrepareViewForSkeleton.swift b/SkeletonViewCore/Sources/Internal/SkeletonExtensions/PrepareViewForSkeleton.swift index 660e6683..49a6388b 100644 --- a/SkeletonViewCore/Sources/Internal/SkeletonExtensions/PrepareViewForSkeleton.swift +++ b/SkeletonViewCore/Sources/Internal/SkeletonExtensions/PrepareViewForSkeleton.swift @@ -22,6 +22,7 @@ extension UIView { startTransition { [weak self] in self?.backgroundColor = .clear + self?.layer.borderColor = nil } } @@ -101,7 +102,13 @@ extension UIButton { } startTransition { [weak self] in - self?.setTitle(nil, for: .normal) + guard let self = self else { return } + + self.setTitle(nil, for: self.state) + self.setTitleColor(nil, for: self.state) + self.setAttributedTitle(nil, for: self.state) + self.setImage(nil, for: self.state) + self.setBackgroundImage(nil, for: self.state) } } diff --git a/SkeletonViewCore/Sources/Internal/SkeletonExtensions/Recoverable.swift b/SkeletonViewCore/Sources/Internal/SkeletonExtensions/Recoverable.swift index a4e644c5..79f1051c 100644 --- a/SkeletonViewCore/Sources/Internal/SkeletonExtensions/Recoverable.swift +++ b/SkeletonViewCore/Sources/Internal/SkeletonExtensions/Recoverable.swift @@ -32,6 +32,7 @@ extension UIView: Recoverable { self.layer.cornerRadius = storedViewState.cornerRadius self.layer.masksToBounds = storedViewState.clipToBounds + self.layer.borderColor = storedViewState.borderColor if self.isUserInteractionDisabledWhenSkeletonIsActive { self.isUserInteractionEnabled = storedViewState.isUserInteractionsEnabled @@ -177,8 +178,26 @@ extension UIButton { override func recoverViewState(forced: Bool) { super.recoverViewState(forced: forced) startTransition { [weak self] in - if self?.title(for: .normal) == nil { - self?.setTitle(self?.buttonState?.title, for: .normal) + guard let self = self, let buttonState = self.buttonState else { return } + + let state = buttonState.state + + if let attributedTitle = buttonState.attributedTitle, self.attributedTitle(for: state) == nil || forced { + self.setAttributedTitle(attributedTitle, for: state) + } else if let title = buttonState.title, self.title(for: state) == nil || forced { + self.setTitle(title, for: state) + + if let titleColor = buttonState.titleColor, self.titleColor(for: state) == nil || forced { + self.setTitleColor(titleColor, for: state) + } + } + + if let image = buttonState.image, self.image(for: state) == nil || forced { + self.setImage(image, for: state) + } + + if let backgroundImage = buttonState.backgroundImage, self.backgroundImage(for: state) == nil || forced { + self.setBackgroundImage(backgroundImage, for: state) } } } diff --git a/SkeletonViewCore/Sources/Internal/UIKitExtensions/CALayer+Extensions.swift b/SkeletonViewCore/Sources/Internal/UIKitExtensions/CALayer+Extensions.swift index fc4bc637..49d01775 100644 --- a/SkeletonViewCore/Sources/Internal/UIKitExtensions/CALayer+Extensions.swift +++ b/SkeletonViewCore/Sources/Internal/UIKitExtensions/CALayer+Extensions.swift @@ -51,12 +51,14 @@ extension CALayer { } } - func playAnimation(_ anim: SkeletonLayerAnimation, key: String, completion: (() -> Void)? = nil) { + func playAnimation(_ anim: @escaping SkeletonLayerAnimation, key: String, completion: (() -> Void)? = nil) { skeletonSublayers.recursiveSearch(leafBlock: { - DispatchQueue.main.async { CATransaction.begin() } - DispatchQueue.main.async { CATransaction.setCompletionBlock(completion) } - add(anim(self), forKey: key) - DispatchQueue.main.async { CATransaction.commit() } + DispatchQueue.main.async { + CATransaction.begin() + CATransaction.setCompletionBlock(completion) + self.add(anim(self), forKey: key) + CATransaction.commit() + } }) { $0.playAnimation(anim, key: key, completion: completion) } @@ -71,15 +73,17 @@ extension CALayer { } func setOpacity(from: Int, to: Int, duration: TimeInterval, completion: (() -> Void)?) { - DispatchQueue.main.async { CATransaction.begin() } - let animation = CABasicAnimation(keyPath: #keyPath(CALayer.opacity)) - animation.fromValue = from - animation.toValue = to - animation.duration = duration - animation.timingFunction = CAMediaTimingFunction(name: CAMediaTimingFunctionName.easeInEaseOut) - DispatchQueue.main.async { CATransaction.setCompletionBlock(completion) } - add(animation, forKey: "setOpacityAnimation") - DispatchQueue.main.async { CATransaction.commit() } + DispatchQueue.main.async { + CATransaction.begin() + CATransaction.setCompletionBlock(completion) + let animation = CABasicAnimation(keyPath: #keyPath(CALayer.opacity)) + animation.fromValue = from + animation.toValue = to + animation.duration = duration + animation.timingFunction = CAMediaTimingFunction(name: CAMediaTimingFunctionName.easeInEaseOut) + self.add(animation, forKey: "setOpacityAnimation") + CATransaction.commit() + } } func insertSkeletonLayer(_ sublayer: SkeletonLayer, atIndex index: UInt32, transition: SkeletonTransitionStyle, completion: (() -> Void)? = nil) { diff --git a/SkeletonViewCore/Sources/Internal/UIKitExtensions/UIView+Transitions.swift b/SkeletonViewCore/Sources/Internal/UIKitExtensions/UIView+Transitions.swift index 66b3a610..aa31c661 100644 --- a/SkeletonViewCore/Sources/Internal/UIKitExtensions/UIView+Transitions.swift +++ b/SkeletonViewCore/Sources/Internal/UIKitExtensions/UIView+Transitions.swift @@ -7,7 +7,7 @@ extension UIView { func startTransition(transitionBlock: @escaping () -> Void) { guard let transitionStyle = _currentSkeletonConfig?.transition, transitionStyle != .none else { - transitionBlock() + UIView.performWithoutAnimation(transitionBlock) return } From 60f3f4dc9bfd8d9c6f4741362eeafa83aab1bc6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andre=CC=81=20Pacheco=20Neves?= Date: Tue, 22 Nov 2022 17:09:06 +0000 Subject: [PATCH 2/2] Remove `DispatchQueue.main.async` in CALayer operations ## Changes - `DispatchQueue.main.async` calls in `CALAyer` extension cause animations to become out of sync with other UI events/animations like navigation pushes and/or other elements that are animated while skeleton is being shown/hidden. Removing them ensures better animation ordering/orchestration. --- .../UIKitExtensions/CALayer+Extensions.swift | 32 ++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/SkeletonViewCore/Sources/Internal/UIKitExtensions/CALayer+Extensions.swift b/SkeletonViewCore/Sources/Internal/UIKitExtensions/CALayer+Extensions.swift index 49d01775..6531f6c7 100644 --- a/SkeletonViewCore/Sources/Internal/UIKitExtensions/CALayer+Extensions.swift +++ b/SkeletonViewCore/Sources/Internal/UIKitExtensions/CALayer+Extensions.swift @@ -53,12 +53,10 @@ extension CALayer { func playAnimation(_ anim: @escaping SkeletonLayerAnimation, key: String, completion: (() -> Void)? = nil) { skeletonSublayers.recursiveSearch(leafBlock: { - DispatchQueue.main.async { - CATransaction.begin() - CATransaction.setCompletionBlock(completion) - self.add(anim(self), forKey: key) - CATransaction.commit() - } + CATransaction.begin() + CATransaction.setCompletionBlock(completion) + self.add(anim(self), forKey: key) + CATransaction.commit() }) { $0.playAnimation(anim, key: key, completion: completion) } @@ -73,24 +71,22 @@ extension CALayer { } func setOpacity(from: Int, to: Int, duration: TimeInterval, completion: (() -> Void)?) { - DispatchQueue.main.async { - CATransaction.begin() - CATransaction.setCompletionBlock(completion) - let animation = CABasicAnimation(keyPath: #keyPath(CALayer.opacity)) - animation.fromValue = from - animation.toValue = to - animation.duration = duration - animation.timingFunction = CAMediaTimingFunction(name: CAMediaTimingFunctionName.easeInEaseOut) - self.add(animation, forKey: "setOpacityAnimation") - CATransaction.commit() - } + CATransaction.begin() + CATransaction.setCompletionBlock(completion) + let animation = CABasicAnimation(keyPath: #keyPath(CALayer.opacity)) + animation.fromValue = from + animation.toValue = to + animation.duration = duration + animation.timingFunction = CAMediaTimingFunction(name: CAMediaTimingFunctionName.easeInEaseOut) + self.add(animation, forKey: "setOpacityAnimation") + CATransaction.commit() } func insertSkeletonLayer(_ sublayer: SkeletonLayer, atIndex index: UInt32, transition: SkeletonTransitionStyle, completion: (() -> Void)? = nil) { insertSublayer(sublayer.contentLayer, at: index) switch transition { case .none: - DispatchQueue.main.async { completion?() } + completion?() case .crossDissolve(let duration): sublayer.contentLayer.setOpacity(from: 0, to: 1, duration: duration, completion: completion) }