I have this example table made with material design lite . The last column of the table has a DELETE button that, when pressed, should display a dialog box where the option to delete the selected row is given.
Right now this is my code. It seems to be working, however I'm using a global variable fila
which I don't like too much. What recommendations can you give me to improve this code?
<!doctype html>
<html>
<head>
<link rel="stylesheet" href="https://fonts.googleapis.com/icon?family=Material+Icons">
<link rel="stylesheet" href="https://code.getmdl.io/1.1.1/material.indigo-pink.min.css">
</head>
<body>
<dialog class="mdl-dialog">
<h4 class="mdl-dialog__title">Borrado filas</h4>
<div class="mdl-dialog__content">
<p>
Desea eliminar la fila seleccionada?
<p>
</div>
<div class="mdl-dialog__actions">
<button type="button" class="mdl-button aceptar">Aceptar</button>
<button type="button" class="mdl-button close">Cancelar</button>
</div>
</dialog>
<table class="mdl-data-table mdl-js-data-table mdl-data-table--selectable mdl-shadow--2dp">
<thead>
<tr>
<th class="mdl-data-table__cell--non-numeric">Material</th>
<th>Cantidad</th>
<th>Borrar</th>
</tr>
</thead>
<tbody>
<tr>
<td class="mdl-data-table__cell--non-numeric">Acrylic (Transparent)</td>
<td>25</td>
<td><button id="show-dialog" type="button" class="mdl-button borrar">Borrar</button></td>
</tr>
<tr>
<td class="mdl-data-table__cell--non-numeric">Plywood (Birch)</td>
<td>50</td>
<td><button id="show-dialog" type="button" class="mdl-button borrar">Borrar</button></td>
</tr>
<tr>
<td class="mdl-data-table__cell--non-numeric">Laminate (Gold on Blue)</td>
<td>10</td>
<td><button id="show-dialog" type="button" class="mdl-button borrar">Borrar</button></td>
</tr>
</tbody>
</table>
<script src="http://code.jquery.com/jquery-1.12.0.min.js"></script>
<script defer src="https://code.getmdl.io/1.1.1/material.min.js"></script>
<script>
var dialog = document.querySelector('dialog');
var showDialogButton = document.querySelector('#show-dialog');
var fila; //variable global
if (! dialog.showModal) {
dialogPolyfill.registerDialog(dialog);
}
$('body').on('click', '.borrar', function(e) {
fila = $(this).parent().parent();
dialog.showModal();
});
dialog.querySelector('.close').addEventListener('click', function() {
dialog.close();
});
dialog.querySelector('.aceptar').addEventListener('click', function() {
fila.remove();
dialog.close();
});
</script>
</body>
</html>
If you want to avoid the global variable, one option would be to add a class (or an attribute
data-*
) to the row you are operating on. So: if the user clicks "Cancel", the class is removed from the row; or if the user clicks on "Accept" the element with that class is deleted.Although not directly related to the global variable, another thing I would change in the code is this:
$(this).parent().parent()
. That works fine with the current structure, but if you change the structure in the future, you'll have to change that line as well. Instead if you replace it with$(this).closest("tr")
, you no longer have to worry about the button changing its relative position within the row.So the code would look like this:
The first solution to eliminate the use of global variables is to use an IIFE , that way all the variables you declare are local and there is no contamination of the
global scope
.You
IIFE
can find it in these two formats. The only thing that changes is the position of the calling parentheses.The second variant I recommend is that you wrap all your code in a jQuery ready function since IIFEs are not a replacement for these events . These will be executed immediately and in your code there is a script with attribute
defer
which means that it will be executed after the document is loaded which can leave your functiondialogPolyfill
as undefined for example.Either of these two formats is recommended
Finally and as a personal note I see that in some places you use jQuery and in others pure javascript for the selectors. I recommend consistency, if you decide to put jQuery use jQuery because browsers have inconsistencies between them and jQuery takes care of dealing with them for you, otherwise it would be up to you to deal with them to have a uniform experience. This is general advice, it doesn't necessarily apply to your code snippet.