Skip to content

simple dynamic load of giscus#12657

Merged
gordonwoodhull merged 1 commit intov1.7from
bugfix/load-giscus-generate-script-tag-simple-1.7
May 1, 2025
Merged

simple dynamic load of giscus#12657
gordonwoodhull merged 1 commit intov1.7from
bugfix/load-giscus-generate-script-tag-simple-1.7

Conversation

@gordonwoodhull
Copy link
Copy Markdown
Member

@gordonwoodhull gordonwoodhull commented May 1, 2025

Solves wrong-theming of Giscus in Chrome when Giscus is scrolled off-screen.

Generate script tag based on user preference at page load.
No need for mutation observer because class is already set.

This is the v1.7 backport of #12659

@posit-snyk-bot
Copy link
Copy Markdown
Collaborator

posit-snyk-bot commented May 1, 2025

🎉 Snyk checks have passed. No issues have been found so far.

security/snyk check is complete. No issues have been found. (View Details)

license/snyk check is complete. No issues have been found. (View Details)

generate script tag based on user preference at page load
no need for any mutation observers, because class is already set
@gordonwoodhull gordonwoodhull force-pushed the bugfix/load-giscus-generate-script-tag-simple-1.7 branch from fb79ed1 to e0d0702 Compare May 1, 2025 19:53
@gordonwoodhull gordonwoodhull marked this pull request as ready for review May 1, 2025 19:58
@gordonwoodhull gordonwoodhull merged commit 027acbf into v1.7 May 1, 2025
44 checks passed
@gordonwoodhull gordonwoodhull deleted the bugfix/load-giscus-generate-script-tag-simple-1.7 branch May 1, 2025 22:09
const getTheme = () => {
let baseTheme = document.getElementById('giscus-base-theme').value;
let altTheme = document.getElementById('giscus-alt-theme').value;
if (authorPrefersDark) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Where is this authorPrefersDark variable defined? Utterances has stopped working for me because of this. Chrome raises an error due to undefined variable.

If I add the variable to the top of the page, it works.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for reporting, @amitness! The bug with Giscus has already been reported and fixed (#12701) and will be released in the next patch release of Quarto. Sorry for the inconvenience.

I don't think you mean Utterances, because the loadGiscus() function is not included for Utterances.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great to know, thank you! And yes I meant giscus.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fix released in 1.7.31

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