# jquery – JavaScript function that reuses a modal for adding, deleting, and viewing an item

Avoid inline handlers – they have way too many problems to be worth using nowadays, such as a demented scope chain and quote escaping issues. Attach event listeners properly using Javascript with `addEventListener` instead.

While you could iterate over all buttons and add a listener to each, consider using event delegation instead: add a single click listener to the table, and on click, if the click was on a button, examine the button’s attributes or index to see which one was clicked, and examine the `data-id` attribute to get the ID.

Something along the lines of:

``````<td>
<i class="far fa-eye"></i>
</button>
&nbsp;
<i class="far fa-edit"></i>
</button>
&nbsp;
<i class="far fa-trash-alt"></i>
</button>
</td>
``````
``````\$(table).on('click', 'button', function() {
const { id, mode } = this.dataset;
// ...
});
``````

Always use `const` when you can – see https://softwareengineering.stackexchange.com/questions/278652/how-much-should-i-be-using-let-vs-const-in-es6. `let` permits reassignment, which makes the code’s intent harder to understand at a glance – whenever a developer sees a `let`, they may well think “This was declared with `let`, not `const`, so I need to be on the lookout for where this will get reassigned.”

Give functions names with verbs – the `loadingOverlay` function might be more readable if it was named something like `showLoadingOverlay` or `loadOverlay` or something of the sort. (not sure what it does)

Accidental use of the comma operator`rowObjects('license', id)` is equivalent to `rowObjects(id)`. Did you mean `rowObjects('license' + id)`? Consider using a linter to warn you about these sorts of potential mistakes automatically.

Array keys should be numeric only – if your keys are going to contain non-numeric strings like `license`, or if you aren’t going to have all elements of an array filled, use an object instead:

``````const rowObjects = {};
``````

Assigning titles concisely can be done by making an object indexed by `mode` string. When you need to figure out the title to set, just look up the property on the object: see below.

Abstract separate functionality into functions – see this anecdote. While you don’t have to go that far, if you have a non-trivial block of code that does something somewhat separate from the rest of the code in a section, consider putting it into a function instead. I’d put the population of the `\$form` into a function – see below.

Button toggling Instead of `needsButton` and `inputsDisabled`, consider hiding both buttons by default, then showing one or the other depending on whether `delete`, or something other than `view` was pressed:

``````const titlesByMode = {
};
\$(table).on('click', 'button', function() {
const { id, mode } = this.dataset;
modalMode = mode;
\$form.find('input').val('');
} else {
populateForm(id);
}
\$form.find(':input').prop('disabled', mode === 'view' || mode === 'delete');

\$('h4#modal-title', modal).text(titlesByMode(mode));
// Hide both buttons by default:
\$('#submitButton', modal).hide();
\$('#deleteButton', modal).hide();
if (mode === 'delete') {
\$('#deleteButton', modal).show();
} else if (mode !== 'view') {
\$('#submitButton', modal).show();
}
});

const populateForm = (id) => {
const data = rowObjects('license' + id);

\$form.find('input#email').val(data.email);
\$form.find('input#key').val(data.activation_key);
\$form.find('input#expiration').val(moment(data.expiration).format('MM/DD/YYYY'));
\$form.find('select#type-select').val(data.type);
\$form.find('input#hidden-id').val(data.id);
};
$$```$$
``````