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

SOLR-17342 Avoid loading from remote CDN #136

Merged
merged 6 commits into from
Jan 23, 2025

Conversation

janhoy
Copy link
Contributor

@janhoy janhoy commented Jan 22, 2025

See https://issues.apache.org/jira/browse/SOLR-17342 for background.

This PR stores some fonts and CSS locally in the static site instead of relying on CDN.

For the "Slick" carousel I cannot see that it is in use on our site now, so I have commented it out instead of downloading and adding to the repo. If you know that we'll need a carousel/slider we can re-enable it once we need it. Otherwise, the code can simply be removed.

I used the tool Google Font Downloader for downloading the two google fonts. FontAwesome was downloaded and added manually. It is not the latest version...

Tested the PR locally with ./build.sh -l

@janhoy janhoy requested review from epugh, raboof and dsmiley January 22, 2025 19:14
Copy link
Contributor

@epugh epugh left a comment

Choose a reason for hiding this comment

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

Didn't know about #} as a comment pattern. Thanks for this chore.

Add comment in css for unused slider css
@janhoy
Copy link
Contributor Author

janhoy commented Jan 22, 2025

Didn't know about #} as a comment pattern. Thanks for this chore.

I did not either, so I changed it to <!-- :-) My IDEA did it for me, since it believed that this file was a Jinja2 template, probably since it is inside a "templates" folder. It would be a valid Jinja block comment. But not HTML.

EDIT: Pelican templates folder is indeed parsed by Jinja2, so {# is a valid block comment

@janhoy
Copy link
Contributor Author

janhoy commented Jan 22, 2025

Interestingly, the staging site already reports errors due to CSP.
Skjermbilde 2025-01-22 kl  21 59 30

So after merge we'll be able to verify that these are now gone.

Add license headers and LICENSE files
@janhoy
Copy link
Contributor Author

janhoy commented Jan 22, 2025

I added licensing headers for CSS and LICENSE.txt files for the fonts to be more explicit

@janhoy janhoy merged commit 21e551f into apache:main Jan 23, 2025
1 check passed
@janhoy janhoy deleted the local-fonts-and-css branch January 23, 2025 09:48
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.

2 participants