search

dask / dask-sphinx-theme

Sphinx theme for Dask documentation
BSD 3-Clause "New" or "Revised" License
6 stars 15 forks source link

Remove vendored fonts in favour of Google CDN #77

Closed jacobtomlinson closed 2 years ago

jacobtomlinson commented 2 years ago

Instead of vendoring font .ttf files directly into the theme I've switched them out for the hosted versions from Google Fonts. This should give us more flexibility in the future if we need to add more weights and styles as we can do this by just tweaking the URL instead of having to find and vendor in more font files and have the page make more requests.

Changes:

@scharlottej13 I noticed the Inter-SemiBold weight was set to bold, the same as Inter-Bold which was never used. I've updated things to just use inter with a weight of 700 which will not result in any changes. But did you intend to make the nav elements semi bold (weight of 600) or are we ok to leave this as it is?

This should also resolve the issues in https://github.com/dask/dask-examples/issues/224

scharlottej13 commented 2 years ago

Thanks @jacobtomlinson! this looks great! My intention was to make the nav elements 'heavier' than the body text, so I think we're good to leave as is.

If you have a second to explain, I'm still a little confused why using Google Fonts will fix dask-examples? I thought the vendored fonts were also packaged up with the rest of the dask-sphinx-theme?

jacobtomlinson commented 2 years ago

My intention was to make the nav elements 'heavier' than the body text, so I think we're good to leave as is.

Ok great.

If you have a second to explain, I'm still a little confused why using Google Fonts will fix dask-examples? I thought the vendored fonts were also packaged up with the rest of the dask-sphinx-theme?

I made these changes to try and reduce the number of HTTP requests each page makes and to make maintenance easier in the future. The tradeoff is depending on Google's font CDN instead of being self-sufficient, but I think Google Fonts is reliable and widely used so we are fine to depend on it.

You are right that it should work, and I'm not sure why it doesn't in that one case. This PR doesn't fix the issue in dask-examples but it does work around it which is useful 😄.

scharlottej13 commented 2 years ago

I made these changes to try and reduce the number of HTTP requests each page makes and to make maintenance easier in the future. The tradeoff is depending on Google's font CDN instead of being self-sufficient, but I think Google Fonts is reliable and widely used so we are fine to depend on it.

Good to know, thanks for taking the time to explain @jacobtomlinson!

jrbourbeau commented 2 years ago

Pushing out dask-sphinx-theme=3.0.3 to get these changes out