call toString on fontWeight else throws error if passed an integer #10483
Conversation
|
By analyzing the blame information on this pull request, we identified @spicyj and @davidaurelio to be potential reviewers. |
|
Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours has expired. Before we can review or merge your code, we need you to email cla@fb.com with your details so we can update your status. |
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! |
|
This is going to break if someone doesn't specify a font weight -- can you handle those cases gracefully too? |
|
@davidlrnt updated the pull request - view changes |
|
@ide udpated! |
Libraries/ART/ReactNativeART.js
Outdated
| @@ -439,11 +439,12 @@ function extractFont(font) { | |||
| } | |||
| var fontFamily = extractSingleFontFamily(font.fontFamily); | |||
| var fontSize = +font.fontSize || 12; | |||
| var fontWeight = font.fontWeight ? font.fontWeight.toString() : "400"; | |||
| return { | |||
There was a problem hiding this comment.
quotes: Strings must use singlequote.
|
@davidlrnt updated the pull request - view changes |
1 similar comment
|
@davidlrnt updated the pull request - view changes |
|
@davidlrnt updated the pull request - view changes |
facebook-github-bot
added
the
CLA Signed
Libraries/ART/ReactNativeART.js
Outdated
| @@ -439,11 +439,12 @@ function extractFont(font) { | |||
| } | |||
| var fontFamily = extractSingleFontFamily(font.fontFamily); | |||
| var fontSize = +font.fontSize || 12; | |||
| var fontWeight = font.fontWeight ? font.fontWeight.toString() : '400'; | |||
| return { | |||
There was a problem hiding this comment.
If i set fontWeight to 0, it will fallback to 400, so i think it's better using font.fontWeight != null ? font.fontWeight.toString() : '400'
|
Ping @davidlrnt there seems to be some additional feedback here before the PR can be approved. |
facebook-github-bot
added
GH Review: review-needed
and removed
GH Review: needs-revision
labels
|
Updated after additional feedback |
|
Do you know why the tests aren't passing? |
facebook-github-bot
added
GH Review: accepted
Import Started
|
@shergin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Summary: expects a string, throws error NSNumber cannot be converted to NSString if passed an integer, added toString() method to allow integers, and keep consistency, fontSize allows int. Closes facebook#10483 Differential Revision: D5128581 Pulled By: shergin fbshipit-source-id: 21b1ddd35210c8f061506d71b936cc0ff490d999
expects a string, throws error NSNumber cannot be converted to NSString if passed an integer, added toString() method to allow integers, and keep consistency, fontSize allows int.