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

Fix for displaying multiple comment forms on the same page #1945

Closed
wants to merge 1 commit into from
Closed

Fix for displaying multiple comment forms on the same page #1945

wants to merge 1 commit into from

Conversation

nesjett
Copy link
Contributor

@nesjett nesjett commented Oct 19, 2016

Because the use of IDs in order of Classes as identifiers, the comment
form was not able to work properly.
NOTE: Next and Prev buttons doesnt work, but I think its not related to
this issue. Please, confirm that they are not working on the e107
previous this commit.

Because the use of IDs in order of Classes as identifiers, the comment
form was not able to work properly.
NOTE: Next and Prev buttons doesnt work, but I think its not related to
this issue. Please, confirm that they are not working on the e107
previous this commit.
@nesjett
Copy link
Contributor Author

nesjett commented Oct 19, 2016

Fix related to #1944

@nesjett nesjett changed the title Fix for displaying multiple comment forms Fix for displaying multiple comment forms on the same page Oct 19, 2016
@@ -80,12 +80,15 @@ $(document).ready(function()

$(document).on("click", ".e-comment-submit", function(){

var commentsParent = $(this).parent().parent().parent().parent().parent().parent().parent();
Copy link
Member

Choose a reason for hiding this comment

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

This won't work if you use custom templates for comments, because the HTML structure may be not the same. Use parents() instead. For example if you want to get the form, which contains your element, use something like this:

var $form = $(this).parents('form:first');

@CaMer0n
Copy link
Member

CaMer0n commented Oct 21, 2016

Closing this as the proposed changes would break as per the comments mentioned above.

@CaMer0n CaMer0n closed this Oct 21, 2016
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