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

Added Staticman submit comments and subscribe to comment threads #438

Closed
wants to merge 20 commits into from

Conversation

binarymist
Copy link

@binarymist binarymist commented Jan 8, 2018

Initial commit of Add support for Staticman commenting system

I'm not sure how you would like me to address the CSS colours, just let me know what changes you would like? This feature can be seen in action here

I've done a blog post on this, but it won't be going live until the end of next month. I'm happy to tweak it to provide documentation for this feature, or just create a new post outlining how to turn Staticman on.

Contact me and I'll flick

The documentation for this is in a post going out at the end of this month. Check it to help get your head around how things hang together https://github.com/binarymist/BinaryMistBlog/blob/post-static-man/content/post/hugo-with-staticman-commenting-and-subscriptions.md

To turn Staticman on, the staticman.yml file is also required, but this needs to be customised to suite the website owners requirements.

@eduardoboucas example staticman.sample.yml
My staticman.yml

I also need to add the following to each of my posts front matter. Where do I need to add this to?

# Slug is required for counting comments.
slug = "hugo-with-staticman-commenting-and-subscriptions"

I've also added the subscribe to be notified of new blog posts to my own overrides. Currently my overrides are only supporting Staticman, let me know if/when you would like a PR for this also. I'll probably have to add some more conditionals if you want this extra feature. It can be seen right at the top here

@gcushen
Copy link
Collaborator

gcushen commented Jan 8, 2018

Thanks for submitting this. I'll try to review it when I get a chance.

I noticed a few issues already such as:

  1. the code not using the localization files for language strings (they shouldn't be hard-coded into HTML)
  2. why is slug explicitly required in a post front-matter when Hugo already generates slugs based on the filename?

Documentation will need to be added to the documentation website if this PR is accepted. You'll find an Edit button at the top of the docs pages to edit them. May be best to wait until once this PR has been reviewed though.

@binarymist
Copy link
Author

binarymist commented Jan 9, 2018

  1. Made this change. Not sure about locales other than en. What do you usually do here?
  2. If you are talking about .Source.BaseFileName it doesn't seem to have a value in article_metadata.html. Is there another way to access the slug from within article_metadata.html, if so, what is it, and I'll use that instead?

Happy to add docs when you want.

gcushen and others added 4 commits January 13, 2018 02:52
No longer attempt to duplicate content from documentation website. Just
link to it.

Replace Readme's theme table with new gallery shortcode.
Use `relLangURL` rather than `relURL` for project links.

See HugoBlox#442
@binarymist
Copy link
Author

Any progress on this?
Thanks.

@gcushen
Copy link
Collaborator

gcushen commented Jan 24, 2018

Generally it looks good! However, it's a lot of changes and at the end of the day, I am the person who will likely end up having to try to maintain it.

Are you able to simplify it further? Especially with respect to the amount of additions for the language and style files.

Also, did you manage to fix the issue from point (2) above regarding having to manually define slugs in every single content file? Otherwise, that could be a blocker.

@binarymist
Copy link
Author

binarymist commented Jan 24, 2018

I wouldn't have done the work twice if I wasn't prepared to help maintian it. I just thought that there must be others using academic that would like/benefit from an open commenting system. I'm happy to assist with this going forward, as I'll be using it myself for years to come.

I can simplify it as much as you think necessary but while still maintianing the functionality that most would benefit from. Just let me know.

In regards to point 2, as I mentioned, the reason I added the slug to the post front-matter, is because I wasn't sure of any other way to access it in article_metadata.html, as .Source.BaseFileName doesn't have a value in article_metadata.html. Is there another way to access the slug from that context?

@gcushen
Copy link
Collaborator

gcushen commented Jan 24, 2018

Glad to hear that you would be happy to help maintain it going forward :)

RE point 2, you can use {{ $.Source.BaseFileName }} after line 2 in article_metadata.html where the $ context is established.

@binarymist
Copy link
Author

Ok, that works.

@binarymist
Copy link
Author

How's progress on this?

@gcushen
Copy link
Collaborator

gcushen commented Feb 6, 2018

It appears that you merged master updates into this PR branch, so I can no longer easily see your changes (take a look at the Files Changed tab in Github). Perhaps rebasing can help fix this...

As per my previous comments, please simplify the changes as much as possible. Especially with respect to the amount of additions for the language and style files.

This PR remains low priority compared to the other open issues which were submitted prior to it. Feel free to help resolve the other open issues if you have time, especially the help wanted ones :)

@binarymist
Copy link
Author

binarymist commented Feb 7, 2018

That's right, merged master into this branch to keep up to date. Diffing from master to this branch should show my changes.

I believe I have simplified as much as possible. I was under the impression due to your comment:

the code not using the localization files for language strings (they shouldn't be hard-coded into HTML)

that you wanted the variable values that people see added to the language files? Do you now want these removed again, or am I misunderstanding your request? Please clarify?

I think in regards to the CSS, if I remove it, then users will only have to add it back as overrides. Please propose an alternative working solution if you're not happy with what I've submitted and I can alter to suit.

I understand this PR is not high priority, but the other open issues submitted before this were not high priority for me as far as I can tell.

I could rebase, or do what ever is necessary, but I still don't know if you even want this?

I don't have time sadly, but just thought that if I'm getting benefit from staticman, then others should to, and I seemed to be in the best position to help others benefit by submitting PR, so I took the hit and subbed PR.

Feel free to close if you think others will not appreciate the free and open commenting system in academic.

@netw0rkf10w
Copy link

@binarymist Great resource! I was looking for a commenting system and it turns out Staticman best suits my needs. While waiting for it to be merged, I'll follow your tutorial to add it to my site. Thanks a lot!

@binarymist
Copy link
Author

You should find everything you need @netw0rkf10w here: https://binarymist.io/blog/2018/02/24/hugo-with-staticman-commenting-and-subscriptions/

Leave a staticman comment ;-)

@netw0rkf10w
Copy link

@binarymist Done ;)

@binarymist
Copy link
Author

@gcushen
Copy link
Collaborator

gcushen commented Nov 20, 2018

Closing this as the PR was never rebased on recent Academic and the requested changes were not implemented.

@gcushen gcushen closed this Nov 20, 2018
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.

4 participants