chore: implement self-hosted fonts for improved privacy and performance#418
chore: implement self-hosted fonts for improved privacy and performance#418
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements self-hosted fonts to replace external Google Fonts CDN dependencies, improving privacy, performance, and reliability for the HugeGraph documentation website.
- Disabled Google Fonts loading and removed external CDN font imports
- Added comprehensive @font-face declarations for 4 font families with multiple weights
- Maintained multilingual support for Arabic, Persian, and Hebrew languages
Reviewed Changes
Copilot reviewed 3 out of 41 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| themes/docsy/assets/scss/_variables.scss | Disabled Google Fonts loading by setting flag to false |
| themes/docsy/assets/scss/rtl/_main.scss | Removed external font imports for RTL language support |
| assets/scss/_variables_project.scss | Added @font-face declarations for all self-hosted font families |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 41 changed files in this pull request and generated no new comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| font-style: normal; | ||
| font-weight: 300; | ||
| font-display: swap; | ||
| src: url('/assets/fonts/OpenSans-Light.woff2') format('woff2'), |
There was a problem hiding this comment.
Should Fix: Path consistency issue with font URLs
The font paths use absolute URLs (/assets/fonts/) which could break in subdirectory deployments or development environments. Consider using relative paths or Hugo's asset pipeline:
| src: url('/assets/fonts/OpenSans-Light.woff2') format('woff2'), | |
| src: url('../fonts/OpenSans-Light.woff2') format('woff2'), | |
| url('../fonts/OpenSans-Light.woff') format('woff'); |
This applies to all 38 @font-face declarations. Using relative paths would ensure the fonts load correctly regardless of the deployment path.
| $google_font_name: "Open Sans" !default; | ||
| $google_font_family: "Open+Sans:300,300i,400,400i,700,700i" !default; | ||
| $web-font-path: "https://fonts.googleapis.com/css?family=#{$google_font_family}&display=swap"; | ||
| // $web-font-path: "https://fonts.googleapis.com/css?family=#{$google_font_family}&display=swap"; // Disabled for self-hosted fonts |
There was a problem hiding this comment.
Should Fix: Commented code should be removed instead of disabled
Rather than commenting out the $web-font-path variable, it would be cleaner to remove it entirely since it's no longer needed with self-hosted fonts. Dead code comments can cause confusion for future maintainers.
| font-display: swap; | ||
| src: url('/assets/fonts/OpenSans-BoldItalic.woff2') format('woff2'), | ||
| url('/assets/fonts/OpenSans-BoldItalic.woff') format('woff'); | ||
| } |
There was a problem hiding this comment.
Nice to Have: Consider font subsetting for performance optimization
For the multilingual fonts (Arabic, Persian, Hebrew), consider using Unicode range subsetting to reduce file sizes for users who don't need these character sets:
| } | |
| @font-face { | |
| font-family: 'Rubik'; | |
| font-style: normal; | |
| font-weight: 300; | |
| font-display: swap; | |
| unicode-range: U+0590-05FF, U+FB1D-FB4F; /* Hebrew range */ | |
| src: url('/assets/fonts/Rubik-Light.woff2') format('woff2'), | |
| url('/assets/fonts/Rubik-Light.woff') format('woff'); | |
| } |
This would improve loading performance for users who don't need these specific language characters.
|
|
||
| */ | ||
|
|
||
| // Self-hosted font declarations |
There was a problem hiding this comment.
Nice to Have: Add documentation for font configuration
Consider adding a comment block at the top of this file documenting:
- Total font file sizes per family
- Browser compatibility notes (WOFF2 vs WOFF fallback strategy)
- Instructions for adding new font families
- Font preloading strategy for critical fonts
This would help future maintainers understand the font hosting setup and make informed decisions about modifications.
PR Review Summary✅ Overall AssessmentThe implementation for self-hosting fonts is technically sound and achieves the stated goals of improving privacy, performance, and reliability. The PR effectively removes external CDN dependencies. 📋 Review FindingsCritical Problems: None identified Should Fix (2 items):
Nice to Have (3 items):
🧪 Additional Testing Recommendations
✅ Pre-merge Checklist
The privacy and performance benefits justify the added font files. Main concerns are around path handling for different deployment scenarios. Great work on implementing this improvement! 👍 |
| $google_font_name: "Open Sans" !default; | ||
| $google_font_family: "Open+Sans:300,300i,400,400i,700,700i" !default; | ||
| $web-font-path: "https://fonts.googleapis.com/css?family=#{$google_font_family}&display=swap"; | ||
| // $web-font-path: "https://fonts.googleapis.com/css?family=#{$google_font_family}&display=swap"; // Disabled for self-hosted fonts |
There was a problem hiding this comment.
Commented-out code should be removed rather than left as a comment. Consider cleaner removal:
| // $web-font-path: "https://fonts.googleapis.com/css?family=#{$google_font_family}&display=swap"; // Disabled for self-hosted fonts | |
| // Removed for self-hosted fonts - see assets/scss/_variables_project.scss |
This improves maintainability by avoiding commented-out code.
| @@ -13,7 +13,7 @@ $enable-shadows: true !default; | |||
|
|
|||
There was a problem hiding this comment.
Critical: Avoid editing files under themes/. Set $td-enable-google-fonts: false in project override (assets/scss/_variables_project.scss) to prevent breakage on theme upgrades. Revert this change here and keep theme files pristine.
| @@ -66,7 +66,7 @@ $link-hover-decoration: none !default; | |||
|
|
|||
There was a problem hiding this comment.
Not necessary to comment out $web-font-path here if $td-enable-google-fonts: false is set via project override. Prefer keeping the theme file untouched and place overrides in assets/scss/_variables_project.scss.
close apache/hugegraph#2854
🚀 Description
This PR implements self-hosted fonts for the HugeGraph documentation website, replacing external CDN font loading with local font files. This change improves website privacy, performance, and reliability by eliminating external dependencies for font loading.
🔧 Changes Made
Font Implementation
Configuration Updates
$td-enable-google-fonts: falseassets/scss/_variables_project.scssMultilingual Support
📈 Benefits
🧪 Testing
hugo server📁 File Changes
assets/fonts/(WOFF and WOFF2 formats)assets/scss/_variables_project.scss- Added @font-face declarationsthemes/docsy/assets/scss/_variables.scss- Disabled Google Fontsthemes/docsy/assets/scss/rtl/_main.scss- Removed external font imports🔗 Related Issues
Addresses website performance and privacy concerns by implementing self-hosted fonts as requested.
Total size: ~1.8MB of font files added
Formats supported: WOFF2 (modern browsers) + WOFF (fallback)
Languages supported: English, Arabic, Persian/Farsi, Hebrew