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\$

2 Answers 2

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\$
3
  • 1
    \$\begingroup\$ interesting I will check this out, I think you forgot ";" after val(...) \$\endgroup\$
    – utdev
    Feb 17, 2017 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, 2017 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, 2017 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\$
2
  • \$\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, 2017 at 13:39
  • \$\begingroup\$ @Martijn, well, you can use first solution anyway. \$\endgroup\$
    – Dan Cantir
    Feb 20, 2017 at 14:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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