5
\$\begingroup\$

I'm working with Codeigniter and I have a jQuery file to add some functionalities. But I think I'm doing it in a wrong way.

$(function() {
    var list = '../indoamericana/intranet/employeesList/';
    var home = '../indoamericana/administrar/callHome/';
    var profile = '../indoamericana/user/';
    var addUser = '../indoamericana/user/addUSer';

    $(document).on('click', '#documentacion', function(e) { 
        var head = $('iframe').contents().find('head');
        head.append($('<link/>', { 
            rel: 'stylesheet', 
            href: '/indoamericana/webroot/css/style.css', 
            type: 'text/css' }));
        head.append($('<link/>', { rel: 'stylesheet', 
            href: '/indoamericana/webroot/css/custom.css', 
            type: 'text/css' }));
        e.preventDefault();
    });

    $(document).on('click', 'a[data-target=#ajax]', function(ev){
        var target = $(this).attr("href");
        $("#ajax").load(target, function() { 
            $("#ajax").modal("show"); 
        });
        ev.preventDefault();
    });

    var loadPage = function(page){
        $(".page-content").load(page);  
    };

    $(".sub-menu .menu-item, .module-item").click(function(event) {
        $(".page-content").load($(this).data('target'));  
        $("title").text( $( this ).text() );
    });

    $( document ).on( 'click', '.page-sidebar-menu li', function(e) { 
        $( this ).addClass('active');
        $( this ).siblings().removeClass('active');
    });

    var last_clicked = '';
    var order = 'asc';

    $(document).on('click', '#admin-support a i', function(e){
        var index = $( this ).attr('data-sort'); 
        if( last_clicked == index )
            order = ( order == 'asc' ) ? order = 'desc' : order = 'asc';
        last_clicked = index;
        var page  = '../indoamericana/soporte/allSupports/' + index + '/' + order; 
        $(".page-content").load(page);
    });
});

I'm calling this file (call-functionalities.js) in a script type of my 'head':

<script src="webroot/scripts/call-functionalities.js" type="text/javascript"></script>

I don't want to use anonymous functions. I have seen some pages that use

jQuery(document).ready(function() {     
  App.init();
  Index.init();
});
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Why do you think it's the wrong way? It looks like a fairly ok jquery code to me. I'd prefer to use a more specific selector rather than $(document) for attaching the event handlers, but that's really a matter of style, I guess. The only thing that definitely can be improved is: order = ( order == 'asc' ) ? order = 'desc' : order = 'asc'; is better rewritten as order = ( order == 'asc' ) ? 'desc' : 'asc'; \$\endgroup\$
    – Lie Ryan
    Jan 28, 2014 at 20:14

1 Answer 1

2
\$\begingroup\$
  1. Apart from the fact I can't find any use of var loadPage It doesn't need to be an expression. Instead try:

    function loadPage(page) {
    }
    

    It is still accessible only within your code here, it is cleaner and easier to debug.

  2. Keep your click events consistent

    $(".sub-menu .menu-item, .module-item").click(function(event) {
        $(".page-content").load($(this).data('target'));  
        $("title").text( $( this ).text() );
    });
    

    Is the only place you're using .click( I'd stick with the $(parent).on('click', '.selector', function(){ style.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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