iOS: Fix font weight resolution #15162
Conversation
**Issue:** Some fonts are defined with weights that don't match with the UIFontWeight constants. **Example:** UIFontWeightTraits for Roboto font Light: -0.230 Thin: -0.365 Currently, the UIFontWeightTrait is always used if it != 0.0, and given the UIFontWeight constants for Light and Thin: UIFontWeightThin -0.6 UIFontWeightLight -0.4 A style font weight of "300" or "200" will both resolve to Roboto-Thin as its weight -0.365 is closer to -0.4 (UIFontWeightLight) and -0.6 (UIFontWeightThin) than -0.230 (Roboto-Light). **Proposed fix:** When resolving `getWeightOfFont` try to match the name of weight to the name of the font first, and guess the font with UIFontWeightTrait as the fall back. **Test Plan:** Attempt to display Roboto at weights "200" and "300" and Roboto-Thin and Roboto-Light should be displayed correctly.
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
facebook-github-bot
added
GH Review: review-needed
CLA Signed
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
facebook-github-bot
added
GH Review: accepted
Import Started
|
@javache has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
Makes sense - I've been struggling with this issue for a while. Glad to see it's fixed! |
|
Seems there is a regression with |
|
After: It's obviously that the |
|
@nihgwu good catch. Thats actually an issue with the original name suffix check. It looks like it's finding ultralight before light. I'll add a check for a weight prefix, ultrabold and semibold would be affected the same. Actually, the suffix check wont work with italic fonts either. Even with this fix, font selection is still guess work, which I don't like. I'll investigate. |
|
@kipricker good to know, I know nothing about Objective-C, if you come with the patch, can we also cherry-pick to |
|
Yeah, I'll cherry-pick it as soon as there's a reasonable fix. Let's take
time and try to come up with a working algorithm.
…On Thu, 3 Aug 2017 at 04:30 Neo ***@***.***> wrote:
@kipricker <https://github.com/kipricker> good to know, I know nothing
about Objective-C, if you come with the patch, can we also cherry-pick to
0.48-stable branch @grabbou <https://github.com/grabbou>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#15162 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACWcxsssXolbCfSObeRwIyHfJ3xVrKbhks5sUTDGgaJpZM4OgrkE>
.
|
|
@kipricker any progress on this? |
Summary: fix the regression I mentioned in #15162 (comment) as no one is working on this, I take the step, although I know nothing about Objective C I find the key point is that the keys in `NSDictionary` are not ordered as presented, it's a hash table, so no grantee on keys order, so I create a new array to do that, then it will check `ultralight` before `light` and `semibold` before `bold` Closes #15825 Differential Revision: D5782142 Pulled By: shergin fbshipit-source-id: 5346b0cb263e535c0b445e7a2912c452573248a5
|
@nihgwu I've been super busy and haven't had a lot of time. I am still investigating how to accurately detect font weights beyond just string comparison. I'm going to make the time to have a fix for this by next week. |
Summary: fix the regression I mentioned in #15162 (comment) as no one is working on this, I take the step, although I know nothing about Objective C I find the key point is that the keys in `NSDictionary` are not ordered as presented, it's a hash table, so no grantee on keys order, so I create a new array to do that, then it will check `ultralight` before `light` and `semibold` before `bold` Closes #15825 Differential Revision: D5782142 Pulled By: shergin fbshipit-source-id: 5346b0cb263e535c0b445e7a2912c452573248a5
Summary: fix the regression I mentioned in #15162 (comment) as no one is working on this, I take the step, although I know nothing about Objective C I find the key point is that the keys in `NSDictionary` are not ordered as presented, it's a hash table, so no grantee on keys order, so I create a new array to do that, then it will check `ultralight` before `light` and `semibold` before `bold` Closes #15825 Differential Revision: D5782142 Pulled By: shergin fbshipit-source-id: 5346b0cb263e535c0b445e7a2912c452573248a5
Issue:
Some fonts are defined with weights that don't match with the UIFontWeight constants.
Example:
UIFontWeightTraits for Roboto font
Light: -0.230
Thin: -0.365
Currently, the UIFontWeightTrait is always used if it != 0.0, and given the UIFontWeight constants for Light and Thin:
UIFontWeightThin -0.6
UIFontWeightLight -0.4
A style font weight of "300" or "200" will both resolve to Roboto-Thin as its weight -0.365 is closer to -0.4 (UIFontWeightLight) and -0.6 (UIFontWeightThin) than -0.230 (Roboto-Light).
Proposed fix:
When resolving
getWeightOfFonttry to match the name of weight to the name of the font first, and guess the font with UIFontWeightTrait as the fall back.Test Plan:
Attempt to display Roboto at weights "200" and "300" and Roboto-Thin and Roboto-Light should be displayed correctly.
Current:
Fixed:
