IfnotFr / PopConfirm

A simple action confirmation plugin for jQuery based on Twitter Bootstrap Popover
MIT License
99 stars 35 forks source link

Can't submit form when clicking Yes button on PopConfirm #6

Closed bhall7 closed 10 years ago

bhall7 commented 10 years ago

For some reason, I'm having a hard time submitting an HTML form using the latest version PopConfirm. I tried to create an example on JSFiddle (http://jsfiddle.net/bhall7x/Y4t8a/12/). When you click the Delete button in the example, then click Yes in the confirmation popover, it should submit the form, but it doesn't submit anything. I thought that not being able to submit the form was just a JSFiddle issue, but I tried it on my web server and had the same problem. What am I doing wrong?

When I don't use PopConfirm, the form submits fine.

IfnotFr commented 10 years ago

I will take a look this week :)

bhall7 commented 10 years ago

I noticed that if I add container: false to the PopConfirm options, it lets me submit the form. I suspect that there is something to do with the container that prevents an adjacent form from being submitted with the latest version of PopConfirm. Note that regardless I cannot get a sample form to submit on JSFiddle.

pacahon commented 10 years ago

Hi, I have the save problem. This code is broken I guess https://github.com/Ifnot/PopConfirm/blob/master/jquery.popconfirm.js#L97 (and 104 line) because .popover element have a body tag parent Is it clear to change this line to jQuery('.popover:visible').find bla-bla-bla Or maybe you have a better solution?

PlippiePlop commented 10 years ago

I noticed that on click of the button its expecting the 'next' element is the popover. Since the default setting for container is 'body' the self.next function will never work. See line 97 in the script:

self.next('.popover').find('.confirm-dialog-btn-confirm').bind('click', function(e) {
                    for(var i = 0; i < arrayActions.length; i++) {
                        arrayActions[i].apply(self);
                    }

                    self.popover('hide');
                });
                self.next('.popover').find('.confirm-dialog-btn-abord').bind('click', function(e) {
                    self.popover('hide');
                });

I changed it to this:

    if(options.container==="body"){

                    $('.popover').on('click','.confirm-dialog-btn-confirm', function(e) {
                        for(var i = 0; i < arrayActions.length; i++) {                      
                            arrayActions[i].apply(self);
                        }
                        self.popover('hide');
                    }).on('click','.confirm-dialog-btn-abord', function(e) {                        
                        console.log(options);
                        self.popover('hide');
                    });

                } else {                    

                    self.next('.popover').on('click','.confirm-dialog-btn-confirm', function(e) {
                        for(var i = 0; i < arrayActions.length; i++) {
                            arrayActions[i].apply(self);
                        }                       
                        self.popover('hide');
                    }).on('click','.confirm-dialog-btn-abord', function(e) {                        
                        console.log('cancel');
                        self.popover('hide');
                    });

                }
IfnotFr commented 10 years ago

Sorry i am overbusy at the moment. :(

I think i will have more time in a couple of weeks.

Can-you fork/push your proposition to the project ? :)

Thanks for your contrib :)

bhall7 commented 10 years ago

I can confirm that @PlippiePlop's changes have fixed the issue and it is now working as intended. @Ifnot, please merge, pending your testing and approval.

IfnotFr commented 10 years ago

Ok, great, thanks @PlippiePlop :)

I will make some improvements to the code before merging it : got many duplicated code.

Then after i will merge it.

iliajie commented 10 years ago

I can confirm PlippiePlop fix is working!! Thanks!!

thierrydussau commented 10 years ago

@Ifnot :Thanks for your work on this, love it. @PlippiePlop : Thanks for the fix, it was working pretty well.

I had one problem though, in my use case, the click event bound to the "popConfirm" Element was firing a javascript action but not reloading the page, Hence the event was bound then fired as many times as the user had clicked on "yes".

I ended up adding a bunch of "off", seems to be working for me :

if (options.container === "body") {
                        $('.popover').on('click', '.confirm-dialog-btn-confirm',function (e) {
                            for (var i = 0; i < arrayActions.length; i++) {
                                arrayActions[i].apply(self);
                            }
                            self.popover('hide');
                            $(this).off(e);
                        }).on('click', '.confirm-dialog-btn-abord', function (e) {
                            self.popover('hide');
                            $(this).off(e);
                        });

                    } else {
                        self.next('.popover').on('click', '.confirm-dialog-btn-confirm',function (e) {
                            for (var i = 0; i < arrayActions.length; i++) {
                                arrayActions[i].apply(self);
                            }
                            self.popover('hide');
                            $(this).off(e);
                        }).on('click', '.confirm-dialog-btn-abord', function (e) {
                            self.popover('hide');
                            $(this).off(e);
                        });
                    }
PlippiePlop commented 10 years ago

You can also use .one() to single fire a click

iliajie commented 10 years ago

@thierrydussau I can confirm, that your code prevent multiple executions for my AJAX handlers as described here https://github.com/Ifnot/PopConfirm/issues/9

Thanks

holly73 commented 10 years ago

Hmmm, maybe I'm too stupid for all this JavaScript, but I tried all the changes you guys suggested and it still doesn't send the form.

PlippiePlop commented 10 years ago

@holly73 Did you attach the popconfirm to a submit button?

holly73 commented 10 years ago

Here is the button's HTML

<button type="submit" id="deleteform" name="deleteform" form="del_form" class="btn btn-default pull-right" />

But the button is outside the

tags, thats's why there is form="del_form". Does this matter?

And at the end of the page there is:

<script src="../assets/js/jquery.popconfirm.js"></script>
<script type="text/javascript">
$(document).ready( function() {
   $("#deleteform").popConfirm();
});     
</script>

Without the PopConfirm code the form just works fine.

PlippiePlop commented 10 years ago

Well normaly the button should be placed inside the form. The script of PopConfirm expect it to be inside the form. This is a flaw in the script.

    var form = $(this).parents('form:first');

The above will never work if the form submit button is outside the form. You are however correct with the HTML5 spec of form="formID" but since older browsers do not support this and of all IE browsers I think only IE11 supports it. It is not recommended to use this HTML5 feature.

What PopConfirm badly needs is a callback function on the Cancel and Confirm buttons. So you can register the click event and take action accordingly.

IfnotFr commented 10 years ago

This problem looks to be fixed with #10 can someone confirm ? (i tried a jsfiddle but I do not know if it was always good).

@PlippiePlop sorry to not merge your fork, but it was a good starting point for fixing problems.

PlippiePlop commented 10 years ago

@ifNot no problem m8, I am rewriting my fork as we speak, cleaning up a lot of code and also adding callbacks into the code for confirm and cancel.

IfnotFr commented 10 years ago

Closing this issue as this problem is solved :)

holly73 commented 10 years ago

I'm still struggling to get it to work. My problem is: I've got a form with two submit buttons. One to update the data, the other to delete them. The dialog only pops up when deleting. Unfortunately, with PopConfirm it doesn't pass the value attribute of the button, nor the name. Before, I tried it with two forms and the form="form_id" added to one button outside the form, but as you guys told me this doesn't work. I'm looking for a solution now to send some "identifier" for the script to identify it the data should be updated or deleted. But I've got no idea how to do this.

IfnotFr commented 10 years ago

Can-you put a example showing the problem on a jsfiddle ?

PlippiePlop commented 10 years ago

You have to write a callback function on the popup 'ok' setting. I was working on adding callbacks for ok and cancel on my own extended fork but had to stop temporary because of workload....

Something like: $('#second-button').popconfirm({ yesBtn: function(callback){ // On ok click do action like trigger $('#form-02').submit() } });

IfnotFr commented 10 years ago

I consider this problem solved as no responses in 15 days :)