Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect padding and multiline layer frame calculation #340

Merged
merged 2 commits into from
Oct 26, 2020
Merged

Fix incorrect padding and multiline layer frame calculation #340

merged 2 commits into from
Oct 26, 2020

Conversation

yzhao198
Copy link

@yzhao198 yzhao198 commented Oct 9, 2020

Summary

Thank you for writing this library. Here is a snippet of code I used when testing SkeletonView:

import UIKit
import SkeletonView

class ViewController: UIViewController {

    private lazy var label: UILabel = {
        let label = UILabel()
        label.translatesAutoresizingMaskIntoConstraints = false
        label.numberOfLines = 2
        label.skeletonLineSpacing = 8
        label.font = .systemFont(ofSize: 10)
        label.lastLineFillPercent = 50
        label.skeletonPaddingInsets = .init(top: 12, left: 0, bottom: 12, right: 30)
        label.isSkeletonable = true
        
        return label
    }()
    
    override func viewDidLoad() {
        super.viewDidLoad()
        
        view.backgroundColor = .white
        
        SkeletonAppearance.default.multilineHeight = 12
        
        view.addSubview(label)
        
        label.heightAnchor.constraint(equalToConstant: 54).isActive = true
        label.widthAnchor.constraint(equalToConstant: 250).isActive = true
        label.centerXAnchor.constraint(equalTo: view.centerXAnchor).isActive = true
        label.centerYAnchor.constraint(equalTo: view.centerYAnchor).isActive = true
        
        view.isSkeletonable = true
        view.showSkeleton()
    }
}

The desired result should be

Screen Shot 2020-10-08 at 8 30 09 PM

but the current version (1.10.0) gives me

Screen Shot 2020-10-08 at 8 34 58 PM

Given my testing device is NOT RTL, it is clear the padding is incorrectly rendered. After the changes made in this MR, the desired result is achieved.

Requirements (place an x in each of the [ ])

@Juanpe
Copy link
Owner

Juanpe commented Oct 21, 2020

Hi @yzhao198, thanks for your first contribution. I've checked the code and it looks perfect.

There is only one thing to do before merging. You should update the CHANGELOG file including this PR, ok?

Thanks again

@yzhao198
Copy link
Author

Thanks for your review @Juanpe I've updated CHANGELOG.md.

Copy link
Owner

@Juanpe Juanpe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice! 👌🏼

@Juanpe Juanpe merged commit 932a1f6 into Juanpe:develop Oct 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants