Hello and welcome to our community! Is this your first visit?
Register
Enjoy an ad free experience by logging in. Not a member yet? Register.
Results 1 to 5 of 5
Like Tree2Likes
  • 1 Post By glenngv
  • 1 Post By glenngv

Thread: Can This Be Optimised?

  1. #1
    New Coder
    Join Date
    Mar 2014
    Posts
    18
    Thanks
    0
    Thanked 0 Times in 0 Posts

    Post Can This Be Optimised?

    Finally got this code working perfectly, just wanted to check if there were any ways to optimise the script before setting it live.

    Code:
    $(document).on("click", '.load', function(event) { 
        var $projects = $("#projects"),
            id = $(this).data('id');
        $projects.html("").data('id', id);
        $("#loading").show();
        $projects.load("projects.html #div" + id, function () {
            $("#projects").show();
            $("#loading").hide();
        })
    });
    
    $(document).on("click", '.next', function(event) { 
        var $projects = $("#projects");
        pid = $projects.data('id');
        $projects.data('id', (pid == 6) ? 1 : (pid + 1));
        $projects.load("projects.html #div" + $projects.data('id'));
    });
    
    
    $(document).on("click", '.previous', function(event) { 
        var $projects = $("#projects");
        pid = $projects.data('id');
        $projects.data('id', (pid == 1) ? 6 : (pid - 1)); 
        $projects.load("projects.html #div" + $projects.data('id'));
    });
    
    $(document).on("click", '.exit', function(event) { 
        $("#projects").hide();
    });
    Anyone see any ways to improve / optimise?

  • #2
    Supreme Master coder! glenngv's Avatar
    Join Date
    Jun 2002
    Location
    Philippines
    Posts
    11,043
    Thanks
    0
    Thanked 251 Times in 247 Posts
    Without seeing the html, here's how I would improve it:
    Code:
    $(document)
        .on("click", '.load', function(event) { 
            var $projects,
                id = $(this).data('id');
    
            $("#loading").show();
    
            $projects = $("#projects")
                .hide()
                .html("")
                .data('id', id)
                .load("projects.html #div" + id, function () {
                    $projects.show();
                    $("#loading").hide();
                });
        })
        .on("click", '.next', function(event) { 
            var $projects = $("#projects");
    
            pid = $projects.data('id');
    
            $projects
                .data('id', (pid == 6) ? 1 : (pid + 1))
                .load("projects.html #div" + $projects.data('id'));
        })
        .on("click", '.previous', function(event) { 
            var $projects = $("#projects");
    
            pid = $projects.data('id');
    
            $projects
                .data('id', (pid == 1) ? 6 : (pid - 1)); 
                .load("projects.html #div" + $projects.data('id'));
        })
        .on("click", '.exit', function(event) { 
            $("#projects").hide();
        });
    But I recommend to change $(document) to the closest parent or ancestor (that exists on the page at the time of the call) of the delegated elements .load, .next, .previous and .exit. If they have different parents/ancestors, change each selector accordingly. If they are all direct children of the body tag, then there's nothing to change.

  • #3
    New Coder
    Join Date
    Mar 2014
    Posts
    18
    Thanks
    0
    Thanked 0 Times in 0 Posts
    Cheers, Glen, looks a lot cleaner! There was a slight error in the previous function, but managed to fix that and it's working great!

    One quick issue regarding what you said about the $document. Here's a quick look at the HTML.

    Code:
    <html>
    <main>
      <div class="container">
        <div id="projects">
          <!--All the things to do with that function are loaded into here-->
        </div>
      </div>
    </main>
    </html>
    Do you mean use something like
    Code:
    $(document.getElementById("projects"))
    or something?

  • #4
    Supreme Master coder! glenngv's Avatar
    Join Date
    Jun 2002
    Location
    Philippines
    Posts
    11,043
    Thanks
    0
    Thanked 251 Times in 247 Posts
    If the elements .load, .next, .previous, and .exit are all descendants of #projects, then use #projects and then you can reference it inside the event handler as event.delegateTarget.

    Code:
    $('#projects')
        .on("click", '.load', function(event) { 
            var $projects,
                id = $(this).data('id');
    
            $("#loading").show();
    
            $projects = $(event.delegateTarget)
                .hide()
                .html("")
                .data('id', id)
                .load("projects.html #div" + id, function () {
                    $projects.show();
                    $("#loading").hide();
                });
        })
        .on("click", '.next', function(event) { 
            var $projects = $(event.delegateTarget);
    
            pid = $projects.data('id');
    
            $projects
                .data('id', (pid == 6) ? 1 : (pid + 1))
                .load("projects.html #div" + $projects.data('id'));
        })
        .on("click", '.previous', function(event) { 
            var $projects = $(event.delegateTarget);
    
            pid = $projects.data('id');
    
            $projects
                .data('id', (pid == 1) ? 6 : (pid - 1)); 
                .load("projects.html #div" + $projects.data('id'));
        })
        .on("click", '.exit', function(event) { 
            $(event.delegateTarget).hide();
        });

  • #5
    New Coder
    Join Date
    Mar 2014
    Posts
    18
    Thanks
    0
    Thanked 0 Times in 0 Posts
    I actually tried that and it broke the script, only reason I can think of is that they're loaded in via AJAX and in the external file they're all direct children of the body so I think the first script should be fine.

    Cheers Glen!


  •  

    Posting Permissions

    • You may not post new threads
    • You may not post replies
    • You may not post attachments
    • You may not edit your posts
    •