-
-
Notifications
You must be signed in to change notification settings - Fork 262
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
[Optional feature]: Staticman Nested Comment Support #119
Conversation
CSS to be added
Missed "container"
i18n doesn't work under assets. I've adopted Minimal Mistakes' approach to get this work.
I've done d818b0c against the wrong branch!
1. Forgotten the last "fieldset" originally present in Bruno Adele's code for Beautiful Hugo's native Staticman support 2. Clear form after successful submission 3. Shorter jQuery code to clear all form (and hidden) input fields
Look back at what I've written, I believe that a self-review would help others.
I would appreciate any thoughts and code changes. |
@VincentTam First of all: Thank you very much for the time and effort you put into this! The code you wrote it very elegant and fits well into hugo. But I'm not sure if this is something I want to add to this theme. The theme is about setting up a simple and minimalistic personal site. Blogging is just a site feature of this theme. I assume most people just want to set up there site and don't care about comments. @VincentTam Thoughts on this? Out of interests: Why did you put work into this theme and not any other? @vickylai Would love to hear your thoughts on this! |
Thank you for your reply despite my unfinished work. Since I'm not a developer, I publish it here for interested users. This PR aims at providing comment support alternative to existing Disqus support. For Disqus vs Staticman, you may google for the rationale of preferring the later. (I'm replying on mobile.) Whether comment is important is subjective. A theme designer may (or may not) offer the freedom for its users to enable this feature. With Staticman's config parameters missing or blank (default), such features will be disabled. Looking back, my code isn't the best. I've messed up the code for the reply target in each comment reply. Please see the last point for details. I did integrate Staticman into other templates. You may consult
In fact, this is a derivative of halogenica/beautifulhugo#222, which aims to fix the logic problem mentioned in dancwilliams/networkhobo#373. IMHO, interactions with readers are beneficial to site owners, who often get great ideas from readers. Moreover, having static comments (with Gravatar of other users) is great in terms of SEO because search engines view this as the activities of the page. |
I’d leave this one to the community to decide, if anyone wishes to weigh in. Introduction is meant to be a minimalist personal site, but I’m not opposed to the addition of comment functionality, as long as it is clearly optional. Especially with Staticman over Disqus. I would like to be able to take a look at the styling within the next couple weeks. If anyone else wants to give it a go in the meantime, please do! |
I've recently revised the template code in another Hugo theme Huginn. The reasons are two-folded.
Since the Go template & JS code used for both themes were essentially the same at the beginning, an overall comparison |
Hello @VincentTam! I made some updates to your work but am unable to push to this PR. If you'd like to keep it going, would you kindly follow these instructions to grant me permission to make edits? If you don't want to do that, you can still look over my changes in the new branch and see if there's anything you'd like to implement on your own. |
@victoriadotdev Thanks for merging At the bottom right corner of the screenshot, I can see that the relevant checkbox is checked. I'm going to contact GitHub support for that. |
@victoriadotdev I've just got a reply from GitHub, who suggested me to uncheck and check the checkbox. I've done that, and please verify whether the write permission problem has been solved. |
1. Changed to light theme, which is the default 2. Fixed indentation of Staticman param 3. Make Staticman optional
1. Improved UI 2. Fixed error at the top of staticman JS 3. Fixed error in allowedFields due to copying from Beautifhul Hugo and Miminal Mistakes
@victoriadrake Another update: Despite further discussions with GitHub Support staff, she couldn't find out the reason why you were unable to push to this PR. Since this PR's submission, the write permissions from maintainers have always been enabled. I tried asking if your recent change of user name had contributed to this error, by she was unable to verify this theory after some tests. It seems that this push failure is due to past conflicts of translation files, in which your user name appeared at the bottom.
As a result, each translation file is modified on both sides at the end of the file, and this causes a merge conflict. As can be seen from the commit tree of the HEAD of this PR, I've merged your changes this week, so the conflicts have been resolved. Hope this solves your difficulties in committing against the head of this PR. A self-review on the recent changes of this PR
As before, please feel free to change anything and/or leave a comment/question(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Practice GitHub review.
$('.page__comments .comment').on('click', '.reply-btn', function (evt){ | ||
resetReplyTarget(); | ||
var cmt = $(evt.delegateTarget); | ||
$('.page__comments input[name="fields[replyThread]"]').val(this.title); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you find my approach of storing the _id
of a comment's eldest ancestor in the title
attribute of an <a>
tag not satisfying, I can change the this.title
to replyThread
, where var replyThread = cmt.find('.someClassName')
.
1. Add Staticman bot/app, depending on your Git service provider. | ||
- GitHub: choose either one of the following method | ||
+ GitHub App: refer to issue | ||
[https://github.com/eduardoboucas/staticman/issues/243](https://github.com/eduardoboucas/staticman/issues/243) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@staticmanapp and https://api.staticman.net/v2
no logner work, as reported in eduardoboucas/staticman#306.
Then help **[@staticmanlab](https://github.com/staticmanlab)** | ||
accept the invitation by going to | ||
|
||
https://staticman3.herokuapp.com/v3/connect/github/<username>/<repo-name> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Staticman's documentation only covers v2, so there's a need to give a self-contained setup guide.
|
||
$.ajax({ | ||
type: $(this).attr('method'), | ||
url: [endpoint, 'v3/entry', gitProvider, repo, branch, 'comments'].join('/'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separated POST request URL in JS code to prevent abuses by spam bots through scraping the generated HTML source code.
/* Static comments */ | ||
$avatar__size: 5rem | ||
$avatar__sep: 3rem | ||
$reply__indent: 2rem |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to change the values here. Since I'm not a web designer, I haven't considered the mobile loading issue.
Error made while incorporating code from two different themes: 1. Beautifhul Hugo 2. Minimal Mistakes
In the same spirit of halogenica/beautifulhugo#270.
It looks like this PR has been idle a while, so I am marking it as stale. Remove the label or comment if this is still being worked on. |
It looks like this PR has been idle a while, so I am marking it as stale. Remove the label or comment if this is still being worked on. |
Motivation
Provide a free commenting service.
Hugo's built-in Disqus has no Markdown support. Staticman is open-source and transmits HTML form data as YML files to be merged in GitHub/GitLab repos. Despite its MIT license, the project had been bundled with GitHub for years. Thanks to Nicholas Tsim's PR, Staticman now supports GitLab.
To add Staticman, I've combined Minimal Mistakes and Beautiful Hugo's code so as to avoid breaking the original modals. I am not good at CSS at the moment, so the visual layout will be strange. It took me hours to understand the role of
container
. During local preview, the SASS comes out. However, when it's deployed to GitLab, this has disappeared. However, the logic is already there at https://vincenttam.gitlab.io/introduction. Web developers should be able to fix the display much more efficiently than I do. I'm creating this PR to publish my code.Features
exampleSite
Quick guide to test this PR
I assume the user has installed his/her Hugo theme(s) as a Git submodule.
On the recent Staticman's public API bottleneck
Due to eduardoboucas/staticman#227, the parameter
api
inconfig.toml
defaults to the API address associated with @staticmanlab.