-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Font-family difference between platforms #2648
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,7 @@ import HTML, { | |
splitBoxModelStyle, | ||
} from 'react-native-render-html'; | ||
import Config from '../CONFIG'; | ||
import styles, {webViewStyles} from '../styles/styles'; | ||
import styles, {webViewStyles, getFontFamilyMonospace} from '../styles/styles'; | ||
import fontFamily from '../styles/fontFamily'; | ||
import AnchorForCommentsOnly from './AnchorForCommentsOnly'; | ||
import InlineCodeBlock from './InlineCodeBlock'; | ||
|
@@ -22,6 +22,9 @@ const EXTRA_FONTS = [ | |
fontFamily.GTA_BOLD, | ||
fontFamily.GTA_ITALIC, | ||
fontFamily.MONOSPACE, | ||
fontFamily.MONOSPACE_ITALIC, | ||
fontFamily.MONOSPACE_BOLD, | ||
fontFamily.MONOSPACE_BOLD_ITALIC, | ||
fontFamily.SYSTEM, | ||
]; | ||
|
||
|
@@ -65,11 +68,28 @@ function CodeRenderer({ | |
// We split wrapper and inner styles | ||
// "boxModelStyle" corresponds to border, margin, padding and backgroundColor | ||
const {boxModelStyle, otherStyle: textStyle} = splitBoxModelStyle(style); | ||
|
||
// Get the correct fontFamily variant based in the fontStyle and fontWeight | ||
const font = getFontFamilyMonospace({ | ||
fontStyle: textStyle.fontStyle, | ||
fontWeight: textStyle.fontWeight, | ||
}); | ||
|
||
const textStyleOverride = { | ||
fontFamily: font, | ||
|
||
// We need to override this properties bellow that was defined in `textStyle` | ||
// Because by default the `react-native-render-html` add a style in the elements, | ||
// for example the <strong> tag has a fontWeight: "bold" and in the android it break the font | ||
fontWeight: undefined, | ||
fontStyle: undefined, | ||
}; | ||
|
||
return ( | ||
<InlineCodeBlock | ||
TDefaultRenderer={TDefaultRenderer} | ||
boxModelStyle={boxModelStyle} | ||
textStyle={textStyle} | ||
textStyle={{...textStyle, ...textStyleOverride}} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should pass the new styles as Array as per the new StyleGuide. Change the prop type if necessary. Thanks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I take a look and what this component expect is an object 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well I am not sure about the style prop that Internal TDefaultRenderer expect so its fine here. Otherwise you can just change that to array type and see how it works. |
||
defaultRendererProps={defaultRendererProps} | ||
key={key} | ||
/> | ||
|
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.
Can't we just set it on the default styles for Code blocks as we are just resetting them unconditionally? Also, a comment saying why
undefined
is used would be helpful as I worked over this a little back and these type of changes does not really make sense until specified.https://github.com/Expensify/Expensify.cash/blob/53acb9ce968a9ea3007302e3debb8b2d9a345ebb/src/styles/styles.js#L1409
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.
No, when the tags (strong, i, b ...) is within a code block, the style from this tags will be merged and passed as
text Style
, and with that we can know which font must be used.If we add
undefined
in these properties(font Weight and font Family) in this tags(strong, i, b...), when these tags is inside a code block, we can't know which font must be used.https://github.com/Expensify/Expensify.cash/blob/7a7fd7d8fc2836c9c2f2a4457776c6ec06407fdc/src/components/RenderHTML.js#L67
I will do this change ❤️