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

Optimize Theme with Assets Bundles and fixes missing lang attribute in HTML #81

Merged
merged 11 commits into from
Mar 9, 2023

Conversation

patrickfav
Copy link
Contributor

@patrickfav patrickfav commented Jan 2, 2023

This PR creates asset for all stylesheets and javascript files. By bundling all files into one and adding a hash value to the filename to support cache busting loading of the page is increased because its faster to load fewer bigger resources - this still keeps the maintainability of having the sources in seperate files.

Further improvements:

  • Adds missing required HTML attribute lang (issue found using Google Lighthouse)
  • Update jquery to 3.6.3
  • Add integrity attribute on scripts and stylsheets for improved security
  • Remove fork awesome stylesheet "source" files which are copied to the page output wasting space on the webserver

I made these optimiziations for my own site while using your theme so I figured the upstream project my use them as well.

@janraasch
Copy link
Collaborator

janraasch commented Feb 13, 2023

Hi @patrickfav, please excuse my late reply. This looks very good. Currently I don't have the time to test this on my machine - which I would like to do before merging. Actually, I am looking for somebody to take over ownership of this theme / repo to keep this theme up-2-date and develop it further. Would you be interested to take over? If so, please get in touch at [email protected].

layouts/partials/head.html Outdated Show resolved Hide resolved
@zjedi
Copy link
Owner

zjedi commented Mar 7, 2023

a later PR seems to partially overlap: #98
I'll get to review this on the weekend.

<script type="text/javascript" src='{{ "js/index.js" |absURL }}'></script>
{{ $scriptIcons := resources.Get "js/icons.js" }}
{{ $scriptIndex := resources.Get "js/index.js" }}
{{ $js := slice $scriptJquery $scriptIcons $scriptIndex | resources.Concat "js/script.js" | resources.Minify | resources.Fingerprint }}
Copy link
Owner

Choose a reason for hiding this comment

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

It works.
This basically adds production-quality (minification, fingerprint hash) - that's something we definitely want, although it may be a bit harder to tune the theme js alone. Hugo features used: https://gohugo.io/categories/asset-management/

I'm not sure if there is a simple way to keep the scripts linked separately for development, while automatically using this for publishing the final result. Maybe with some flag in config. Anyways, developers should be capable of finding a simple workaround if needed.

layouts/_default/single.html Show resolved Hide resolved
@zjedi
Copy link
Owner

zjedi commented Mar 8, 2023

To check: what is the minimal Hugo version that supports these features?

@patrickfav
Copy link
Contributor Author

@zjedi I dont know the exact minimal version, I use current version 0.109 -> I dont see a problem forcing this version though?

@zjedi
Copy link
Owner

zjedi commented Mar 9, 2023

@zjedi I dont know the exact minimal version, I use current version 0.109 -> I dont see a problem forcing this version though?

The pipelines feature seems to have been finished quite some time ago, in https://gohugo.io/news/0.46-relnotes/

The min required version of Hugo is currently ancient 0.41, from five years ago. https://github.com/janraasch/hugo-scroll/blob/master/theme.toml#L27

So yes, it's time for upgrade.

@zjedi zjedi merged commit 5152e5d into zjedi:master Mar 9, 2023
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.

3 participants