1
\$\begingroup\$

I'm working on a custom CMS and I'm primarily a PHP/database guy. This is my first real project using jQuery in any complexity. I've got it all functioning, but needless to say, it's pretty messy and I know jQuery veterans would frown at the code.

What can I do to clean up my jQuery? What bad practices am I employing? I'm not looking for someone to rewrite it all for me, just want some tips that I can use for the future as well.

I use Twitter Bootstrap, so most of the functions are a part of that. wysihtml5 is a WYSIWYG editor, tagit is for post tags (think Wordpress).

One thing I'm wondering about is my .ajax stuff, such as if I'm handling responses correctly.

$(document).ready(function() {

    // add buttons to content textarea
    $('#item_content').wysihtml5({
        "html": true
    });

    $("#image_library").load('/admin/file/library', { 'csrf_atk' : $("input[name=csrf_atk]").val(), 'images' : $('#post_images').val() });

    $("#post_tags").tagit({
        allowSpaces: true,
    });

    $('#post_date').datepicker();

    $('.container').hide();

    var post_type = '<?php echo $type; ?>';
    $('#'+post_type+'_container').show();

    $('.post_types button').click(function(){
        var target = "#" + $(this).data("target");
        $(".container").not(target).hide();
        $(target).show();
        $('#post_type').val($(this).text());
    });

        $('.post_status button').click(function(){
        $('#post_status').val($(this).text());
    });

        // on delete button click, remove image from site folder, database, queue, and from hidden field
        $('#image_container').on('click', '.image-delete', function() {

            var $this = $(this);

            $.ajax({
                url: '<?=base_url();?>admin/file/delete_image_relationship',
                type: 'POST',
                data: { 
                    csrf_atk: $("input[name=csrf_atk]").val(),
                    item_id: $("input[name=item_id]").val(),
                    image_id: $(this).val(),
                },
                success: function(response){

                    console.log(response);

                    $this.parent().parent().remove();

                    // get image_id in delete button
                    var id = $this.val();

                    //find id in hidden field and remove it
                   $("#post_images").val(function(i, v) {
                    return v.replace( new RegExp('(?=(?:^|,))(,?)' + id + '(?=(?:,|$)),?'), '$1' );
                    });

                },
            });

        });


        // make save button display "Saving..."
        $('#save').button();

        // on save click, serialize all form elements and insert/update database
        $('#save').click(function() {

            $('#save').button('loading');

            // get url to post data to
            var url = $('#item_form').attr('action');

            // save form data to serialized array
            var data = $('#item_form').serializeArray();

            // get post tags
            var tags = $('#post_tags').tagit('assignedTags');

            // add tags to serialized array
            data.push({ name: "tags", value: tags});

            $.ajax({
                url: url,
                type: 'POST',
                data: data,
                success: function(data){
                    console.log(data);

                    json = $.parseJSON(data);

                    if(json.status == 'success') {

                        console.log(json);

                    } else {

                        $('#response_msg').html(json.message).addClass('alert alert-error');
                    }

                },
                error: function(jqXHR, textStatus, errorThrown) {
                    if (errorThrown == 'Forbidden') {
                        alert("Session has expired, login again to save");
                    } else {
                        alert(errorThrown);
                    }

                },
                complete: function(){
                    setTimeout(function () {
                      $('#save').button('reset')
                    }, 1000)
                }
            });

        });

        // on button click, set image type in database
        $('#image_container').on('click', '.image-type button', function() {

            $this = $(this);

            $.ajax({
                url: '<?=base_url();?>admin/file/set_image_type',
                type: 'POST',
                data: { 
                    csrf_atk: $("input[name=csrf_atk]").val(),
                    type: $this.text(),
                    image_id: $this.val(),
                },
                error: function(jqXHR, textStatus, errorThrown) {
                        alert(errorThrown);
                }
            });

        });

            $('.fileUpload').fileUploader({
                autoUpload: true,
                allowedExtension: 'jpg|jpeg|gif|png',
                afterEachUpload: function(data, status, formContainer) {

                // add image_id to hidden field
                $('#post_images').val(function(i,val) { 
                    return val + (val ? ',' : '') + json.image_id;
                });

                $("#image_library").load('/admin/file/library', { 'csrf_atk' : $("input[name=csrf_atk]").val(), 'images' : $('#post_images').val() });


                }

            });

});
\$\endgroup\$
2
\$\begingroup\$

You'll want to think in terms of javascript rather than jQuery when you develop. jQuery is just a toolkit, not a framework. You may want to review some JS design patterns (Google, Essential JS Design Patterns) and make sure to check out the MV* chapter.

Moving forward with your code, improve performance by not calling the jQuery object on a element via a selector more than once. In other words store $('#save') into a var like $save. Learn how to utilize $(this). Use chaining more. This will also prevent having to change 5 lines of code when you change an id.

Organize your code by consolidating into closed modules -or- objects, at the least, and bring those variables like post_type into scope.

Taking the next step would be organizing your objects into views, models, and event managers. You may want to check out Backbone and Underscore for more efficient event handling, data management, templating, and utilities.

\$\endgroup\$

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.