opt: do not block for load font, close #1089#1119
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a performance optimization by modifying how external fonts are loaded. The change aims to enhance the user experience by ensuring that font loading does not block the initial rendering of the web page, leading to faster perceived load times. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a performance optimization by changing the way Google Fonts are loaded to be non-render-blocking. The implementation uses rel="preload" with an onload handler to apply the stylesheet asynchronously, with a noscript fallback. This is a good improvement.
However, I've noticed that the Manrope font is being loaded, but it doesn't seem to be defined in the application's CSS. This leads to an unnecessary download, which negatively impacts performance. I've added a comment with a suggestion to remove the unused font from the loading URLs.
| <link | ||
| rel="preload" | ||
| as="style" | ||
| href="https://fonts.googleapis.com/css2?family=Inter:ital,opsz,wght@0,14..32,100..900;1,14..32,100..900&family=Manrope:[email protected]&display=swap" | ||
| rel="stylesheet" | ||
| onload="this.onload=null;this.rel='stylesheet'" | ||
| /> | ||
| <noscript> | ||
| <link | ||
| rel="stylesheet" | ||
| href="https://fonts.googleapis.com/css2?family=Inter:ital,opsz,wght@0,14..32,100..900;1,14..32,100..900&family=Manrope:[email protected]&display=swap" | ||
| /> | ||
| </noscript> |
There was a problem hiding this comment.
The Manrope font is being loaded here (both in the preload link and the noscript fallback), but it doesn't appear to be defined or used in index.css. This results in an unnecessary download, impacting performance.
If Manrope is not used, please remove it from the font loading URLs as suggested. If it is intended to be used, it needs to be added to the CSS theme definitions in index.css.
| <link | |
| rel="preload" | |
| as="style" | |
| href="https://fonts.googleapis.com/css2?family=Inter:ital,opsz,wght@0,14..32,100..900;1,14..32,100..900&family=Manrope:[email protected]&display=swap" | |
| rel="stylesheet" | |
| onload="this.onload=null;this.rel='stylesheet'" | |
| /> | |
| <noscript> | |
| <link | |
| rel="stylesheet" | |
| href="https://fonts.googleapis.com/css2?family=Inter:ital,opsz,wght@0,14..32,100..900;1,14..32,100..900&family=Manrope:[email protected]&display=swap" | |
| /> | |
| </noscript> | |
| <link | |
| rel="preload" | |
| as="style" | |
| href="https://fonts.googleapis.com/css2?family=Inter:ital,opsz,wght@0,14..32,100..900;1,14..32,100..900&display=swap" | |
| onload="this.onload=null;this.rel='stylesheet'" | |
| /> | |
| <noscript> | |
| <link | |
| rel="stylesheet" | |
| href="https://fonts.googleapis.com/css2?family=Inter:ital,opsz,wght@0,14..32,100..900;1,14..32,100..900&display=swap" | |
| /> | |
| </noscript> |
1 participant
Uh oh!
There was an error while loading. Please reload this page.