-
Notifications
You must be signed in to change notification settings - Fork 195
Fix intrinsicContentSize dependence on custom images. #44
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
base: master
Are you sure you want to change the base?
Conversation
Before the fix custom images were scaled to match fixed 44 point height.
|
Probably it's a bad idea, because a tappable area may become less than 44 points. So there should be other solution for custom sized "stars" |
…l custom images. Now custom images won't scale if size of view is bigger than side of image
|
hsousa
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for only looking at this now! 😞
Code overall LGTM, just have some minor tweaks and should be good to go!
| - (CGSize)intrinsicContentSize { | ||
| CGFloat height = 44.f; | ||
| return CGSizeMake(_maximumValue * height + (_maximumValue-1) * _spacing, height); | ||
| CGFloat imageSide = self.emptyStarImage.size.height; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Total nitpick here, but could we call this imageHeight?
| CGFloat availableWidth = rect.size.width - (_spacing * (_maximumValue - 1)) - 2; | ||
| CGFloat cellWidth = (availableWidth / _maximumValue); | ||
| CGFloat starSide = (cellWidth <= rect.size.height) ? cellWidth : rect.size.height; | ||
| if (self.emptyStarImage) starSide = self.emptyStarImage.size.height; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should rely on self.shouldUseImages for this for consistency.
|
Oh, forgot to mention in the review but: Thanks for this! 😄 |
Before the fix custom images were scaled to match fixed 44 point height.
Now
intrinsicContentSizedepends on size of custom image to prevent image scaling.Hi. I hope I don't miss anything, but it works for me.