Eng 22484 ability to select multiple line items in a single click during refund #374
Conversation
… a list by a single click.
…ple-line-items-in-a-single-click-during-refund
| <div class="form-group"> | ||
| <label class="col-sm-2 control-label"></label> | ||
| <div class="col-sm-10"> | ||
| <input type="checkbox" id="selectall"> |
There was a problem hiding this comment.
nit: how about using camelCase for new checkbox identifier?
(there are two naming styles exist for id in this file:
- snake like for
div_refund_amount - camel like for
invoiceItems)
There was a problem hiding this comment.
Nice suggestion we have renamed the selectall to selectAll.
| */ | ||
| var onClickInvoiceItemsSelectAll = function(event){ | ||
| var isChecked = false; | ||
| if ($(this).is(':checked')) { |
There was a problem hiding this comment.
as we have this function specifically for selectAll (as it is reflected in its name), I think we can directly check by id like:
var isChecked = $('#selectAll').prop('checked');
There was a problem hiding this comment.
We have changed $(this).is(':checked') to $('#selectAll').prop('checked'); as $('#selectAll') make more sense here.
| /* | ||
| * Attach handler onClickInvoiceItemsSelectAll for selectall checkbox | ||
| */ | ||
| $('input').filter(function() { |
There was a problem hiding this comment.
Since there is only one Select All checkbox (pls correct if I am wrong) with known id (selectall or selectAll), can we use it directly like
$('#selectAll').click(onClickInvoiceItemsSelectAll);?
There was a problem hiding this comment.
Thanks for pointing out we have update the event binding to $('#selectAll').click(onClickInvoiceItemsSelectAll);
| $('input').filter(function() { return this.id.match(/tf_adj_/) }).each(function() { | ||
| var textFieldId = this.id; | ||
| var checkboxId = textToCheckboxId(this.id); | ||
| if(checkboxId && textFieldId){ |
There was a problem hiding this comment.
Checking textFieldId might be redundant as there is a check of textToCheckboxId(textFieldId)
There was a problem hiding this comment.
textFieldId is removed
| var checkboxId = textToCheckboxId(this.id); | ||
| if(checkboxId && textFieldId){ | ||
| $("#" + checkboxId).prop('checked', isChecked); | ||
| $("#" + textFieldId).prop('readonly', isChecked); |
There was a problem hiding this comment.
I guess to not to create new jQuery object repeatedly we can catch the object like
var textField = $(this); in the beginning of the function as use it like
textField.prop('readonly', isChecked);
if (!isChecked) {
textField.attr('value', textField.attr('originalValue'));
}
There was a problem hiding this comment.
Have you pushed it? Because I still see the old way of creating jQuery objects
|
|
||
| /* | ||
| * When clicking checkbox for each item, disable amount and recompute total refund amount | ||
| * When clicking checkbox for each item, disable amount, check select all status and recompute total refund amount |
There was a problem hiding this comment.
Check Select All status or clicking on Select all
grana-equinix
deleted the
ENG-22484-Ability-to-select-multiple-line-items-in-a-single-click-during-refund
branch
Fix #368
onClickInvoiceItemsSelectAllandcheckSelectAllCheckboxStatusto handle click events on selecting the checkbox.onClickInvoiceItemsSelectAll- will trigger when the select-all is checked and uncheckedcheckSelectAllCheckboxStatus- will trigger when line items are checked and uncheckedkillbill-admin-ui_issues_368.pdf