Antondomashnev / ADTickerLabel

An objective-c UIView which provide a mechanism to show numbers with rolling effect, like in counter
MIT License
113 stars 27 forks source link

Incorrect line height calculation, and other issues #10

Closed Igor-Palaguta closed 8 years ago

Igor-Palaguta commented 9 years ago

Hello,

First of all thank you for library. It looks very pretty!

1) try your library with [UIFont fontWithName: @"HelveticaNeue-Light" size: 20]; Position of characters is broken. Instead of lineHeight use:

[text boundingRectWithSize:CGSizeMake(self.characterWidth, MAXFLOAT)
                                                  options:NSStringDrawingUsesLineFragmentOrigin
                                               attributes:@{NSFontAttributeName: self.font}
                                                  context:0]

NSStringDrawingUsesLineFragmentOrigin is required!

2) Instead of changing frame of label, better create subview and change subview frame and add all characters there.

3) It does not work with autolayout

4) It can not be used inside xib

I created fork with these changes, you can check it there: https://github.com/Igor-Palaguta/ADTickerLabel

Antondomashnev commented 9 years ago

Hello @Igor-Palaguta

Thanks for these changes and fixes. I haven't checked your commits well, but anyway why did you decide to delete "strong/weak/unsafe_unretained" keywords by refactoring?

Igor-Palaguta commented 9 years ago

Hello @Antondomashnev

strong is not required for object types, unsafe_unretained or assign are not required for plain types. Compiler correctly adds them automatically weak - did not remove

Antondomashnev commented 9 years ago

Ok,

To my mind it's better to keep these keywords for a better code understanding. As you can see in all public well known code style guides there are these keywords.

On Monday, 20 April 2015, Igor-Palaguta notifications@github.com wrote:

Hello @Antondomashnev https://github.com/Antondomashnev

strong is not required for object types, unsafe_unretained or assign are not required for plain types weak - did not remove

— Reply to this email directly or view it on GitHub https://github.com/Antondomashnev/ADTickerLabel/issues/10#issuecomment-94379419 .


Anton Domashnev || Conichi, iPhone Developer mob .: +7 926 683 1069 anton.domashmev@conichi.com anton.domashnev@empatika.com anton.domashnev@gmail.com anton.domashnev@empatika.com Skype: anton_domashnev

Igor-Palaguta commented 9 years ago

It depends on author. I prefer not to write it. But really a lot of libs still uses it

Antondomashnev commented 9 years ago

Ok,

It it's not a problem for you, please could you return back these keywords and I'll merge this pull request.

Thanks again for these changes.

On Mon, Apr 20, 2015 at 9:46 AM, Igor-Palaguta notifications@github.com wrote:

It depends on author. I prefer not to write it. But really a lot of libs still uses it

— Reply to this email directly or view it on GitHub https://github.com/Antondomashnev/ADTickerLabel/issues/10#issuecomment-94383004 .


Anton Domashnev || Conichi, iPhone Developer mob .: +7 926 683 1069 anton.domashmev@conichi.com anton.domashnev@empatika.com anton.domashnev@gmail.com anton.domashnev@empatika.com Skype: anton_domashnev

Igor-Palaguta commented 9 years ago

Np, Yes sure will back it.

Igor-Palaguta commented 9 years ago

Done. Please check other cases. I started fork not from head revision. Could lost some of cases

Antondomashnev commented 9 years ago

Hello @Igor-Palaguta,

Please could you make a pull request?

Igor-Palaguta commented 9 years ago

Hello @Antondomashnev

Please check https://github.com/Antondomashnev/ADTickerLabel/pull/12