Fix UIButton prepare/recover and improve .none transition
#527
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
## 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.
## 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
The current
RecoverableButtonViewStatemodel 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.borderColoris non-nil - the border can be seen for a while until the skeleton layer covers it.Additionally, using
SkeletonTransitionStyle.nonewould still allow animations to be performed if any change done inprepareViewForSkeletontriggers a layout pass. This is especially noticeable when pushing a screen in a navigation controller and enabling skeleton mode inviewDidLoad.Changes
Add
state,attributedTitle,titleColor,imageandbackgroundImagetoRecoverableButtonViewState, clear them onprepareViewForSkeletonand recover them onrecoverViewState(according tostate).Set the
SkeletonLayer.maskLayer'szPositiontoFloat.greatestFiniteMagnitudeso that it always sits on top of the view's content.Add
borderColortoRecoverableViewState, clear it onprepareViewForSkeletonand recover it inrecoverViewState.Wrap
startTransitionblock in aUIView.performWithoutAnimationwhenSkeletonTransitionStyle.none.Remove
DispatchQueue.mainusage inCALayeranimation extensions, as they 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.Demo
skeleton_button_issue_before.mov
skeleton_button_issue_fixed.mov
Requirements