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

feat: added support for Disqus #120

Merged
merged 2 commits into from
Jan 13, 2020
Merged

feat: added support for Disqus #120

merged 2 commits into from
Jan 13, 2020

Conversation

dwmkerr
Copy link
Contributor

@dwmkerr dwmkerr commented Jan 5, 2020

This PR adds support for Disqus. If the standard Hugo disqusShortname configuration is provided, comments will be enabled for pages. This is a non-breaking change, comments are disabled by default.

image

Apologies if I've got something wrong here, am fairly new to Hugo!

Disqus can be disabled for any content by setting the following in the frontmatter:

disable_comments: true

See effectiive-shell for an example. Disqus is disabled on the main index page, and enabled for each chapter.

@alex-shpak
Copy link
Owner

Hi! Looks good, thanks for your contribution.
I would suggest few things:

  • Add space between footer and Disqus block (maybe 1 or 2 em)
  • Rename disable_comments to BookDisqus defaulting to true. To keep it consistent with other params.

Since DisqusShortname already checked here
https://github.com/gohugoio/hugo/blob/master/tpl/tplimpl/embedded/templates/disqus.html#L3

It could look like

{{ if default true .Params.BookDisqus }}
  {{ template "_internal/disqus.html" . }}
{{ end }}

Set:

    bookDisableComments: true

On a page to disable disqus for the content.
@dwmkerr
Copy link
Contributor Author

dwmkerr commented Jan 11, 2020

Hi @alex-shpak, sorry for the delay, I've been travelling! OK here are the updates:

  1. Added padding of 1rem to the disqus thread
  2. I've renamed disable_comments to bookDisableComments to keep the parameters consistent.

I don't think I'd fully updated the PR before; there are actually two new configuration fields:

  1. disqus_shortname - one of the standard fields from Hugo (https://gohugo.io/templates/internal/#configure-disqus). I've used this rather than bookDisqusShortname as a bit like the Google Analytics config it is one of the standard fields
  2. bookDisableComments - this is used in frontmatter only, to disable comments for specific content. For example, effective-shell has Disqus enabled via disqus_shortname, but comments are disabled on the frontpage only.

I think this makes sense; it means nothing should change for existing users, they still need to opt-in for disqus, but they can opt-out of pages if it doesn't make sense to have comments for them. Thanks for the heads up on the spacing too - looks much cleaner now!

BTW if you'd prefer me to make this a single commit, I can open a new PR with these commits squashed.

@alex-shpak
Copy link
Owner

Good,
I will merge and do some minor changes.

I will add BookComments site config, for global switch, which will have similar frontmatter switch.
To be able to disable by default and enable for some pages only.

@alex-shpak alex-shpak merged commit 36a8cf5 into alex-shpak:master Jan 13, 2020
@alex-shpak
Copy link
Owner

Pushed to master.
Global BookDisqus setting and per page bookDisqus frontmatter param

@dwmkerr
Copy link
Contributor Author

dwmkerr commented Jan 14, 2020

Thanks @alex-shpak, I appreciate the patience as I get familiar with the project!

@alex-shpak
Copy link
Owner

alex-shpak commented Jan 17, 2020

@dwmkerr Hi!
I made some changes to comments: 4b641b0
Comments got separate partial partials/docs/comments so that alternative commenting engine can be added. Also BookDisqus param is not BookComments

@dwmkerr
Copy link
Contributor Author

dwmkerr commented Jan 18, 2020

Looks great @alex-shpak, I think it's much cleaner the way you've structured it, really nice to be able to potential switch out comment systems 😄

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