#1
  1. No Profile Picture
    Super Moderator
    Devshed Intermediate (1500 - 1999 posts)

    Join Date
    Jun 2009
    Location
    Hartford, WI
    Posts
    1,509
    Rep Power
    111

    Better way to group this?


    I relatively have the same 5 things defined for each page. Instead of completely defining each of the following as below, is there a suggested way to overall "group" the actions within a single .balance class as a wrapper or such?
    Code:
    // Add new balance
    $('.balance').on('click', '.add', function() {
    });
    
    // Edit this balance
    $('.balance').on('click', '.edit', function() {
    });
    
    // Delete this balance
    $('.balance').on('click', '.delete', function() {
    });
    
    // Save balance changes
    $('.balance').on('click', '.save', function() {
    });
    
    // Cancel balance changes
    $('.balance').on('click', '.cancel', function() {
    });
    He who knows not that he knows not is a fool, ignore him. He who knows that he knows not is ignorant, teach him. He who knows not that he knows is asleep, awaken him. He who knows that he knows is a leader, follow him.
  2. #2
  3. Wiser? Not exactly.
    Devshed God 2nd Plane (6000 - 6499 posts)

    Join Date
    May 2001
    Location
    Bonita Springs, FL
    Posts
    6,126
    Rep Power
    4103
    What kind of grouping are you after exactly? It's not clear to me what your asking for.

    You could tie into just .balance and check for the classes yourself, but I don't really see much point in that.
    Code:
    $('.balance').click(function(e){
        //Assumes target class is on an element with no children
        var $target = $(e.target);
        if ($target.is('.add')){ ... }
        else if ($target.is('.edit')){ ... }
        ...
    });

    You could put the code in to a jquery extension
    Code:
    jQuery.fn.balance = function(){
        return this.each(function(){
            var $this = $(this);
        $this.on('click', '.add', function(){ ... });
        $this.on('click', '.edit', function(){ ... });
        ...
        };
    }
    Then on balance pages just call that extension.
    Code:
    $('.balance').balance();
    Recycle your old CD's



    If I helped you out, show some love with some reputation, or tip with Bitcoins to 1N645HfYf63UbcvxajLKiSKpYHAq2Zxud
  4. #3
  5. No Profile Picture
    Super Moderator
    Devshed Intermediate (1500 - 1999 posts)

    Join Date
    Jun 2009
    Location
    Hartford, WI
    Posts
    1,509
    Rep Power
    111
    It's more that I've this same list for any page with such function, so it creates a long list. I don't exactly mind it too much, but your first 2 examples seem to kinda be what I'm aiming for. Since the initial class which really defines the actual page is just repeated every time for the set of actions, I figured there was a basic way of making a single item to hold the actions.

    Extended example:
    Code:
    // Add new account
    $('.account').on('click', '.add', function() {
    });
    
    // Edit this account
    $('.account').on('click', '.edit', function() {
    });
    
    // Delete this account
    $('.account').on('click', '.delete', function() {
    });
    
    // Save account changes
    $('.account').on('click', '.save', function() {
    });
    
    // Cancel account changes
    $('.account').on('click', '.cancel', function() {
    });
    
    // Add new balance
    $('.balance').on('click', '.add', function() {
    });
    
    // Edit this balance
    $('.balance').on('click', '.edit', function() {
    });
    
    // Delete this balance
    $('.balance').on('click', '.delete', function() {
    });
    
    // Save balance changes
    $('.balance').on('click', '.save', function() {
    });
    
    // Cancel balance changes
    $('.balance').on('click', '.cancel', function() {
    });
    
    // Add new invoice
    $('.invoice').on('click', '.add', function() {
    });
    
    // Edit this invoice
    $('.invoice').on('click', '.edit', function() {
    });
    
    // Delete this invoice
    $('.invoice').on('click', '.delete', function() {
    });
    
    // Save invoice changes
    $('.invoice').on('click', '.save', function() {
    });
    
    // Cancel invoice changes
    $('.invoice').on('click', '.cancel', function() {
    });
    Overall, there are only 3 initial classes/pages in this example. (.account, .balance, .invoice) They all offer 5 functions. I would aim to build a single structure if the process for each action was the same, but sadly it is not.

    This is a row in the page's edit view, but the row is in read mode. (Until someone clicks the Edit image at its end)
    Code:
    <FORM class="tr">
      <SPAN class="td">X</SPAN>
      <SPAN class="td">06</SPAN>
      <SPAN class="td">$205.29</SPAN>
      <SPAN class="td">Chase Auto</SPAN>
      <SPAN class="td"></SPAN>
      <SPAN class="td">Scheduled via 'Automated' (THRU March)</SPAN>
      <SPAN class="action td">
        <SPAN class="row-info">
          <INPUT name="year" type="hidden" value="2017" />
          <INPUT name="month" type="hidden" value="12" />
          <INPUT name="inv_id" type="hidden" value="229" />
        </SPAN>
        <SPAN class="view-icons"><IMG class="edit" src="./img/edit.png" /> &nbsp; <IMG class="delete" src="./img/delete.png" /></SPAN>
        <SPAN class="edit-icons"><IMG class="save" src="./img/save.png" /> &nbsp; <IMG class="cancel" src="./img/cancel.png" /></SPAN>
      </SPAN>
    </FORM>
    The Add is an independent form at the top of the page. The 2 set of icons at the row's end swap back and forth based on if the row itself is in read or edit mode.

    Edit: My current functions.js file. http://projects.hellzoneinc.com/devs...his-979581.txt
    Last edited by Triple_Nothing; December 6th, 2017 at 08:45 AM.
    He who knows not that he knows not is a fool, ignore him. He who knows that he knows not is ignorant, teach him. He who knows not that he knows is asleep, awaken him. He who knows that he knows is a leader, follow him.
  6. #4
  7. Wiser? Not exactly.
    Devshed God 2nd Plane (6000 - 6499 posts)

    Join Date
    May 2001
    Location
    Bonita Springs, FL
    Posts
    6,126
    Rep Power
    4103
    If your handler functions do different things for each case, then there's not much you can do to combine them. What I showed in my first case doesn't really add any benefit as you'd just be moving your current code from different .on() calls to a single .on call with a massive if/else if branch which I'd argue is worse code.

    What you need to do if you want to reduce/reuse your code is try to generalize the handler functions so they can apply to different cases. For example, your .add handlers seem to be mostly the same, so what you could is combine them into a function with some parameters for the parts that vary
    Code:
    // Add new invoice
    $('.invoice').on('click', '.add', submitAction('invoice', 'add'));
    $('.account').on('click', '.add', submitAction('account', 'add'));
    
    function submitAction(page, action){
        return function(){
            $(this).actionCall(page, action, $(this).closest('form').serialize());
        };
    }
    Your edit functions are similar, but different enough that I'm not sure if there is much to be gained from trying to generalize them. If you could alter the application structure to reduce the number of if statements maybe something could be done. That's something you'd have to decide whether or not it's worth doing.

    Given your current code, this is the best I could come up with:

    Code:
    $('.invoice').on('click', '.edit', editHandler(['paid', 'due', 'value', 'name', 'adj_value', 'comment'], function(name) {
        if(name === 'paid') {
            var orig = ($(this).text() === 'X') ? 'checked' : null;
            $(this).html($('<input />', { name: name, type: 'checkbox', checked: orig }));
        } else if(name === 'value' || name === 'adj_value') {
            var edit = $(this).text().replace('$', '');
            $(this).html($('<input />', { class: 'currency', name: name, value: edit }));
        } else if(name === 'name') {
            var v = $(this).text();
            $(this).actionCall('invoice', 'select', v); // Remove the hidden input from the actionCall success, it's handled below.
        } else {
            var orig = $(this).text();
            $(this).html($('<input />', { name: name, value: orig }));
        }
    }));
    
    $('.account').on('click', '.edit', editHandler(['active', 'name'], function(name){
        if(name === 'active') {
            var orig = ($(this).text() === 'Active') ? 'checked' : null;
            $(this).html($('<input />', { name: name, type: 'checkbox', checked: orig }));
        } else {
            var orig = $(this).text();
            $(this).html($('<input />', { name: name, value: orig }));
        }
    }));
    
    function editHandler(nameList, transformFn){
        return function(){
            var i=0;
            $(this).closest('span.td').siblings().each(function(){
                var originalText = $(this).text();
                transformFn.call(this, nameList[i++]);
                $(this).append($('<input />', { type: 'hidden', value: originalText }));
            });
            $(this).closest('span.td').toggleClass('editing');
        };
    }

    I tend to create named functions for my handlers rather than massive anonymous functions also. I find it keeps the code looking cleaner and lets me keep all the event attaching code nicely groupped together, so I'd probably refactor the above edit stuff further into something like:
    Code:
    $('.invoice').on('click', '.edit', invoiceTransform);
    $('.account').on('click', '.edit', accountTransform);
    
    function editHandler(nameList, transformFn){
        var i=0;
        $(this).closest('span.td').siblings().each(function(){
            var originalText = $(this).text();
            transformFn.call(this, nameList[i++]);
            $(this).append($('<input />', { type: 'hidden', value: originalText }));
        });
        $(this).closest('span.td').toggleClass('editing');
    }
    
    function invoiceEditHandler() {
        var nameList = ['paid', 'due', 'value', 'name', 'adj_value', 'comment'];
        editHandler.call(this, nameList, function(name){
            if(name === 'paid') {
                var orig = ($(this).text() === 'X') ? 'checked' : null;
                $(this).html($('<input />', { name: name, type: 'checkbox', checked: orig }));
            } else if(name === 'value' || name === 'adj_value') {
                var edit = $(this).text().replace('$', '');
                $(this).html($('<input />', { class: 'currency', name: name, value: edit }));
            } else if(name === 'name') {
                var v = $(this).text();
                $(this).actionCall('invoice', 'select', v); // Remove the hidden input from the actionCall success, it's handled below.
            } else {
                var orig = $(this).text();
                $(this).html($('<input />', { name: name, value: orig }));
            }
        });
    }
    
    function accountTransform(){
        var nameList = ['active', 'name'];
        editHandler.call(this, nameList, function(name){
            if(name === 'active') {
                var orig = ($(this).text() === 'Active') ? 'checked' : null;
                $(this).html($('<input />', { name: name, type: 'checkbox', checked: orig }));
            } else {
                var orig = $(this).text();
                $(this).html($('<input />', { name: name, value: orig }));
            }   
        });
    }
    Your cancel function appears to be generic and the same for both invoices and accounts (and presumably the others) so you could separate that to a named function.
    Code:
    $('.invoice').on('click', '.cancel', cancelEditing);
    $('.account').on('click', '.cancel', cancelEditing);
    
    function cancelEditing(){
        $(this).closest('span.td').siblings().each(function(index, cell) {
            $(this).text($(this).find('input[type=hidden]').val());
        });
        $(this).closest('span.td').toggleClass('editing')
    }
    Your save code could probably be re-done in a style similar to the edit code above. Possibly you could combine your edit/save transformations into a single function.
    Recycle your old CD's



    If I helped you out, show some love with some reputation, or tip with Bitcoins to 1N645HfYf63UbcvxajLKiSKpYHAq2Zxud
  8. #5
  9. No Profile Picture
    Super Moderator
    Devshed Intermediate (1500 - 1999 posts)

    Join Date
    Jun 2009
    Location
    Hartford, WI
    Posts
    1,509
    Rep Power
    111
    Aight. That gave me a few ideas. Thanks for the input.
    He who knows not that he knows not is a fool, ignore him. He who knows that he knows not is ignorant, teach him. He who knows not that he knows is asleep, awaken him. He who knows that he knows is a leader, follow him.
  10. #6
  11. No Profile Picture
    Super Moderator
    Devshed Intermediate (1500 - 1999 posts)

    Join Date
    Jun 2009
    Location
    Hartford, WI
    Posts
    1,509
    Rep Power
    111
    Actually, I'm just a bit unclear on something here. It's more that I work little in the JavaScript world. The thing I'm unclear about is the executing and handling of functions within functions. I've seen a few explanations out there, but based on the examples provided, I may be overlooking something...

    In the "Add New" section, the .on('click') calls the inner submitAction function, providing variables within parenthesis. That function is defined with a 'return function() { ... }'.
    In the "Cancel" section, the .on('click') calls the inner cancelEditing function, but without parenthesis. That function is defined without a 'return'.

    Overall, the general posts out there made it sound if the outer item using the function had the parenthesis attached, it would execute it returning the results. Without the parenthesis, it sounded it was just returning the internally defined function itself. I'm guessing it has something to do with the way each is called, but a bit unclear on that. If nothing is to truly be returned, does the "Add New" need to be after a 'return'?

    Current:
    Code:
    // Add new item
    $('.account').on('click', '.add', basicAction('account', 'add'));
    $('.invoice').on('click', '.add', basicAction('invoice', 'add'));
    
    // Cancel editing
    $('.account').on('click', '.cancel', basicAction('account', 'cancel'));
    $('.invoice').on('click', '.cancel', basicAction('invoice', 'cancel'));
    
    // Basic function definitions
    function basicAction(page, action) {
      if(action == 'add') {
        return function() {
          $(this).actionCall(page, 'add', $(this).closest('form').serialize());
        }
      } else if(action == 'cancel') {
        $(this).closest('span.td').siblings().each(function(index, cell) {
          $(this).text($(this).find('input[type=hidden]').val());
        });
        $(this).closest('span.td').toggleClass('editing')
      }
    }
    Edit: Or, as I was just glancing at it, is the requirement of the 'return' more in relation to the actionCall() function itself since it has returns...? And the 'Cancel' doesn't need a 'return' since it's a straight forward full action, and not calling any actions within itself that may have returns of their own...?
    Last edited by Triple_Nothing; December 8th, 2017 at 10:34 AM.
    He who knows not that he knows not is a fool, ignore him. He who knows that he knows not is ignorant, teach him. He who knows not that he knows is asleep, awaken him. He who knows that he knows is a leader, follow him.
  12. #7
  13. Wiser? Not exactly.
    Devshed God 2nd Plane (6000 - 6499 posts)

    Join Date
    May 2001
    Location
    Bonita Springs, FL
    Posts
    6,126
    Rep Power
    4103
    Originally Posted by Triple_Nothing
    In the "Add New" section, the .on('click') calls the inner submitAction function, providing variables within parenthesis. That function is defined with a 'return function() { ... }'.
    In the "Cancel" section, the .on('click') calls the inner cancelEditing function, but without parenthesis. That function is defined without a 'return'.
    The parenthesis/lack of parenthesis affect whether the function is invoked or not at that point in time.
    Code:
    $('.account').on('click', '.add', submitAction('account', 'add'));
    
    function submitAction(page, action){
        return function(){
            $(this).actionCall(page, action, $(this).closest('form').serialize());
        };
    }
    Here, the submitAction function is invoked immediately and the value it returns is passed into the .on function as the actual event handler. Since it's return value is the actual event handler, that's why you see return function(){ ... };

    Code:
    $('.account').on('click', '.cancel', cancelEditing);
    
    function cancelEditing(){
        $(this).closest('span.td').siblings().each(function(index, cell) {
            $(this).text($(this).find('input[type=hidden]').val());
        });
        $(this).closest('span.td').toggleClass('editing')
    }
    Here, because there are no parenthesis following the function name the function is not being invoked but instead just referenced similar to a variable. So a reference to the function is being passed into the .on function as the event handler and will be invoked later when the actual event occurs.

    Comments on this post

    • Triple_Nothing agrees : Thanks. ;-)
    Recycle your old CD's



    If I helped you out, show some love with some reputation, or tip with Bitcoins to 1N645HfYf63UbcvxajLKiSKpYHAq2Zxud

IMN logo majestic logo threadwatch logo seochat tools logo