Skip to content
This repository has been archived by the owner on Nov 30, 2024. It is now read-only.

Comments form intense debate #187

Merged
merged 2 commits into from
May 25, 2015

Conversation

pierre-jean
Copy link
Contributor

By default, the comment provider of JB for intensedebate display a links in the form "comments (0)".
The link itself has no effect by default, so it's kind of confusing for beginners who just set up the comments provider on the config and are hoping for a comments section.
With this commit, the comment form is displayed at the bottom of the post.

Intense Debate comments provider:
Setting the default behavior to had the comments form below the post instead of adding a link with the comments count
@marshallshen
Copy link
Collaborator

@pierre-jean: thanks for submitting the pull request! Agree with the confusing display of the link, please see my comments. Thanks.

@@ -3,4 +3,5 @@ var idcomments_acct = '{{ site.JB.comments.intensedebate.account }}';
var idcomments_post_id;
var idcomments_post_url;
</script>
<script type="text/javascript" src="http://www.intensedebate.com/js/genericLinkWrapperV2.js"></script>
<span id="IDCommentsPostTitle" style="display:none"></span>
Copy link
Collaborator

Choose a reason for hiding this comment

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

inline CSS is not preferred. Move it into stylesheet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I think I can totally remove the span "IDCommentsPostTitle", and just leave the script.
I added it because it was the code suggested by intensedebate website, but removing it has no effect, the comments section is still at the same place, and the title isn't displayed in both cases.
Do you agree?

@pierre-jean
Copy link
Contributor Author

@marshallshen Thanks for your comment, I preferred removing the span than adding some CSS for a element unvisible by default. Please see comment on the diff.

@groundh0g groundh0g added this to the v 0.5.0 milestone Apr 4, 2015
@groundh0g groundh0g self-assigned this Apr 4, 2015
@groundh0g
Copy link
Collaborator

Looks like a low-risk edit. @marshallshen's concern was addressed by @pierre-jean. Merging.

groundh0g added a commit that referenced this pull request May 25, 2015
@groundh0g groundh0g merged commit 5f3f155 into plusjade:master May 25, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants