2
\$\begingroup\$

I have from a recommendation on my other question here:

My other review post

Developed a truncate plugin, and was wondering if you guys could help me improve it or see where I have gone wrong.

Plugin code:

$.fn.vishTrunc = function(options) {

    var settings = $.extend({
            size: 100,
            omission: '...',
        }, options);

    return this.each(function() {

        var elements = $(this),
            regex    = /\s+/,
            init     = function() {
                elements.each(function() {
                var textDefault = $(this).text();

                    if (textDefault.length > settings.size) {
                        var words = textDefault.trim().split(regex).slice(0,settings.size);
var replacement = $("<p class='replacement'/>").text(words.join(" ") + settings.omission);

        $(this).parent().find('.read-more').click(function(){
   if($(this).text() === 'Read More'){
    replacement.hide();
    $(this).parent().find('p:eq(1)').show();    
   } else if($(this).text() === 'Read Less'){
       replacement.show();
    $(this).parent().find('p:eq(1)').hide();
   }
    $(this).text(function(i,old){
    return old==='Read More' ?  'Read Less' : 'Read More';
});
return false
});


$(this).before(replacement);
$(this).hide();

                    }
                });
            };
        init();
    });
};
\$\endgroup\$
1
\$\begingroup\$

all of your indentation makes it very difficult to read the code, this is what your code should look like. you were also missing a Semi-Colon after return false

$.fn.vishTrunc = function(options) {
    var settings = $.extend({
            size: 100,
            omission: '...',
        }, options);

    return this.each(function() {
        var elements = $(this),
            regex    = /\s+/,
            init     = function() {
                elements.each(function() {
                    var textDefault = $(this).text();
                    if (textDefault.length > settings.size) {
                        var words = textDefault.trim().split(regex).slice(0,settings.size);
                        var replacement = $("<p class='replacement'/>").text(words.join(" ") + settings.omission);

                        $(this).parent().find('.read-more').click(function(){   
                            if($(this).text() === 'Read More'){
                                replacement.hide();
                                $(this).parent().find('p:eq(1)').show();    
                            } else if($(this).text() === 'Read Less'){
                                replacement.show();
                                $(this).parent().find('p:eq(1)').hide();
                            }

                            $(this).text(function(i,old){
                                return old==='Read More' ?  'Read Less' : 'Read More';
                            });

                            return false;
                        });

                        $(this).before(replacement);
                        $(this).hide();

                    }
                });
            };
        init();
    });
};

Personally, I think this is a lot of indentation and a lot of nested functions.

I think that you should break out the functions, this would reduce the amount of nesting and help document what each function is doing by giving them names.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Indentation messed up in pasting sorry \$\endgroup\$ – Vaishal Patel Jan 27 '17 at 14:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.