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.

Sign up for GitHub

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

Jump to bottom

✨IMPROVE: Move CSS hash to GET parameter #397

Merged
merged 3 commits into from
Sep 23, 2021
Merged

✨IMPROVE: Move CSS hash to GET parameter #397

merged 3 commits into from
Sep 23, 2021

Conversation

jacobtomlinson
Copy link
Contributor

@jacobtomlinson jacobtomlinson commented Sep 23, 2021 •
edited by choldgraf
Loading

Closes #376.

This PR makes use of the hash filter introduced in executablebooks/web-compile#5 (web compile will fail until this is merged and released) to add the hash to the URL at the end as a GET parameter rather than baking it into the filename.

This means that when the file changes and the hash is updated the cache will be busted, but the file still has a consistent name for referencing in downstream themes.

@choldgraf
Copy link
Member

Looks like executablebooks/web-compile#5 was merged - once there's a release I can merge this one in, and cut a release of the book theme

@chrisjsewell
Copy link
Member

Already released as v0.2.2 😉

chrisjsewell
chrisjsewell requested changes Sep 23, 2021
View reviewed changes
Copy link
Member

@chrisjsewell chrisjsewell left a comment

Choose a reason for hiding this comment

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

Cheers, we also need to remove src/jinja/theme.conf.j2 (and its generation in web-compile-config.yml) and then also change the name in sphinx_book_theme/theme.conf (and remove the comment at the top of that file)

@chrisjsewell
Copy link
Member

Cheers, we also need to remove src/jinja/theme.conf.j2 (and its generation in web-compile-config.yml) and then also change the name in sphinx_book_theme/theme.conf (and remove the comment at the top of that file)

Oh wait, sorry I misread the code, yeh I see what's happening now

@choldgraf
Copy link
Member

it looks to be working to me! here's the relevant HTML from the RTD build:

<html>
<body>
<!--StartFragment-->

<link rel="stylesheet" type="text/css" href="_static/pygments.css" />
--
  | <link rel="stylesheet" type="text/css" href="_static/sphinx-book-theme.css?digest=c3fdc42140077d1ad13ad2f1588a4309" />
  | <link rel="stylesheet" type="text/css" href="_static/togglebutton.css" />
  | <link rel="stylesheet" type="text/css" href="_static/mystnb.css" />

<!--EndFragment-->
</body>
</html>

@choldgraf choldgraf changed the title Move CSS hash to GET parameter ✨IMPROVE: Move CSS hash to GET parameter Sep 23, 2021
@choldgraf choldgraf merged commit c85925c into executablebooks:master Sep 23, 2021
@welcome
Copy link

welcome bot commented Sep 23, 2021

Congrats on your first merged pull request in this project! 🎉
congrats

Thank you for contributing, we are very proud of you! ❤️

@jacobtomlinson jacobtomlinson deleted the css-hash-digest branch September 24, 2021 09:08
@jacobtomlinson
Copy link
Contributor Author

Thanks for pushing to this and getting it over the line for me!

@choldgraf choldgraf mentioned this pull request Jan 2, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@choldgraf choldgraf choldgraf left review comments

@pradyunsg pradyunsg pradyunsg left review comments

@chrisjsewell chrisjsewell chrisjsewell approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Hash our CSS/JS assets with digest requests instead of in filenames
4 participants
@jacobtomlinson @choldgraf @chrisjsewell @pradyunsg