Preload web fonts #12983

roryabraham · 2022-11-24T05:50:33Z

Details

This PR does a few things:

Adds the webpack-font-preload-plugin plugin to preload fonts. This helps to ensure that fonts are loaded faster and prevent FOUT, but causes fonts to be downloaded with higher priority than the app JS bundle, which can delay the initial render time of the main app. It was minor but noticeable, especially on slower connections. To optimize this trade-off for our mainline use cases, only the .woff2 font are preloaded. .woff fonts are not, which means that the preloading optimization won't benefit internet explorer the same as other browsers.
Updates the splash screen JS to ensure that we wait until fonts are loaded before rendering the main app, which should completely eliminate FOUT and make it so that we don't need to think about web-safe fallback fonts that look similar to our brand font. Note that there are some independent problems with the splash screen itself.
Also removed deprecated file-loader webpack plugin (no longer needed since webpack 5 introduced asset modules)

Fixed Issues

$ #12267

Tests (web)

With the network console open, load the site. You should see that the .woff2 fonts are downloaded right away.
Throttle your network connection so that you are on Slow 3G, and refresh the site.
Verify that once the site loads (takes about 60 seconds), you do not see a flash of unstyled text (FOUT)
For good measure, go to https://localhost:8080/statements/202301 and verify that the statements modal is visible in the RHP (this is testing that react-native-webview still works despite no longer using deprecated file-loader.

Tests (native)

Just regression tests – all the existing fonts should continue to display as expected.

Verify that no errors appear in the JS console

Offline tests

None.

QA Steps

Same as Tests. Marked as Internal QA because the testing steps are just a bit more technical than usual. Note that the splash screen also isn't working on production due to another unrelated issue.

Verify that no errors appear in the JS console

PR Author Checklist

PR Reviewer Checklist

The reviewer will copy/paste it into a new comment and complete it after the author checklist is completed

Screenshots/Videos

Web

Web.mov

Mobile Web - Chrome

Mobile Web - Safari

Desktop

Desktop.mov

iOS

iOS unaffected by this web-only change.

Android

Android unaffected by this web-only change.

roryabraham · 2022-11-24T17:17:06Z

Note to self and reviewers:

Preloading both the woff and woff2 files was noticeably slower. woff2 works on all browsers except IE, so we just preload woff2. We could still end up seeing FOUT on IE with this PR. We could probably configure webpack to provide the url of the woff fonts to JS via ProvidePlugin, then in splash.js check if the browser is IE and load the woff fonts with JS and wait for that to finish before hiding the splash screen. Didn't bother for now because IE at EOL

marcaaron · 2022-11-28T21:21:20Z

We could still end up seeing FOUT on IE with this PR

I don't think we support IE. We are not even trying to workaround Firefox bugs so IE seems out.

roryabraham · 2022-11-28T21:36:32Z

I don't think we support IE. We are not even trying to workaround Firefox bugs so IE seems out.

100% agree, IE is officially EOL

roryabraham · 2022-11-29T20:06:14Z

After chatting with @Luke9389 I'm going to put this on HOLD for #12656 (review)

Luke9389 · 2022-11-29T20:56:35Z

put myself down as a reviewer bc I have context

# Conflicts: # assets/css/fonts.css # assets/fonts/native/GTAmericaExp-BdIt.otf # assets/fonts/native/GTAmericaExp-Bold.otf # assets/fonts/native/GTAmericaExp-Light.otf # assets/fonts/native/GTAmericaExp-MdIt.otf # assets/fonts/native/GTAmericaExp-Medium.otf # assets/fonts/native/GTAmericaExp-Regular.otf # assets/fonts/native/GTAmericaExp-RgIt.otf # assets/fonts/native/GTAmericaExp-Thin.otf # assets/fonts/native/GTAmericaExpMono-Bd.otf # assets/fonts/native/GTAmericaExpMono-BdIt.otf # assets/fonts/native/GTAmericaExpMono-Rg.otf # assets/fonts/native/GTAmericaExpMono-RgIt.otf # config/webpack/webpack.common.js # ios/NewExpensify.xcodeproj/project.pbxproj # ios/NewExpensify/Info.plist

roryabraham · 2022-12-16T00:39:39Z

HOLDING on #13415

# Conflicts: # android/link-assets-manifest.json # assets/css/fonts.css # ios/NewExpensify.xcodeproj/project.pbxproj # ios/link-assets-manifest.json # package-lock.json # package.json

melvin-bot · 2023-02-04T21:03:22Z

Hey, I noticed you changed some webpack configuration files. This can break production builds. Did you remember to run a production build locally to verify they still work?

roryabraham · 2023-02-04T21:06:15Z

web/splash/splash.js

@@ -2,10 +2,15 @@ import './splash.css';
 import newExpensifyLogo from 'logo?raw';
 import themeColors from '../../src/styles/themes/default';

+let areFontsReady = false;
+document.fonts.ready.then(() => {


According to https://caniuse.com/mdn-api_document_fonts this works on everything except IE, which is officially EOL as of Summer 2022, so I really don't think we need to continue supporting it.

melvin-bot · 2023-02-04T21:40:14Z

@sobitneupane @neil-marcellini One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

sobitneupane

Screenshots/Videos

Web

Screen.Recording.2023-02-06.at.13.50.38.mov

Screen.Recording.2023-02-06.at.13.54.46.mov

Mobile Web - Chrome

Screen.Recording.2023-02-06.at.14.22.40.mov

Mobile Web - Safari

Screen.Recording.2023-02-06.at.14.23.54.mov

Desktop

Screen.Recording.2023-02-06.at.14.08.29.mov

iOS

Android

Screen.Recording.2023-02-06.at.14.20.51.mov

sobitneupane

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- Android / native
- Android / Chrome
- iOS / native
- iOS / Safari
- MacOS / Chrome / Safari
- MacOS / Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Luke9389

Good stuff 👍

OSBotify · 2023-02-06T21:27:08Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

shawnborton · 2023-02-07T11:53:41Z

Are we planning to do a follow-up issue to clean up the font family names too?

OSBotify · 2023-02-07T18:46:53Z

🚀 Deployed to staging by https://github.com/Luke9389 in version: 1.2.67-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

roryabraham · 2023-02-08T08:13:04Z

@shawnborton yep that's still on my radar. Issue here.

roryabraham · 2023-02-08T18:10:37Z

This passed internal QA:

OSBotify · 2023-02-08T23:42:19Z

🚀 Deployed to production by https://github.com/mountiny in version: 1.2.67-7 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

roryabraham added 12 commits November 22, 2022 20:27

Upgrade style-loader to v3, remove unnecessary separate css entry

fe70b13

Upgrade css-loader

898bf1f

Load woff2 first, then woff, then eot

28a3f48

Put web fonts in NewDot repo

b9fe024

Re-link fonts after moving them to assets/fonts/native

dedd5af

Use new webpack assets instead of file-loader

bfe703c

Host fonts in NewDot

4d99fc0

Use auto asset loader

61e9313

Drop support for IE 8 eot files

120ce27

Preload fonts with webpack-font-preload-plugin

7e67132

Only preload woff2 fonts

c0b82fe

Wait until fonts are ready to hide the splash screen

671b829

roryabraham mentioned this pull request Nov 28, 2022

[HOLD for payment 2023-02-15] [$250] Text is displayed in Times New roman when the site renders first #12267

Closed

Merge branch 'main' into Rory-PreloadFonts

3f5f01b

Luke9389 self-requested a review November 29, 2022 20:56

roryabraham added 10 commits December 1, 2022 17:08

Move updated fonts to separate directories for native and web

e14fd8b

Update webpack to look for otf as well as woff/woff2

ecf57ac

Reinstall pods

a9013ed

Add otf to preload for New Kansas

bdef535

Remove unused fonts

41c8312

Fix ExpensifyNewKansas

8755d0a

Relink assets

f693db1

Merge branch 'main' into Rory-PreloadFonts

3dec886

Remove unrelated style-loader change

7f1544d

roryabraham requested a review from shawnborton December 2, 2022 18:29

roryabraham self-assigned this Dec 2, 2022

roryabraham changed the title ~~Preload web fonts~~ [HOLD] Preload web fonts Dec 16, 2022

roryabraham added 5 commits February 3, 2023 16:24

Merge branch 'main' into Rory-PreloadFonts

958e92d

# Conflicts: # android/link-assets-manifest.json # assets/css/fonts.css # ios/NewExpensify.xcodeproj/project.pbxproj # ios/link-assets-manifest.json # package-lock.json # package.json

Remove otf fonts

9489d99

Reduce diff

d47bc69

Remove otf from FontPreloadPlugin

2356cc9

Remove old webpack file-loader

262b380

roryabraham commented Feb 4, 2023

View reviewed changes

roryabraham changed the title ~~[HOLD] Preload web fonts~~ Preload web fonts Feb 4, 2023

roryabraham marked this pull request as ready for review February 4, 2023 21:39

roryabraham requested a review from a team as a code owner February 4, 2023 21:39

melvin-bot bot requested review from neil-marcellini and sobitneupane and removed request for a team February 4, 2023 21:40

sobitneupane reviewed Feb 6, 2023

View reviewed changes

sobitneupane approved these changes Feb 6, 2023

View reviewed changes

Luke9389 approved these changes Feb 6, 2023

View reviewed changes

Luke9389 merged commit 26fdace into main Feb 6, 2023

Luke9389 deleted the Rory-PreloadFonts branch February 6, 2023 21:19

OSBotify mentioned this pull request Feb 7, 2023

Deploy Checklist: New Expensify 2023-02-07 #14911

Closed

21 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Preload web fonts #12983

Preload web fonts #12983

roryabraham commented Nov 24, 2022 •

edited

Loading

roryabraham commented Nov 24, 2022

marcaaron commented Nov 28, 2022

roryabraham commented Nov 28, 2022

roryabraham commented Nov 29, 2022

Luke9389 commented Nov 29, 2022

roryabraham commented Dec 16, 2022

melvin-bot bot commented Feb 4, 2023

roryabraham Feb 4, 2023

melvin-bot bot commented Feb 4, 2023

sobitneupane left a comment

sobitneupane left a comment

Luke9389 left a comment

OSBotify commented Feb 6, 2023

shawnborton commented Feb 7, 2023

OSBotify commented Feb 7, 2023

roryabraham commented Feb 8, 2023

roryabraham commented Feb 8, 2023

OSBotify commented Feb 8, 2023

Preload web fonts #12983

Preload web fonts #12983

Conversation

roryabraham commented Nov 24, 2022 • edited Loading

Details

Fixed Issues

Tests (web)

Tests (native)

Offline tests

QA Steps

PR Author Checklist

PR Reviewer Checklist

Screenshots/Videos

roryabraham commented Nov 24, 2022

marcaaron commented Nov 28, 2022

roryabraham commented Nov 28, 2022

roryabraham commented Nov 29, 2022

Luke9389 commented Nov 29, 2022

roryabraham commented Dec 16, 2022

melvin-bot bot commented Feb 4, 2023

roryabraham Feb 4, 2023

Choose a reason for hiding this comment

melvin-bot bot commented Feb 4, 2023

sobitneupane left a comment

Choose a reason for hiding this comment

Screenshots/Videos

sobitneupane left a comment

Choose a reason for hiding this comment

Reviewer Checklist

Luke9389 left a comment

Choose a reason for hiding this comment

OSBotify commented Feb 6, 2023

shawnborton commented Feb 7, 2023

OSBotify commented Feb 7, 2023

roryabraham commented Feb 8, 2023

roryabraham commented Feb 8, 2023

OSBotify commented Feb 8, 2023

roryabraham commented Nov 24, 2022 •

edited

Loading