There are a few ways of dealing with duplicates
I prefer the last of those techniques.
Why are numbered identifiers being used in the code? Using a sequential list of unique identifiers is a clear indicator that the wrong technique is being used, and that something other than unique identifiers should be used instead.
What identifiers are being used in the code?
Which of those identifiers are used by the code?
All of those use numbered unique identifiers, committing programming sins that need not occur.
We can remove the need for row_id_nn, by instead using DOM navigation techniques to walk up the DOM until we get to the row.
As you are using jQuery in your code I will use jQuery techniques to achieve that.
In the remove row event handler, the this keyword is representing the button that was clicked on. It can get tricky when the this keyword is used in several placed to refer to different things, so I will use a local variable for the remove button.
// var row_id = $(this).attr("id");
var removeButton = this;
var row_id = $(removeButton).attr("id");
Now that we have an easy and clear reference to the remove button, we can use the jQuery closest element method to go up through the ancestors until we get to the <tr> element.
// $('#row_id_' + row_id).remove();
removeButton.closest("tr").remove();
The row_id_nn identifier is no longer being used to achieve anything, and can be removed from the code.
// html_code += '<tr id="row_id_' + count + '">';
html_code += '<tr>';
The updated code that no longer uses row_id_nn is found at https://jsfiddle.net/vLjdwktc/
It is by other simple techniques that we can improve other parts of the code too, to remove the reliance on numbered identifiers.
Currently the remove button identifier is used to try and access an order_item_final_amount element. However, assuming that there was one, it can be a name attribute or a class attribute instead.
If itās a class attribute, we can easily access it by gaining a reference to the row:
var removeButton = this;
var row = removeButton.closest("tr");
...
// var total_item_amount = $('#order_item_final_amount' + row_id).val();
var total_item_amount = row.find(".order_item_final_amount").val();
and if itās a name instead of a class, we can use an updated selector instead:
var removeButton = this;
var row = removeButton.closest("tr");
...
// var total_item_amount = $('#order_item_final_amount' + row_id).val();
var total_item_amount = row.find("[name=order_item_final_amount]").val();
Either way, the row id is no longer being used by anything, and can now be removed.
// html_code += '<td><button type="button" name="remove_row" id="' + count + '" class="btn btn-danger btn-xs remove_row">X</button></td>';
html_code += '<td><button type="button" name="remove_row" class="btn btn-danger btn-xs remove_row">X</button></td>';
...
// var row_id = $(removeButton).attr("id");
We can also update the previous removeButton code to use that row reference instead too.
// removeButton.closest("tr").remove();
row.remove();
The updated code with the above changes is found at https://jsfiddle.net/vLjdwktc/2/
In the same place that the row id and row remove was being done, I also see count being used there. In the quest to remove the reliance on numbered identifiers, we can remove the need for that global count variable too.
row.remove();
count--;
$('#total_item').val(count);
We can instead count the number of rows in the table.
Right now the table is a bit muddled, with the header row being mixed in with the table body rows. We can move that into its own <thead> section, using <tbody> for the main content of the table.
<table id="invoice-item-table" class="table table-bordered">
<thead>
<tr>
<th width="7%">Sr No.</th>
<th width="20%">Item Name</th>
<th width="5%">Quantity</th>
</tr>
</thead>
<tbody>
<tr>
...
</tr>
</tbody>
</table>
This way, the <tbody> element now contains the correct number of <tr> elements.
As the row itself is being removed, we canāt used that row reference after it has been removed. What we can do is to do the work before the row is removed, or to gain a reference to the tbody element so that we can do the work afterwards. Iāll use the latter technique.
We can easily access the tbody after we get the row.
var row = $(removeButton).closest("tr");
var tbody = row.parent();
We can now update the total_item element.
row.remove();
count--;
// $('#total_item').val(count);
$('#total_item').val(tbody.children().length);
That total_item is a hidden form field, but it still gets updated.
<input type="hidden" name="total_item" id="total_item" value="1" />
<input type="submit" name="create_invoice" id="create_invoice" class
The screen shows Total NaN after removing a row. That part doesnāt work and is not related to any of the changes being made.
The reason why that part is NaN is from a separate issue, because there is no price or tax rate in your example code. I presume that you have removed those for the sake of simplicity, which has left parts of your code in a non-working state.
I will leave things like that for now, and carry on focusing on appropriately removing the numbered identifiers.
For now, the updated code is at https://jsfiddle.net/vLjdwktc/3/
The next numbered identifier to remove is used in calculating the price.
for (var no = 1; no <= count; no++) {
console.log({itemname: $('#item_name' + no).val()});
if ($.trim($('#item_name' + no).val()).length == 0) {
alert("Please Enter Item Name");
$('#item_name' + no).focus();
return false;
}
When looping through each item, it really should be each item row that is looped through. That way we can easily gain access to anything in each row that we need to access.
Before making much progress, we need to replace the return false statements. Those are all designed to prevent the default behaviour of submitting the form from taking place.
A much safer way to achieve that is by using the evt.preventDefault() method instead.
// $('#create_invoice').click(function() {
$('#create_invoice').click(function(evt) {
...
if ($.trim($('#item_name' + no).val()).length == 0) {
alert("Please Enter Item Name");
$('#item_name' + no).focus();
// return false;
evt.preventDefault();
return;
}
We can now turn that for loop into a forEach loop instead. The row index starts at 0, so Iāll have the no value be one more than the row index.
// for (var no = 1; no <= count; no++) {
rows.each(function (index, itemRow) {
no = index + 1;
...
// }
});
That lets us keep on using the no variable for now, as we convert the rest of the code to use itemRow instead.
Because preventDefault() is being used, we donāt need to use return anymore inside of the each loop.
rows.each(function (index, itemRow) {
...
evt.preventDefault();
// return;
}
With the console.log, we can make a simple request for the item name, and use that instead.
var itemName = $(itemRow).find("select");
// console.log({itemname: $('#item_name' + no).val()});
console.log({itemname: itemName.val()});
We can now replace all old occurrences of item_name with the updated itemName one instead.
// if ($.trim($('#item_name' + no).val()).length == 0) {
if ($.trim(itemName.val()).length == 0) {
...
// $('#item_name' + no).focus();
itemName.focus();
We can do the same thing with itemQuantity too.
var itemQuantity = $(itemRow).find("[name='order_item_quantity[]']");
// if ($.trim($('#order_item_quantity' + no).val()).length == 0) {
if ($.trim(itemQuantity.val()).length == 0) {
...
// $('#order_item_quantity' + no).focus();
itemQuantity.focus();
Now that things are working without the no variable, we can remove that, and the numbered item_name and item_quantity identifiers too.
// html_code += '<td><select name="item_name[]" id="item_name' + count + '" class="form-control select3"><option value="">ŚÆŪŲ±Ų§Ł</option>' + options + '</select></td>';
html_code += '<td><select name="item_name[]" class="form-control select3"><option value="">ŚÆŪŲ±Ų§Ł</option>' + options + '</select></td>';
// html_code += '<td><input type="text" name="order_item_quantity[]" id="order_item_quantity' + count + '" data-srno="' + count + '" class="form-control input-sm number_only order_item_quantity" /></td>';
html_code += '<td><input type="text" name="order_item_quantity[]" data-srno="' + count + '" class="form-control input-sm number_only order_item_quantity" /></td>';
...
rows.each(function (index, itemRow) {
// no = index + 1;
I also removed the final submit line as it seems to achieve nothing.
// $('#invoice_form').submit();
That was in the click event handler of the submit button, so the event handler is going to submit anyway, so long as none of the preventDefault() parts of the code are executed.
The updated code with the above changes is found at https://jsfiddle.net/vLjdwktc/4/
Lastly, we can remove the global count variable from the code by getting the number of rows in the tbody section of the table instead.
Because the item count is used in several places, we can put that in a function to help simplify things.
function getItemCount() {
var invoice_item_table = $("#invoice-item-table");
var rows = invoice_item_table.find("tbody").children();
var count = rows.length;
return count;
}
We can now use that function in the code where needed:
$(document).ready(function() {
var final_total_amt = $('#final_total_amt').text();
// var count = 1;
...
$(document).on('click', '#add_row', function() {
// count++;
var count = getItemCount() + 1;
$('#total_item').val(count);
...
// function cal_final_total(count) {
function cal_final_total() {
var count = getItemCount();
var final_item_total = 0;
for (j = 1; j <= count; j++) {
...
// cal_final_total(count);
cal_final_total();
There is now no more global count variable throughout the code.
The updated code with those changes is found at https://jsfiddle.net/nump9twz/
Even though there are still some numbered unique identifiers, such as sr_no and data-srno, it would be irresponsible to remove those without knowing if that removal will impact other things.
Despite that, now that the numbered unique identifiers that were being used by the code have all been removed, it is now easier to understand and update the code to meet changing circumstances.
You can do one of the ways. in here https://jsfiddle.net/vLjdwktc/4/
you could check for duplicates when the selection is being made
you could check for duplicates when a new item is added
you could check for duplicates when the create button is clicked
you could remove the selected options from the dropdown option list when a selection is made
No, none of those ways have been done in that code. That previous work has been doing some cleaning up, making it easier for any of those ways to be achieved.
I got excited about using each to iterate through the rows. Thatās not needed. To allow for exiting early at the first error, Iāll revert things back to using a for loop instead of the each loop that was there.
// rows.each(function (index, itemRow) {
for (i = 0; i < rows.length; i += 1) {
var itemRow = rows[i];
...
// });
}
With those above techniques for checking for duplicates, Iāll start with the create button.
The for loop checks that each row has an item name, and quantity. Before doing those individual checks, it would be useful to first find out if an item name has been duplicated.
We will want to get the invoice item rows. Because we already do that in a few other places in the code, this is a good time to create a getInvoiceRows function to do that.
function getInvoiceRows() {
var invoice_item_table = $("#invoice-item-table");
var rows = invoice_item_table.find("tbody").children();
return rows;
}
We can now replace those other areas of the code that get the rows with that function:
function getItemCount() {
// var invoice_item_table = $("#invoice-item-table");
// var rows = invoice_item_table.find("tbody").children();
var rows = getInvoiceRows();
...
}
...
// var invoice_item_table = $("#invoice-item-table");
// var rows = invoice_item_table.find("tbody").children();
var rows = getInvoiceRows();
for (i = 0; i < rows.length; i += 1) {
...
}
}
Before that for loop, we can now add the code that checks if there are any item duplicates. A nice simple manner to achieve that is to put a list of the selected item names into a set. If there are duplicates then the set will be smaller than the number of items.
First, we use map to get an array list of the item names:
var rows = getInvoiceRows();
var itemNames = rows.find("select").map(function (i, select) {
return select.value;
});
Then, we use Set to get the unique values, and compare against the number of item names.
if (new Set(itemNames).size < itemNames.length) {
alert("Please Remove Duplicate Items");
evt.preventDefault();
return false;
}
I also noticed that some of the uniquely numbered identifiers such as item_name1 and order_item_quantity_1 are floating around in the HTML code, so those have been removed.
The updated code where the create button checks for duplicates, is found at https://jsfiddle.net/2at8ns3L/1/
Because we are checking for duplicates again, it makes sense to move the existing duplicate checking code into a separate function. That way we can easily use it from different places.
function hasDuplicateItems() {
var rows = getInvoiceRows();
var itemNames = rows.find("select").map(function (i, select) {
return select.value;
});
return new Set(itemNames).size < itemNames.length;
}
We can now replace the code from the previous post with the following updated code instead:
if (hasDuplicateItems()) {
alert("Please Remove Duplicate Items");
evt.preventDefault();
return false;
}
That hasDuplicateItems function can also be used when adding a new item.
// $(document).on('click', '#add_row', function(evt) {
$(document).on('click', '#add_row', function() {
if (hasDuplicateItems()) {
alert("Please Remove Duplicate Items");
evt.preventDefault();
return false;
}
...
});
It ends up being a sanity-check, so that the add row doesnāt occur when there are duplicate items.
The updated code where adding rows also checks for duplicate items is found at https://jsfiddle.net/2at8ns3L/2/
We can just use the change event to check the item selection when its changed.
$(document).on("change", "#invoice-item-table select", function(evt) {
if (hasDuplicateItems()) {
alert("Please Change Duplicate Item");
evt.preventDefault();
return false;
}
});
We are now warned whenever the itemname is changed to be one that already exist.
The updated code that checks when items change, is found at https://jsfiddle.net/2at8ns3L/3/
Thank you, these are good way. You explained it very well, and I took advantage of it,
Sorry, thereās a problem with this section.
https://jsfiddle.net/vLjdwktc/4/
Data doesnāt go well into the database,
Deleting this line has created a problem // $('#invoice_form').submit();
If so, arrange this https://jsfiddle.net/2at8ns3L/3/ with this,
https://jsfiddle.net/vLjdwktc/3/
Itās not required to remove it. Leave it in.
I think these two will arrange with each other, and the problem will be solved
https://jsfiddle.net/2at8ns3L/3/ with https://jsfiddle.net/vLjdwktc/3/
because thereās no problem with data going to database
Just arrange javascript code the problem will be solved
Just to explain more fully, each set of code is not a new set with only the situation being explained applied to it. Instead, the sets of code are sequential, with each set of code adding on top of all of the previous code that comes before it.
Code here is not a problem working well with the php and database
The problem is
When I select multiple rows and click create button into database one of the rows decreases
And that was after this update https://jsfiddle.net/vLjdwktc/3/
look at this
With a date and foo 4, bar 5, and baz 6, clicking the create button results in the following form information being submitted from the web page to the server:
order_no: 12345
class_id: <?php echo isset($class_id) ? $class_id: '' ?>
order_date: 2021-03-17
item_name[]: foo
order_item_quantity[]: 4
item_name[]: bar
order_item_quantity[]: 5
item_name[]: baz
order_item_quantity[]: 6
total_item: 3
create_invoice: create
I got that information by opening up the developer tools, and going to the network panel. When clicking the create button the network panel shows the above information in the Form Data section.
I recommend that you do similar, so that you can figure out if the problem exists in that information going to the PHP code, or with the PHP code itself.