5
\$\begingroup\$

I have the following function which opens a modal and puts some value into a div. It works but the function looks ugly, I hope for improvements (maybe more dynamic).

submitDelete: function() {

    // projects
    $('button[name="removeProj"]').on('click', function(e){
        var $form      = $(this).closest('form'); // closest parent form
        var $id        = $(this).data('id');
        var $title     = $(this).data('title');
        var $createdAt = $(this).data('created-at');
        var $updatedAt = $(this).data('updated-at');
        e.preventDefault();
        if($id != '') {
            $("#prodId").empty().append($id);
        } else {
            $("#prodId").empty().append('-');
        }
        if($title != '') {
            $("#prodTitle").empty().append($title);
        } else {
            $("#prodTitle").empty().append('-');
        }
        if($createdAt != '') {
            $("#prodCreatedAt").empty().append($createdAt);
        } else {
            $("#prodCreatedAt").empty().append('-');
        }
        if($updatedAt != '') {
            $("#prodUpdatedAt").empty().append($updatedAt);
        } else {
            $("#prodUpdatedAt").empty().append('-');
        }
        $('#confirm').modal({ backdrop: 'static', keyboard: false })
        $('#delete').click(function() {
            $form.trigger('submit'); // submit the form
        });
    });
\$\endgroup\$
4
\$\begingroup\$

In JavaScript you can do this:

var example = someValue || 'fallbackValue';

if someValue does not have a value, example=='fallbackValue', otherwise it'll be whatever value someValue has; You can integrate this in your code.

// You can now replace this:
if($id != '') {
    $("#prodId").empty().append($id);
} else {
    $("#prodId").empty().append('-');
}
// With this:
$("#prodId").empty().append( $(this).data('id') || '-') 
// Or just set it directly:
$("#prodId").val( $(this).data('id') || '-') 

Resulting in:

submitDelete: function() {

    // projects
    $('button[name="removeProj"]').on('click', function(e){
        e.preventDefault();
        var $form      = $(this).closest('form'); // closest parent form

        $("#prodId").val(        $(this).data('id') || '-'); 
        $("#prodTitle").val(     $(this).data('title') || '-');
        $("#prodCreatedAt").val( $(this).data('created-at') || '-');
        $("#prodUpdatedAt").val( $(this).data('updated-at') || '-'); 

        $('#confirm').modal({ backdrop: 'static', keyboard: false })
        $('#delete').click(function() {
            $form.trigger('submit'); // submit the form
        });
    });
}

*Instead of .val() you can use .text() or .html() to set the values of other types of elements like divs.

\$\endgroup\$
  • 1
    \$\begingroup\$ interesting I will check this out, I think you forgot ";" after val(...) \$\endgroup\$ – utdev Feb 17 '17 at 15:11
  • \$\begingroup\$ I forgot to mention that .val will not work since $("#prodId").. are not forms elements they are regular div + p Tags \$\endgroup\$ – utdev Feb 17 '17 at 15:32
  • \$\begingroup\$ The ; isn't needed, but I do prefer to use them, updated the anders. Also updated my answer with one more line at the bottom. \$\endgroup\$ – Martijn Feb 20 '17 at 8:04
1
\$\begingroup\$

I would write it like this:

$('button[name="removeProj"]').on('click', function(e) {
    e.preventDefault();

    var button = this;
    var $form  = $(this).closest('form');

    [
     { target: 'prodId', key: 'id' },
     { target: 'prodTitle', key: 'title' },
     { target: 'prodCreatedAt', key: 'created-at' },
     { target: 'prodUpdatedAt', key: 'updated-at' },
    ].map(function(item){
        $("#".concat(item.target)).empty().append(button.data(item.key) || '-');
    });

    $('#confirm').modal({ backdrop: 'static', keyboard: false  });

    $('#delete').click(function() {
        $form.trigger('`submit');
    });
});

.map() docs here


Update #2:

If you change your ids like this:

  • prodId -> id
  • prodTitle -> title,
  • prodCreatedAt => created-at,
  • prodUpdatedAt => updated-at

Then you code can be changed to :

submitDelete: function() {
    $('button[name="removeProj"]').on('click', function(e) {
        e.preventDefault();
        var $form = $(this).closest('form');
        var button = this;

        ['id', 'title', 'created-at', 'updated-at'].map(function(item) {
            $("#".concat(item)).empty().append(button.data(item) || '-');
        });

        $('#confirm').modal({  backdrop: 'static', keyboard: false })
        $('#delete').click(function() {
            $form.trigger('submit'); // submit the form
        });
    });
}
\$\endgroup\$
  • \$\begingroup\$ It's not wrong to loop through the ID's, but you're now forcing that your ID's must be those names. It might work in this case, but I've run into enough situations where this seems nice, and came back to bite me in the ... \$\endgroup\$ – Martijn Feb 20 '17 at 13:39
  • \$\begingroup\$ @Martijn, well, you can use first solution anyway. \$\endgroup\$ – Dan Cantir Feb 20 '17 at 14:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.