Skip to content

Conversation

@utkarsh125
Copy link
Contributor

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

netlify bot commented May 20, 2025

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
Copy link
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 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.

Copy link
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.

@ShubhamOulkar ShubhamOulkar changed the title Removed the Google OpenSans, Replaced with fontsource/open-sans Replace Google Font with local variable font May 25, 2025
Copy link
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
Contributor Author

utkarsh125 commented May 25, 2025

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

Copy link
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.

Copy link
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

@bjohansebas bjohansebas merged commit 4e625d4 into expressjs:gh-pages May 28, 2025
10 checks passed
@bjohansebas
Copy link
Member

thanks @utkarsh125

@utkarsh125
Copy link
Contributor Author

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove Google Fonts

4 participants