Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update fonts #111

Merged
merged 1 commit into from
Feb 27, 2024
Merged

Update fonts #111

merged 1 commit into from
Feb 27, 2024

Conversation

ac000
Copy link
Member

@ac000 ac000 commented Feb 26, 2024

Update the bundled Open Sans fonts from
https://gwfh.mranftl.com/fonts/open-sans?subsets=latin,latin-ext

These includes Regular and Italic and Bold (700) and Bold (700) Italic for the latin and latin extended character sets.

Copy link

netlify bot commented Feb 26, 2024

Deploy Preview for nginx-unit ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 602658f
🔍 Latest deploy log https://app.netlify.com/sites/nginx-unit/deploys/65de0f02185e4b00092530f8
😎 Deploy Preview https://deploy-preview-111--nginx-unit.netlify.app/howto/falcon
📱 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 site configuration.

@callahad
Copy link
Collaborator

LGTM once the new woff2's are committed, thank you! Your commit message is missing its closing > on the URL. Didn't know about google-webfonts-helper; very nice.

In the future, it could be neat to ship a single variable font of Open Sans, but not worth the effort for the moment.

@ac000
Copy link
Member Author

ac000 commented Feb 27, 2024

Actually add the fonts

$ git range-diff 96fb35e...8dd0bfe
1:  96fb35e ! 1:  8dd0bfe [WIP] Update fonts
    @@ source/theme/static/OpenSans.woff (deleted)
     
      ## source/theme/static/OpenSans.woff2 (deleted) ##
      Binary files source/theme/static/OpenSans.woff2 and /dev/null differ
    +
    + ## source/theme/static/open-sans-v40-latin_latin-ext-700.woff2 (new) ##
    + Binary files /dev/null and source/theme/static/open-sans-v40-latin_latin-ext-700.woff2 differ
    +
    + ## source/theme/static/open-sans-v40-latin_latin-ext-700italic.woff2 (new) ##
    + Binary files /dev/null and source/theme/static/open-sans-v40-latin_latin-ext-700italic.woff2 differ
    +
    + ## source/theme/static/open-sans-v40-latin_latin-ext-italic.woff2 (new) ##
    + Binary files /dev/null and source/theme/static/open-sans-v40-latin_latin-ext-italic.woff2 differ
    +
    + ## source/theme/static/open-sans-v40-latin_latin-ext-regular.woff2 (new) ##
    + Binary files /dev/null and source/theme/static/open-sans-v40-latin_latin-ext-regular.woff2 differ

@ac000
Copy link
Member Author

ac000 commented Feb 27, 2024

Tweak commit message

$ git range-diff 8dd0bfe...b5d7741
1:  8dd0bfe ! 1:  b5d7741 [WIP] Update fonts
    @@ Metadata
     Author: Andrew Clayton <a.clayton@nginx.com>
     
      ## Commit message ##
    -    [WIP] Update fonts
    +    Update fonts
     
    -    Update the bundled Open Sans fonts from
    -    <https://gwfh.mranftl.com/fonts/open-sans?subsets=latin,latin-ext
    +    Update the bundled Open Sans fonts using
    +    <https://gwfh.mranftl.com/fonts/open-sans?subsets=latin,latin-ext>
     
    -    These includes Regular and Italic and Bold (700) and Bold (700) Italic
    +    These include Regular and Italic and Bold (700) and Bold (700) Italic
         for the latin and latin extended character sets.
     
         Closes: https://github.com/nginx/unit-docs/issues/109

@ac000 ac000 changed the title [WIP] Update fonts Update fonts Feb 27, 2024
@callahad
Copy link
Collaborator

I'm not actually seeing the fonts in the deploy preview; the woff2's are 404'ing it's falling back to Arial.

@ac000 ac000 marked this pull request as ready for review February 27, 2024 13:20
Copy link
Collaborator

@callahad callahad left a comment

Choose a reason for hiding this comment

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

The font URLs should point to _static/, not static/ :)

@ac000
Copy link
Member Author

ac000 commented Feb 27, 2024

Expand the commit message a little regarding WOFF vs WOFF2

$ git range-diff b5d7741...a87c9ab
1:  b5d7741 ! 1:  a87c9ab Update fonts
    @@ Commit message
         These include Regular and Italic and Bold (700) and Bold (700) Italic
         for the latin and latin extended character sets.
     
    +    This removes the older WOFF format font and we now only use WOFF2 fonts.
    +
    +    WOFF2 offers better compression amongst other things. It is supported by
    +    all modern browsers...
    +
    +      Edge (since version 14)
    +      Google Chrome (since version 36)
    +      Firefox (since version 35)
    +      Opera (since version 26)
    +      Safari (since version 10)
    +
         Closes: https://github.com/nginx/unit-docs/issues/109
         Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
     

@ac000
Copy link
Member Author

ac000 commented Feb 27, 2024

The font URLs should point to _static/, not static/ :)

static/ surely? (Yes, I had ../static) the directory is static/ and not _static/ though...

@ac000
Copy link
Member Author

ac000 commented Feb 27, 2024

Fix font paths

$ git range-diff a87c9ab...705adf2
1:  a87c9ab ! 1:  705adf2 Update fonts
    @@ source/theme/layout.html
     +  font-style: normal;
     +  font-weight: 400;
     +  src: local('OpenSans'), local('Open Sans'), local('Open Sans Regular'), local('OpenSans-Regular'),
    -+       url('../static/open-sans-v40-latin_latin-ext-regular.woff2') format('woff2');
    ++       url('static/open-sans-v40-latin_latin-ext-regular.woff2') format('woff2');
      }
     +
     +/* open-sans-italic - latin_latin-ext */
    @@ source/theme/layout.html
     +  font-style: italic;
     +  font-weight: 400;
     +  src: local('OpenSansItalic'), local('Open Sans Italic'), local('OpenSans Italic'), local('OpenSans-Italic'),
    -+       url('../static/open-sans-v40-latin_latin-ext-italic.woff2') format('woff2');
    ++       url('static/open-sans-v40-latin_latin-ext-italic.woff2') format('woff2');
     +}
     +
     +/* open-sans-700 - latin_latin-ext */
    @@ source/theme/layout.html
     +  font-style: normal;
     +  font-weight: 700;
     +  src: local('OpenSansBold'), local('Open Sans Bold'), local('OpenSans Bold'), local('OpenSans-Bold'),
    -+       url('../static/open-sans-v40-latin_latin-ext-700.woff2') format('woff2');
    ++       url('static/open-sans-v40-latin_latin-ext-700.woff2') format('woff2');
     +}
     +
     +/* open-sans-700italic - latin_latin-ext */
    @@ source/theme/layout.html
     +  font-style: italic;
     +  font-weight: 700;
     +  src: local('OpenSansBoldItalic'), local('Open Sans Bold Italic'), local('OpenSans Bold Italic'), local('OpenSans-BoldItalic'), local('OpenSans-Bold-Italic'),
    -+       url('../static/open-sans-v40-latin_latin-ext-700italic.woff2') format('woff2');
    ++       url('static/open-sans-v40-latin_latin-ext-700italic.woff2') format('woff2');
     +}
     +
      </style>

@callahad
Copy link
Collaborator

During build, static files get collected into _static/

You can drop the relative bits and use /_static/ as an absolute path, though; we're never going to deploy this as a subdirectory of anything else.

@ac000
Copy link
Member Author

ac000 commented Feb 27, 2024

OK, I'll try that because this is giving me a 404...

https://deploy-preview-111--nginx-unit.netlify.app/installation/static/open-sans-v40-latin_latin-ext-700.woff2

@ac000
Copy link
Member Author

ac000 commented Feb 27, 2024

Update font paths again...

$ git range-diff 705adf2...676429e
1:  705adf2 ! 1:  676429e Update fonts
    @@ source/theme/layout.html
     +  font-style: normal;
     +  font-weight: 400;
     +  src: local('OpenSans'), local('Open Sans'), local('Open Sans Regular'), local('OpenSans-Regular'),
    -+       url('static/open-sans-v40-latin_latin-ext-regular.woff2') format('woff2');
    ++       url('/_static/open-sans-v40-latin_latin-ext-regular.woff2') format('woff2');
      }
     +
     +/* open-sans-italic - latin_latin-ext */
    @@ source/theme/layout.html
     +  font-style: italic;
     +  font-weight: 400;
     +  src: local('OpenSansItalic'), local('Open Sans Italic'), local('OpenSans Italic'), local('OpenSans-Italic'),
    -+       url('static/open-sans-v40-latin_latin-ext-italic.woff2') format('woff2');
    ++       url('/_static/open-sans-v40-latin_latin-ext-italic.woff2') format('woff2');
     +}
     +
     +/* open-sans-700 - latin_latin-ext */
    @@ source/theme/layout.html
     +  font-style: normal;
     +  font-weight: 700;
     +  src: local('OpenSansBold'), local('Open Sans Bold'), local('OpenSans Bold'), local('OpenSans-Bold'),
    -+       url('static/open-sans-v40-latin_latin-ext-700.woff2') format('woff2');
    ++       url('/_static/open-sans-v40-latin_latin-ext-700.woff2') format('woff2');
     +}
     +
     +/* open-sans-700italic - latin_latin-ext */
    @@ source/theme/layout.html
     +  font-style: italic;
     +  font-weight: 700;
     +  src: local('OpenSansBoldItalic'), local('Open Sans Bold Italic'), local('OpenSans Bold Italic'), local('OpenSans-BoldItalic'), local('OpenSans-Bold-Italic'),
    -+       url('static/open-sans-v40-latin_latin-ext-700italic.woff2') format('woff2');
    ++       url('/_static/open-sans-v40-latin_latin-ext-700italic.woff2') format('woff2');
     +}
     +
      </style>

@callahad
Copy link
Collaborator

That's working! Woo!

Looking at the diff one more time, I noticed that we were using a helper function for building paths (and appending hashes for cache-busting). If we want to follow precedent:

diff --git a/source/theme/layout.html b/source/theme/layout.html
index 3bd369a..27d2797 100644
--- a/source/theme/layout.html
+++ b/source/theme/layout.html
@@ -37,7 +37,7 @@
   font-style: normal;
   font-weight: 400;
   src: local('OpenSans'), local('Open Sans'), local('Open Sans Regular'), local('OpenSans-Regular'),
-       url('/_static/open-sans-v40-latin_latin-ext-regular.woff2') format('woff2');
+       url('{{ pathto('/_static/open-sans-v40-latin_latin-ext-regular.woff2', 1) + '?' + md5('theme/static/open-sans-v40-latin_latin-ext-regular.woff2') }}') format('woff2');
 }
 
 /* open-sans-italic - latin_latin-ext */
@@ -47,7 +47,7 @@
   font-style: italic;
   font-weight: 400;
   src: local('OpenSansItalic'), local('Open Sans Italic'), local('OpenSans Italic'), local('OpenSans-Italic'),
-       url('/_static/open-sans-v40-latin_latin-ext-italic.woff2') format('woff2');
+       url('{{ pathto('/_static/open-sans-v40-latin_latin-ext-italic.woff2', 1) + '?' + md5('theme/static/open-sans-v40-latin_latin-ext-italic.woff2') }}') format('woff2');
 }
 
 /* open-sans-700 - latin_latin-ext */
@@ -57,7 +57,7 @@
   font-style: normal;
   font-weight: 700;
   src: local('OpenSansBold'), local('Open Sans Bold'), local('OpenSans Bold'), local('OpenSans-Bold'),
-       url('/_static/open-sans-v40-latin_latin-ext-700.woff2') format('woff2');
+       url('{{ pathto('/_static/open-sans-v40-latin_latin-ext-700.woff2', 1) + '?' + md5('theme/static/open-sans-v40-latin_latin-ext-700.woff2') }}') format('woff2');
 }
 
 /* open-sans-700italic - latin_latin-ext */
@@ -67,7 +67,7 @@
   font-style: italic;
   font-weight: 700;
   src: local('OpenSansBoldItalic'), local('Open Sans Bold Italic'), local('OpenSans Bold Italic'), local('OpenSans-BoldItalic'), local('OpenSans-Bold-Italic'),
-       url('/_static/open-sans-v40-latin_latin-ext-700italic.woff2') format('woff2');
+       url('{{ pathto('/_static/open-sans-v40-latin_latin-ext-700italic.woff2', 1) + '?' + md5('theme/static/open-sans-v40-latin_latin-ext-700italic.woff2') }}') format('woff2');
 }
 
 </style>

@ac000
Copy link
Member Author

ac000 commented Feb 27, 2024

I'll see if it still works with your change...

@ac000
Copy link
Member Author

ac000 commented Feb 27, 2024

Yeah it works...

@ac000
Copy link
Member Author

ac000 commented Feb 27, 2024

Use the css pathto magic + md5 sum as before

$ git range-diff 533e74a...b328d89
1:  676429e ! 1:  b328d89 Update fonts
    @@ source/theme/layout.html
     +  font-style: normal;
     +  font-weight: 400;
     +  src: local('OpenSans'), local('Open Sans'), local('Open Sans Regular'), local('OpenSans-Regular'),
    -+       url('/_static/open-sans-v40-latin_latin-ext-regular.woff2') format('woff2');
    ++       url('{{ pathto('/_static/open-sans-v40-latin_latin-ext-regular.woff2', 1) + '?' + md5('theme/static/open-sans-v40-latin_latin-ext-regular.woff2') }}') format('woff2');
      }
     +
     +/* open-sans-italic - latin_latin-ext */
    @@ source/theme/layout.html
     +  font-style: italic;
     +  font-weight: 400;
     +  src: local('OpenSansItalic'), local('Open Sans Italic'), local('OpenSans Italic'), local('OpenSans-Italic'),
    -+       url('/_static/open-sans-v40-latin_latin-ext-italic.woff2') format('woff2');
    ++       url('{{ pathto('/_static/open-sans-v40-latin_latin-ext-italic.woff2', 1) + '?' + md5('theme/static/open-sans-v40-latin_latin-ext-italic.woff2') }}') format('woff2');
     +}
     +
     +/* open-sans-700 - latin_latin-ext */
    @@ source/theme/layout.html
     +  font-style: normal;
     +  font-weight: 700;
     +  src: local('OpenSansBold'), local('Open Sans Bold'), local('OpenSans Bold'), local('OpenSans-Bold'),
    -+       url('/_static/open-sans-v40-latin_latin-ext-700.woff2') format('woff2');
    ++       url('{{ pathto('/_static/open-sans-v40-latin_latin-ext-700.woff2', 1) + '?' + md5('theme/static/open-sans-v40-latin_latin-ext-700.woff2') }}') format('woff2');
     +}
     +
     +/* open-sans-700italic - latin_latin-ext */
    @@ source/theme/layout.html
     +  font-style: italic;
     +  font-weight: 700;
     +  src: local('OpenSansBoldItalic'), local('Open Sans Bold Italic'), local('OpenSans Bold Italic'), local('OpenSans-BoldItalic'), local('OpenSans-Bold-Italic'),
    -+       url('/_static/open-sans-v40-latin_latin-ext-700italic.woff2') format('woff2');
    ++       url('{{ pathto('/_static/open-sans-v40-latin_latin-ext-700italic.woff2', 1) + '?' + md5('theme/static/open-sans-v40-latin_latin-ext-700italic.woff2') }}') format('woff2');
     +}
     +
      </style>
2:  533e74a < -:  ------- [TEST] Use pathto magic...

@callahad
Copy link
Collaborator

Check out this sweet before/after. We have bold!

Screenshot 2024-02-27 at 16 13 09

Update the bundled Open Sans fonts using
<https://gwfh.mranftl.com/fonts/open-sans?subsets=latin,latin-ext>

These include Regular and Italic and Bold (700) and Bold (700) Italic
for the latin and latin extended character sets.

This removes the older WOFF format font and we now only use WOFF2 fonts.

WOFF2 offers better compression amongst other things. It is supported by
all modern browsers...

  Edge (since version 14)
  Google Chrome (since version 36)
  Firefox (since version 35)
  Opera (since version 26)
  Safari (since version 10)

Closes: nginx#109
Reviewed-by: Dan Callahan <d.callahan@f5.com>
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
@ac000
Copy link
Member Author

ac000 commented Feb 27, 2024

Add Dan's Reviewed-by

$ git range-diff b328d89...602658f
1:  b328d89 ! 1:  602658f Update fonts
    @@ Commit message
           Safari (since version 10)
     
         Closes: https://github.com/nginx/unit-docs/issues/109
    +    Reviewed-by: Dan Callahan <d.callahan@f5.com>
         Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
     
      ## source/theme/layout.html ##

@ac000 ac000 merged commit 602658f into nginx:master Feb 27, 2024
4 checks passed
@ac000 ac000 deleted the fonts branch February 27, 2024 16:35
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.

Our bundled copy of Open Sans doesn't include italics or bold weights
2 participants