Enhancing delete function with javascript

[AndrewTraub]

I have developed a php cart form that has delete buttons that open another page to confirm the delete action for when javascript is disabled.
I'd now like to iterate through those buttons when javascript is enabled and instead call a function to show a modal delete button. Here's my code which prevents the form submit but is not calling the delete function:

Code: [Select]

for (let btn of document.querySelectorAll("button[data-type=delete]")) {
    const value = btn.value;
    switch (btn.attributes[2].nodeValue) { //[2] = data-object
        case 'cart':
            btn.type = "button";
            btn.onclick = async function() {await confirmDeleteCartItem(value)};
            break;
    }
}
What am I missing?

[Jason Knight]

First thing I'd say you're screwing up on is using array index on the attributes which can vary from page-load to page-load and browser to browser. We have dataset or getAttribute, use it.

Next is your "variable for nothing" in the form of value.

Then there's the assignment of "onclick" on an existing element. Element.addEventListener exists for a reason, use it. I'd also have to see the form, but that you're trapping the "click" event on the button instead of the "submit" on the form also throws up warning flags.

And as much as I defend the use of "case" given these are event handlers creating them as anonymous is bad practice slowing your startup time significantly if you end up with several such 'delete' handlers. Thus I'd use an object of event handler methods instead.

Finally the junk use of promises. I'm just not a fan as IMHO they overcomplicate what should be a relatively simple thing. In point of fact you very much have an example of why I think promises are dumbass. From "needing" to "wait" for the event, to trickery to pass around a value you could extract from the button on the event, and so forth.

I'd have to see more of it to weigh in further, but...

Code: [Select]

{

const eventDelete = {

cart : (e) => {
/*
trigger your confirmation modal here.
Have its event/form do the delete.

Also pull the value from event.currentTarget.value here instead
of dicking around trying to pass it as a parameter before the event

Basically, ditch the promise rubbish and learn to use events properly!

*/
} // eventDelete.cart

}; // eventDelete

for (let button of document.querySelectorAll("button[data-type=delete]")) {
if (eventDelete[button.dataset.object]) {
button.addEventListener("click", eventDelete[button.dataset.object]);
button.type = "button";
}
}

}
... is me guessing wildly. Promises are NOT your friend, no matter how many so-called "experts" seem to be creaming their panties over them.

[AndrewTraub]

Thanks so much Jason!  Too bad you are retired .

[mmerlinn]

Thanks so much Jason!  Too bad you are retired .

Retired? People like Jason can never retire. The industry won't let him retire.

[AndrewTraub]

Fyi, here is the url for the javascript dialog class which uses promises:

https://dev.to/boyum/creating-a-simple-confirm-modal-in-vanilla-js-2lcl

[Jason Knight]

What the?!? that's... either something I wouldn't have JS doing in the first place, or I'd use window.confirm() for during the submit so as to lock out page activity.

Or... I'd just chain the submits since I'm not seeing ANYTHING there that actually warrants being event driven. That is a poster child for "promises for NOTHING" and probably double the code needed to do the job...

[AndrewTraub]

How would you rewrite the ConfirmDialog?