From 25561d17a83a4bd0610164eaa6ef784668764be4 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Tue, 5 Mar 2013 15:37:08 +0100 Subject: [PATCH] API Removed ajax comment submission The JS logic didn't account for edge cases like scrolling to the inserted comment, or attaching the comment in the right sorting logic, on the right pagination page. It also doesn't show any "loading" indication, so is bad usability for the majority of users. A standard form submission does the same job better in this case. Note that this doesn't affect the ability to moderate/delete comments via ajax. Replaced 'use_ajax_commenting' with 'include_js', since its important for behaviour other than comment submission itself, e.g. showing previews (doesn't work without JS) --- code/Commenting.php | 4 +- code/controllers/CommentingController.php | 36 +++++++--------- code/extensions/CommentsExtension.php | 2 +- docs/en/Configuration.md | 14 +++---- javascript/CommentsInterface.js | 51 ++++------------------- 5 files changed, 32 insertions(+), 75 deletions(-) diff --git a/code/Commenting.php b/code/Commenting.php index d7974ad..41f530f 100644 --- a/code/Commenting.php +++ b/code/Commenting.php @@ -25,7 +25,7 @@ class Commenting { private static $default_config = array( 'require_login' => false, // boolean, whether a user needs to login 'required_permission' => false, // required permission to comment (or array of permissions) - 'use_ajax_commenting' => true, // use ajax to post comments. + 'include_js' => true, // Enhance operation by ajax behaviour on moderation links 'use_gravatar' => false, // set to true to show gravatar icons, 'gravatar_size' => 80, // size of gravatar in pixels. This is the same as the standard default 'show_comments_when_disabled' => false, // when comments are disabled should we show older comments (if available) @@ -36,7 +36,7 @@ class Commenting { 'require_moderation' => false, 'html_allowed' => false, // allow for sanitized HTML in comments 'html_allowed_elements' => array('a', 'img', 'i', 'b'), - 'use_preview' => false, // preview formatted comment (when allowing HTML) + 'use_preview' => false, // preview formatted comment (when allowing HTML). Requires include_js=true ); /** diff --git a/code/controllers/CommentingController.php b/code/controllers/CommentingController.php index 6c2cbeb..49f6ba4 100644 --- a/code/controllers/CommentingController.php +++ b/code/controllers/CommentingController.php @@ -251,20 +251,20 @@ class CommentingController extends Controller { $fields = new FieldList( $dataFields = new CompositeField( - TextField::create("Name", _t('CommentInterface.YOURNAME', 'Your name')) - ->setCustomValidationMessage(_t('CommentInterface.YOURNAME_MESSAGE_REQUIRED', 'Please enter your name')) - ->setAttribute('data-message-required', _t('CommentInterface.YOURNAME_MESSAGE_REQUIRED', 'Please enter your name')), + TextField::create("Name", _t('CommentInterface.YOURNAME', 'Your name')) + ->setCustomValidationMessage(_t('CommentInterface.YOURNAME_MESSAGE_REQUIRED', 'Please enter your name')) + ->setAttribute('data-message-required', _t('CommentInterface.YOURNAME_MESSAGE_REQUIRED', 'Please enter your name')), - EmailField::create("Email", _t('CommentingController.EMAILADDRESS', "Your email address (will not be published)")) - ->setCustomValidationMessage(_t('CommentInterface.EMAILADDRESS_MESSAGE_REQUIRED', 'Please enter your email address')) - ->setAttribute('data-message-required', _t('CommentInterface.EMAILADDRESS_MESSAGE_REQUIRED', 'Please enter your email address')) - ->setAttribute('data-message-email', _t('CommentInterface.EMAILADDRESS_MESSAGE_EMAIL', 'Please enter a valid email address')), + EmailField::create("Email", _t('CommentingController.EMAILADDRESS', "Your email address (will not be published)")) + ->setCustomValidationMessage(_t('CommentInterface.EMAILADDRESS_MESSAGE_REQUIRED', 'Please enter your email address')) + ->setAttribute('data-message-required', _t('CommentInterface.EMAILADDRESS_MESSAGE_REQUIRED', 'Please enter your email address')) + ->setAttribute('data-message-email', _t('CommentInterface.EMAILADDRESS_MESSAGE_EMAIL', 'Please enter a valid email address')), - TextField::create("URL", _t('CommentingController.WEBSITEURL', "Your website URL")) - ->setAttribute('data-message-url', _t('CommentInterface.COMMENT_MESSAGE_URL', 'Please enter a valid URL')), + TextField::create("URL", _t('CommentingController.WEBSITEURL', "Your website URL")) + ->setAttribute('data-message-url', _t('CommentInterface.COMMENT_MESSAGE_URL', 'Please enter a valid URL')), - TextareaField::create("Comment", _t('CommentingController.COMMENTS', "Comments")) - ->setCustomValidationMessage(_t('CommentInterface.COMMENT_MESSAGE_REQUIRED', 'Please enter your comment')) + TextareaField::create("Comment", _t('CommentingController.COMMENTS', "Comments")) + ->setCustomValidationMessage(_t('CommentInterface.COMMENT_MESSAGE_REQUIRED', 'Please enter your comment')) ->setAttribute('data-message-required', _t('CommentInterface.COMMENT_MESSAGE_REQUIRED', 'Please enter your comment')) ), HiddenField::create("ParentID"), @@ -416,24 +416,16 @@ class CommentingController extends Controller { } else { $comment->write(); - // extend hook to allow extensions. Also see onBeforePostComment - $this->extend('onAfterPostComment', $comment); + // extend hook to allow extensions. Also see onBeforePostComment + $this->extend('onAfterPostComment', $comment); } // clear the users comment since it passed validation Cookie::set('CommentsForm_Comment', false); - - if(Director::is_ajax()) { - if(!$comment->Moderated && !$isPreview) { - return $comment->renderWith('CommentsInterface_pendingcomment'); - } else { - return $comment->renderWith('CommentsInterface_singlecomment'); - } - } $holder = Commenting::get_config_value($comment->BaseClass, 'comments_holder_id'); - $hash = ($moderated) ? $comment->Permalink() : $holder; + $hash = ($moderated) ? $holder : $comment->Permalink(); $url = (isset($data['ReturnURL'])) ? $data['ReturnURL'] : false; return ($url) ? $this->redirect($url .'#'. $hash) : $this->redirectBack(); diff --git a/code/extensions/CommentsExtension.php b/code/extensions/CommentsExtension.php index 971494d..6395f3d 100644 --- a/code/extensions/CommentsExtension.php +++ b/code/extensions/CommentsExtension.php @@ -97,7 +97,7 @@ class CommentsExtension extends DataExtension { * @see docs/en/Extending */ public function CommentsForm() { - if (Commenting::has_commenting($this->ownerBaseClass) && Commenting::get_config_value($this->ownerBaseClass, 'use_ajax_commenting')) { + if(Commenting::has_commenting($this->ownerBaseClass) && Commenting::get_config_value($this->ownerBaseClass, 'include_js')) { Requirements::javascript(THIRDPARTY_DIR . '/jquery/jquery.js'); Requirements::javascript(THIRDPARTY_DIR . '/jquery-validate/lib/jquery.form.js'); Requirements::javascript(THIRDPARTY_DIR . '/jquery-validate/jquery.validate.pack.js'); diff --git a/docs/en/Configuration.md b/docs/en/Configuration.md index 686130d..a89d1f9 100644 --- a/docs/en/Configuration.md +++ b/docs/en/Configuration.md @@ -7,18 +7,18 @@ The module provides a number of built in configuration settings below are the de // mysite/_config.php Commenting::add('Foo', array( - 'require_login' => false, - 'required_permission' => false, - 'use_ajax_commenting' => true, - 'show_comments_when_disabled' => false, + 'require_login' => false, // boolean, whether a user needs to login + 'required_permission' => false, // required permission to comment (or array of permissions) + 'include_js' => true, // Enhance operation by ajax behaviour on moderation links + 'show_comments_when_disabled' => false, // when comments are disabled should we show older comments (if available) 'order_comments_by' => "\"Created\" DESC", 'comments_per_page' => 10, - 'comments_holder_id' => "comments-holder", - 'comment_permalink_prefix' => "comment-", + 'comments_holder_id' => "comments-holder", // id for the comments holder + 'comment_permalink_prefix' => "comment-", // id prefix for each comment. If needed make this different 'require_moderation' => false, 'html_allowed' => false, // allow for sanitized HTML in comments 'html_allowed_elements' => array('a', 'img', 'i', 'b'), - 'use_preview' => false, // preview formatted comment (when allowing HTML), + 'use_preview' => false, // preview formatted comment (when allowing HTML). Requires include_js=true 'use_gravatar' => false, 'gravatar_size' => 80 ); diff --git a/javascript/CommentsInterface.js b/javascript/CommentsInterface.js index 9536e94..14fcc15 100755 --- a/javascript/CommentsInterface.js +++ b/javascript/CommentsInterface.js @@ -22,16 +22,16 @@ * Validate */ form.validate({ - invalidHandler : function(form, validator) { + invalidHandler : function(form, validator){ $('html, body').animate({ - scrollTop: $(validator.errorList[0].element).offset().top - 30 - }, 200); + scrollTop: $(validator.errorList[0].element).offset().top - 30 + }, 200); }, showErrors: function(errorMap, errorList) { - this.defaultShowErrors(); - // hack to add the extra classes we need to the validation message elements - form.find('span.error').addClass('message required'); - }, + this.defaultShowErrors(); + // hack to add the extra classes we need to the validation message elements + form.find('span.error').addClass('message required'); + }, errorElement: "span", errorClass: "error", @@ -68,44 +68,9 @@ } }); - - /** - * Clicking one of the metalinks performs the operation via ajax - * this inclues the spam and approve links - */ form.submit(function (e) { // trigger validation - if(!form.validate().valid()){ - return false; - } - - previewEl.removeClass('loading').hide(); - - // submit the form - $(this).ajaxSubmit(function(response) { - noCommentsYet.hide(); - - if(!commentsList.length){ - commentsHolder.append(""); - commentsList = $('.comments-list', commentsHolder); - } - - var evenOdd = (commentsList.children('.first').removeClass('first').hasClass('even')) ? 'odd' : 'even'; - var newComment = $('
  • ') - .addClass('comment first ' + evenOdd) - .html(response) - .hide(); - - if(response.match('Spam detected!!')) { - newComment.addClass('spam'); - } - - commentsList.prepend(newComment.fadeIn()); - }); - - $(this).resetForm(); - - return false; + if(!form.validate().valid()) return false; }); /**