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.51k stars 1.1k forks source link

`skeletonPaddingInsets` with `multilineLastLineFillPercent` in centered label is misaligned #470

Closed inickt closed 2 years ago

inickt commented 2 years ago

Description

It appears the behavior for center aligning the skeleton in a multiline label with padding is not properly aligned.

I am updating from 1.9 and noticed a slight regression. I started trying to come up with a fix to submit a PR but am not sure I have the right solution and am out of time to investigate for now.

What type of issue is this?

Requirements


Bug Report

SkeletonView Environment:

SkeletonView version: 1.26.0 Xcode version: 12.5.1 Swift version: 5.4.2

Steps to reproduce:

  1. Make a multiline skeletonable label
  2. Have multilineLastLineFillPercent not be 100%
  3. Add left and right padding to skeletonPaddingInsets

Expected result:

Last line in label should be center aligned, respecting the padding.

Actual result:

Last line in label is offset and not properly centered.

Attachments:

1.9 Behavior: image Just showing what I am upgrading from. I like it now being centered (since thats how I have this label configured) but will stay on this version for the moment due to this bug.

1.26.0 Bug: image You can see the label is not in the middle of the other lines.

1.26.0 "Fix": image In the func alignLayerFrame(_ rect: CGRect, alignment: NSTextAlignment, isRTL: Bool) -> CGRect function, I changed center to newRect.origin.x = ((superlayer?.bounds.width ?? 0) / 2) - (rect.width / 2) for the above screenshot. I don't know if this is 100% the correct fix– in my example my left and right padding is equal so I can just shove it in the middle of the superlayer's bounds since the size is already taking the last multiline line length into account.

Thanks for maintaining this awesome library! I'd be happy to open a PR to fix, I think I just need some guidance on what you would like and need to make sure I am not missing any other edge cases with different paddings for the left and right.

Juanpe commented 2 years ago

Hi @inickt 👋🏼

Sorry for the late response. I'm going to check it! Thanks 👍🏼

inickt commented 2 years ago

Thanks! If you need any help let me know, I'd be happy to dig in a bit more and open a PR but didn't have the bandwidth before.

Juanpe commented 2 years ago

Yes, if you want, feel free to dig in more, I'd really appreciate it 🙂