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.

Sign up for GitHub

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

Merged
merged 3 commits into from
May 7, 2021
Merged

Font-family difference between platforms #2648

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c826d47
fix: add the new font-family
0xthierry Apr 30, 2021
7e2cb8d
fix: add the new font-family to web
0xthierry May 6, 2021
c9c5bcf
fix: extract the logic to get the font family monospace based in font…
0xthierry May 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added android/app/src/main/assets/fonts/GTAmericaExpMono-Bd.otf
View file
Open in desktop
Binary file not shown.
Binary file added android/app/src/main/assets/fonts/GTAmericaExpMono-BdIt.otf
View file
Open in desktop
Binary file not shown.
Binary file added android/app/src/main/assets/fonts/GTAmericaExpMono-RgIt.otf
View file
Open in desktop
Binary file not shown.
21 changes: 21 additions & 0 deletions assets/css/fonts.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,27 @@
src: url('https://www.expensify.com/font/GT-America-Exp-Mono-Regular.eot') format('embedded-opentype'), url('https://www.expensify.com/font/GT-America-Exp-Mono-Regular.woff') format('woff'), url('https://www.expensify.com/font/GT-America-Exp-Mono-Regular.woff2') format('woff2');
}

@font-face {
font-family: GTAmericaExpMono-RgIt;
font-weight: 400;
font-style: italic;
src: url('https://www.expensify.com/font/GT-America-Exp-Mono-Regular-Italic.eot') format('embedded-opentype'), url('https://www.expensify.com/font/GT-America-Exp-Mono-Regular-Italic.woff') format('woff'), url('https://www.expensify.com/font/GT-America-Exp-Mono-Regular-Italic.woff2') format('woff2');
}

@font-face {
font-family: GTAmericaExpMono-Bd;
font-weight: 700;
font-style: normal;
src: url('https://www.expensify.com/font/GT-America-Exp-Mono-Bold.eot') format('embedded-opentype'), url('https://www.expensify.com/font/GT-America-Exp-Mono-Bold.woff') format('woff'), url('https://www.expensify.com/font/GT-America-Exp-Mono-Bold.woff2') format('woff2');
}

@font-face {
font-family: GTAmericaExpMono-BdIt;
font-weight: 700;
font-style: italic;
src: url('https://www.expensify.com/font/GT-America-Exp-Mono-Bold-Italic.eot') format('embedded-opentype'), url('https://www.expensify.com/font/GT-America-Exp-Mono-Bold-Italic.woff') format('woff'), url('https://www.expensify.com/font/GT-America-Exp-Mono-Bold-Italic.woff2') format('woff2');
}

* {
-webkit-font-smoothing: antialiased;
-moz-osx-font-smoothing: grayscale;
Expand Down
Binary file added assets/fonts/GTAmericaExpMono-Bd.otf
View file
Open in desktop
Binary file not shown.
Binary file added assets/fonts/GTAmericaExpMono-BdIt.otf
View file
Open in desktop
Binary file not shown.
Binary file added assets/fonts/GTAmericaExpMono-RgIt.otf
View file
Open in desktop
Binary file not shown.
12 changes: 12 additions & 0 deletions ios/ExpensifyCash.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
8C86654500DCC843A74147B5 /* libPods-ExpensifyCash-ExpensifyCashTests.a in Frameworks */ = {isa = PBXBuildFile; fileRef = ED2AB27DDDFCCE3CD100EA0C /* libPods-ExpensifyCash-ExpensifyCashTests.a */; };
BB6CECBDA023256B6B955321 /* libPods-ExpensifyCash.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 6F2C8BDCC1FF0B64AE2DFC9B /* libPods-ExpensifyCash.a */; };
E9DF872D2525201700607FDC /* AirshipConfig.plist in Resources */ = {isa = PBXBuildFile; fileRef = E9DF872C2525201700607FDC /* AirshipConfig.plist */; };
52477A09739546F4814EA25F /* GTAmericaExpMono-Bd.otf in Resources */ = {isa = PBXBuildFile; fileRef = 0DE5D096095C41EE96746C9E /* GTAmericaExpMono-Bd.otf */; };
ED814D34526B415CAFA0451E /* GTAmericaExpMono-BdIt.otf in Resources */ = {isa = PBXBuildFile; fileRef = 3981452A2C7340EBBA2B9BD1 /* GTAmericaExpMono-BdIt.otf */; };
DB77016704074197AB6633BB /* GTAmericaExpMono-RgIt.otf in Resources */ = {isa = PBXBuildFile; fileRef = 5150E5D0D7F74DBA8D7C1914 /* GTAmericaExpMono-RgIt.otf */; };
/* End PBXBuildFile section */

/* Begin PBXContainerItemProxy section */
Expand Down Expand Up @@ -70,6 +73,9 @@
ED2971642150620600B7C4FE /* JavaScriptCore.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = JavaScriptCore.framework; path = Platforms/AppleTVOS.platform/Developer/SDKs/AppleTVOS12.0.sdk/System/Library/Frameworks/JavaScriptCore.framework; sourceTree = DEVELOPER_DIR; };
ED2AB27DDDFCCE3CD100EA0C /* libPods-ExpensifyCash-ExpensifyCashTests.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = "libPods-ExpensifyCash-ExpensifyCashTests.a"; sourceTree = BUILT_PRODUCTS_DIR; };
F4BC0E78FF1E9BD8B2D38C66 /* Pods-ExpensifyCash.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-ExpensifyCash.debug.xcconfig"; path = "Target Support Files/Pods-ExpensifyCash/Pods-ExpensifyCash.debug.xcconfig"; sourceTree = "<group>"; };
0DE5D096095C41EE96746C9E /* GTAmericaExpMono-Bd.otf */ = {isa = PBXFileReference; name = "GTAmericaExpMono-Bd.otf"; path = "../assets/fonts/GTAmericaExpMono-Bd.otf"; sourceTree = "<group>"; fileEncoding = undefined; lastKnownFileType = unknown; explicitFileType = undefined; includeInIndex = 0; };
3981452A2C7340EBBA2B9BD1 /* GTAmericaExpMono-BdIt.otf */ = {isa = PBXFileReference; name = "GTAmericaExpMono-BdIt.otf"; path = "../assets/fonts/GTAmericaExpMono-BdIt.otf"; sourceTree = "<group>"; fileEncoding = undefined; lastKnownFileType = unknown; explicitFileType = undefined; includeInIndex = 0; };
5150E5D0D7F74DBA8D7C1914 /* GTAmericaExpMono-RgIt.otf */ = {isa = PBXFileReference; name = "GTAmericaExpMono-RgIt.otf"; path = "../assets/fonts/GTAmericaExpMono-RgIt.otf"; sourceTree = "<group>"; fileEncoding = undefined; lastKnownFileType = unknown; explicitFileType = undefined; includeInIndex = 0; };
/* End PBXFileReference section */

/* Begin PBXFrameworksBuildPhase section */
Expand Down Expand Up @@ -181,6 +187,9 @@
A8D6F2F722FD4E66A38EBBB6 /* GTAmericaExp-BdIt.otf */,
DB5A1365442D4419AF6F08E5 /* GTAmericaExp-MdIt.otf */,
918D7FEFF96242E6B5F5E14D /* GTAmericaExp-RgIt.otf */,
0DE5D096095C41EE96746C9E /* GTAmericaExpMono-Bd.otf */,
3981452A2C7340EBBA2B9BD1 /* GTAmericaExpMono-BdIt.otf */,
5150E5D0D7F74DBA8D7C1914 /* GTAmericaExpMono-RgIt.otf */,
);
name = Resources;
sourceTree = "<group>";
Expand Down Expand Up @@ -307,6 +316,9 @@
6856B78873B64C44A92E51DB /* GTAmericaExp-MdIt.otf in Resources */,
8821A238A081483FA947BC4E /* GTAmericaExp-RgIt.otf in Resources */,
70CF6E82262E297300711ADC /* BootSplash.storyboard in Resources */,
52477A09739546F4814EA25F /* GTAmericaExpMono-Bd.otf in Resources */,
ED814D34526B415CAFA0451E /* GTAmericaExpMono-BdIt.otf in Resources */,
DB77016704074197AB6633BB /* GTAmericaExpMono-RgIt.otf in Resources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
Expand Down
5 changes: 4 additions & 1 deletion ios/ExpensifyCash/Info.plist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
<key>NSCameraUsageDescription</key>
<string>Your camera is used to create chat attachments.</string>
<key>NSLocationWhenInUseUsageDescription</key>
<string></string>
<string/>
<key>NSPhotoLibraryAddUsageDescription</key>
<string>Your camera roll is used to store chat attachments.</string>
<key>NSPhotoLibraryUsageDescription</key>
Expand All @@ -82,6 +82,9 @@
<string>GTAmericaExp-BdIt.otf</string>
<string>GTAmericaExp-MdIt.otf</string>
<string>GTAmericaExp-RgIt.otf</string>
<string>GTAmericaExpMono-Bd.otf</string>
<string>GTAmericaExpMono-BdIt.otf</string>
<string>GTAmericaExpMono-RgIt.otf</string>
</array>
<key>UIBackgroundModes</key>
<array>
Expand Down
18 changes: 17 additions & 1 deletion src/components/RenderHTML.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
];

Expand Down Expand Up @@ -65,11 +68,24 @@ function CodeRenderer({
// We split wrapper and inner styles
// "boxModelStyle" corresponds to border, margin, padding and backgroundColor
const {boxModelStyle, otherStyle: textStyle} = splitBoxModelStyle(style);

const italic = textStyle.fontStyle === 'italic' && fontFamily.MONOSPACE_ITALIC;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this logic should be moved to style.js. for Eg
https://github.com/Expensify/Expensify.cash/blob/53acb9ce968a9ea3007302e3debb8b2d9a345ebb/src/styles/styles.js#L1545-L1550

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can move to styles.js, but I think that is not necessary, because this logic is specific for <CodeRenderer />, so in my option is better keep this logic here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We like to keep all the style logic out of JSX.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@parasharrajat nice, so I will move to styles.js, thank you 🤘🏽

const bold = textStyle.fontWeight === 'bold' && fontFamily.MONOSPACE_BOLD;
const italicBold = italic && bold && fontFamily.MONOSPACE_BOLD_ITALIC;

const font = italicBold || bold || italic || fontFamily.MONOSPACE;

const textStyleOverride = {
fontFamily: font,
fontWeight: undefined,
fontStyle: undefined,
Comment on lines +84 to +85
Copy link
Member

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

Copy link
Contributor Author

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?

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

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.

I will do this change ❤️

};

Luke9389 marked this conversation as resolved.
Show resolved Hide resolved
return (
<InlineCodeBlock
TDefaultRenderer={TDefaultRenderer}
boxModelStyle={boxModelStyle}
textStyle={textStyle}
textStyle={{...textStyle, ...textStyleOverride}}
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

@0xthierry 0xthierry May 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take a look and what this component expect is an object 🤔

Copy link
Member

@parasharrajat parasharrajat May 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

The 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}
/>
Expand Down
5 changes: 4 additions & 1 deletion src/styles/fontFamily/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@ const fontFamily = {
GTA_ITALIC: 'GTAmericaExp-RgIt',
GTA_BOLD: bold,
GTA: 'GTAmericaExp-Regular',
MONOSPACE: 'GTAmericaExpMono-Rg',
SYSTEM: 'System',
MONOSPACE: 'GTAmericaExpMono-Rg',
MONOSPACE_ITALIC: 'GTAmericaExpMono-RgIt',
MONOSPACE_BOLD: 'GTAmericaExpMono-Bd',
MONOSPACE_BOLD_ITALIC: 'GTAmericaExpMono-BdIt',
};

export default fontFamily;
5 changes: 2 additions & 3 deletions src/styles/styles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import fontFamily from './fontFamily';
import italic from './italic';
import addOutlineWidth from './addOutlineWidth';
import themeColors from './themes/default';
import fontWeightBold from './fontWeight/bold';
Expand Down Expand Up @@ -1408,7 +1407,7 @@ const webViewStyles = {
tagStyles: {
em: {
fontFamily: fontFamily.GTA_ITALIC,
fontStyle: italic,
fontStyle: 'italic',
},

del: {
Expand All @@ -1418,7 +1417,7 @@ const webViewStyles = {

strong: {
fontFamily: fontFamily.GTA_BOLD,
fontWeight: fontWeightBold,
fontWeight: 'bold',
},

a: styles.link,
Expand Down