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

Transform 'kerning(forFont: )' method to public #324

Merged
merged 2 commits into from
Feb 24, 2018

Conversation

acacuce
Copy link
Contributor

@acacuce acacuce commented Feb 13, 2018

Currently kerning(forFont: ) method in Tracking enum has an internal attribute. It would be good if it will be public

Currently kerning(forFont: ) method in Tracking enum has an internal attribute. It would be good if it will be public
@ZevEisenberg
Copy link
Collaborator

@acacuce thanks for your pull request! Just curious - what's your use case for making this method public?

@ZevEisenberg ZevEisenberg self-requested a review February 13, 2018 18:48
@acacuce
Copy link
Contributor Author

acacuce commented Feb 13, 2018

I use StringStyles to describe the text styles in our application. Our designers use custom Tracking. This text is used in a chat where the sizes of chat bubble are calculated in with the help of NSTextContainer. For the correct calculation, I need to transfer the value of the tracking value.

@acacuce
Copy link
Contributor Author

acacuce commented Feb 13, 2018

As an example, I need to count the hashValue from the value of Tacking

@ZevEisenberg
Copy link
Collaborator

Ah, yep, that makes sense. Would you mind adding a unit test that confirms that this method stays public? It would be a simple test case that does import BonMot instead of @testable import BonMot, and then it just needs to call kerning(forFont:), so if the method ever loses public, the test won't compile.

Also, as a current workaround, you can get the kerning if you query yourStringStyle.attributes[.kerning] as? NSNumber (I think as? CGFloat would work too).

@ZevEisenberg ZevEisenberg merged commit a30288b into Rightpoint:master Feb 24, 2018
@ZevEisenberg
Copy link
Collaborator

I merged this, but then I realized it should really be spelled kerning(for:), so the call site looks like kerning(for: font). I'm going to push a change for this. Just a heads up in case you're depending on this method name.

@ZevEisenberg
Copy link
Collaborator

This has been released as part of BonMot 5.1. Thanks for your contribution, @acacuce! 🚀

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.

2 participants