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

Question

I have a table that has buttons per row that makes views/edit an item in a modal and they have a layout like so:

<td>
    <button class="btn btn-sm btn-default" onclick="setModalMode('view', '8')" data-toggle="modal" data-target="#license-add" data-id="8">
        <i class="far fa-eye"></i>
    </button>
    &nbsp;
    <button class="btn btn-sm btn-info" onclick="setModalMode('update', '8')" data-toggle="modal" data-target="#license-add" data-id="8">
        <i class="far fa-edit"></i>
    </button>
    &nbsp;
    <button class="btn btn-sm btn-danger" onclick="setModalMode('delete', '8')" data-toggle="modal" data-target="#license-add" data-id="8">
        <i class="far fa-trash-alt"></i>
    </button>
</td>

It calls the setModalMode functiton and toggles the mode of the modal:

let modal = 'div#license-add';
let $form = $('form#modal-form');
let modalMode = ''; // this is used by my AJAX function to point what URL should my requests go
let rowObjects = []; // this gets filled up by values on a different function everytime my table generates

function setModalMode(mode, id) {
    loadingOverlay(false, modal);
    modalMode = mode;

    let title = 'Modal title';
    let inputsDisabled = false;
    let hasData = true;
    let needsButton = true;
    let isDelete = false;

    if (mode === 'add') {
        title = 'Add new license';

        hasData = false;
    } else if (mode === 'update') {
        title = 'Updating license';
    } else if (mode === 'view') {
        title = 'Viewing license';

        needsButton = false;
        inputsDisabled = true;
    } else if (mode === 'delete') {
        title = 'Delete this license?';

        inputsDisabled = true;
        isDelete = true;
    } else {
        return;
    }

    if (hasData) {
        let 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);
    } else {
        $form.find(':input').val('');
    }

    if (needsButton) {
        if (isDelete) {
          $('#deleteButton', modal).show();
          $('#submitButton', modal).hide();
        } else {
          $('#deleteButton', modal).hide();
          $('#submitButton', modal).show();
        }
    } else {
        $('#submitButton', modal).hide();
        $('#deleteButton', modal).hide();
    }

    $('h4#modal-title', modal).text(title);
    $form.find(':input').prop('disabled', inputsDisabled);
}

Although the code does its job perfectly well, I feel like my code is a mess of if statements

Answer 1

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>
    <button data-mode="view" class="btn btn-sm btn-default" data-toggle="modal" data-target="#license-add" data-id="8">
        <i class="far fa-eye"></i>
    </button>
    &nbsp;
    <button data-mode="update" class="btn btn-sm btn-info" data-toggle="modal" data-target="#license-add" data-id="8">
        <i class="far fa-edit"></i>
    </button>
    &nbsp;
    <button data-mode="delete" class="btn btn-sm btn-danger" data-toggle="modal" data-target="#license-add" data-id="8">
        <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 = {
    add: 'Add new license',
    update: 'Updating license',
    view: 'Viewing license',
    delete: 'Delete this license?',
};
$(table).on('click', 'button', function() {
    showLoadingOverlay(false, modal);
    const { id, mode } = this.dataset;
    modalMode = mode;
    if (mode === 'add') {
        $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);
};
```

Answer 2

You have four modes and for each of them, you need to render UI respectively. So my suggestion is to write four functions to do that. For flexibility's sake in each function, you can have any rendering logic you want. The parts of logic that are duplicated across different modes can be extracted to separate functions. Also, if you want your code to look even cleaner I suggest using a framework like React because of the JSX.

Here is my solution:

let modal = 'div#license-add';
let $form = $('form#modal-form');
let rowObjects = []; // this gets filled up by values on a different function everytime my table generates
rowObjects[8] = {
    email: 'test@test.com',
    activation_key: 'activation_key',
    expiration: 123,
    type: 'type',
    id: 'id'
}

const fillInputs = ($form, { email, activation_key, expiration, type, id }) => {
    $form.find('input#email').val(email);
    $form.find('input#key').val(activation_key);
    $form.find('input#expiration').val(moment(expiration).format('MM/DD/YYYY'));
    $form.find('select#type-select').val(type);
    $form.find('input#hidden-id').val(id);
}

const hideButtons = () => {
    $('#deleteButton', modal).hide();
    $('#submitButton', modal).hide();
}

const showDeleteButton = () => {
    $('#deleteButton', modal).show();
    $('#submitButton', modal).hide();
}

const showSubmitButton = () => {
    $('#deleteButton', modal).hide();
    $('#submitButton', modal).show();
}

const setTitle = (title) => {
    $('h4#modal-title', modal).text(title);
}

const toggleInputs = ($form, isDisabled) => {
    $form.find(':input').prop('disabled', isDisabled);
}

const clearInputs = ($form) => {
    $form.find(':input').val('');
}

const viewMode = ($form, data) => function () {
    fillInputs($form, data);
    hideButtons();
    setTitle('Viewing license');
    toggleInputs($form, true);
};

const addMode = ($form) => function () {
    clearInputs($form);
    showSubmitButton();
    setTitle('Add new license');
    toggleInputs($form, false);
};

const updateMode = ($form, data) => function () {
    fillInputs($form, data);
    showSubmitButton();
    setTitle('Updating license');
    toggleInputs($form, false);
};

const deleteMode = ($form, data) => function () {
    fillInputs($form, data);
    showDeleteButton();
    setTitle('Delete this license?');
    toggleInputs($form, true);
};

function setModalMode(mode, id) {
    const funcByMode = (mode) => ({
        'add': addMode,
        'view': viewMode,
        'update': updateMode,
        'delete': deleteMode,
    }[mode]);

    const getUpdater = funcByMode(mode);
    const updateUI = getUpdater($form, rowObjects[id]);
    updateUI();
}