Skip to content

Replace Google Font with local variable font#1910

Merged
bjohansebas merged 13 commits intoexpressjs:gh-pagesfrom
utkarsh125:removal-of-google-fonts
May 28, 2025
Merged

Replace Google Font with local variable font#1910
bjohansebas merged 13 commits intoexpressjs:gh-pagesfrom
utkarsh125:removal-of-google-fonts

Conversation

@utkarsh125
Copy link
Copy Markdown
Contributor

@utkarsh125 utkarsh125 commented May 20, 2025

fixes: #1750

I have installed fontsource Open Sans and removed the script link for Google's Open Sans as discussed in the issue.

Packages installed

npm install @fontsource/open-sans

If any more changes are needed I would be happy to make changes

@utkarsh125 utkarsh125 requested review from a team as code owners May 20, 2025 21:19
@netlify
Copy link
Copy Markdown

netlify Bot commented May 20, 2025 •
edited
Loading

✅ Deploy Preview for expressjscom-preview ready!

Name Link
🔨 Latest commit 0167c97
🔍 Latest deploy log https://app.netlify.com/projects/expressjscom-preview/deploys/6836db141392110008640029
😎 Deploy Preview https://deploy-preview-1910--expressjscom-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@utkarsh125 utkarsh125 changed the title Removed the Google OpenSans with fontsource/open-sans Removed the Google OpenSans, Replaced with fontsource/open-sans May 20, 2025
ayuhito
ayuhito reviewed May 21, 2025
View reviewed changes
Comment thread package.json Outdated
ShubhamOulkar
ShubhamOulkar requested changes May 24, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@ShubhamOulkar ShubhamOulkar left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Please remove unused font files and folders. Also add local font names for all fonts, see suggestions below.

I tried to find the fontsource-variable fonts for open-sans but I was not able to find

You can use variable fonts from this repo. This will solve the problem with woff file size. See fontsource/fontsource#773 for more info.

Comment thread css/style.css Outdated
Comment thread fonts/open-sans/woff2/open-sans-latin-ext-300-italic.woff2 Outdated
Comment thread fonts/open-sans/fonts.css Outdated
Comment thread fonts/open-sans/fonts.css Outdated
Comment thread fonts/open-sans/fonts.css Outdated
ShubhamOulkar
ShubhamOulkar requested changes May 25, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@ShubhamOulkar ShubhamOulkar left a comment

Choose a reason for hiding this comment

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

Please remove ttf folder and rename font files.

Comment thread fonts/open-sans/fonts.css
Comment thread fonts/open-sans/fonts.css
@ShubhamOulkar ShubhamOulkar changed the title Removed the Google OpenSans, Replaced with fontsource/open-sans Replace Google Font with local variable font May 25, 2025
ShubhamOulkar
ShubhamOulkar approved these changes May 25, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@ShubhamOulkar ShubhamOulkar left a comment

Choose a reason for hiding this comment

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

LGTM 👍 manually tested on Firefox and Chrome. Fonts are correctly applied and rendering as expected.
Thanks @utkarsh125, for the changes.

@utkarsh125
Copy link
Copy Markdown
Contributor Author

utkarsh125 commented May 25, 2025 •
edited
Loading

Thank you @ShubhamOulkar for pointing out mistakes and guiding me on how to fix them. It was a good learning experience for me, hope to contribute more in the future :D

bjohansebas
bjohansebas reviewed May 28, 2025
View reviewed changes
Comment thread css/style.css Outdated
bjohansebas
bjohansebas requested changes May 28, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@bjohansebas bjohansebas left a comment

Choose a reason for hiding this comment

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

#1910 (comment)

ShubhamOulkar
ShubhamOulkar requested changes May 28, 2025
View reviewed changes
Comment thread _includes/head.html
Comment thread _includes/head.html Outdated
Comment thread _includes/head.html
ShubhamOulkar
ShubhamOulkar approved these changes May 28, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@ShubhamOulkar ShubhamOulkar left a comment

Choose a reason for hiding this comment

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

Manually tested. LGTM

image

Comment thread _includes/head.html Outdated
@bjohansebas bjohansebas merged commit 4e625d4 into expressjs:gh-pages May 28, 2025
10 checks passed
@bjohansebas
Copy link
Copy Markdown
Member

bjohansebas commented May 28, 2025

thanks @utkarsh125

@utkarsh125
Copy link
Copy Markdown
Contributor Author

utkarsh125 commented May 28, 2025

You're welcome @bjohansebas looking forward to contributing more in future :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ShubhamOulkar ShubhamOulkar ShubhamOulkar approved these changes

@bjohansebas bjohansebas bjohansebas approved these changes

+1 more reviewer

@ayuhito ayuhito ayuhito left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Remove Google Fonts

4 participants

@utkarsh125 @bjohansebas @ayuhito @ShubhamOulkar