New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Confirm dialog on submit button is fired twice #6642

Closed
viilveer opened this Issue Dec 24, 2014 · 33 comments

Comments

Projects
None yet
@viilveer

viilveer commented Dec 24, 2014

I created a submit button using following markup:

Html::submitButton(
   Yii::t('admin-game', 'Delete'), 
   [
      'class' => 'btn',
      'data-confirm' => Yii::t('yii', 'Are you sure you want to delete selected items?'),
   ]
),

Now yii.js fires when I click the button and handleAction triggers the form submit again. If this is triggered the confirm popup will be also triggered.

yii.js line 173

$form.trigger('submit');

@qiangxue qiangxue added this to the 2.0.2 milestone Dec 24, 2014

@qiangxue qiangxue self-assigned this Dec 24, 2014

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Dec 26, 2014

Member

Reproduced. This is a tricky issue. It happens when data-confirm is used for a submit button in an active form. The first confirm dialog is triggered by the click handler of the button. The second is triggered by active form when validation succeeds (Line 501, $button.click()). Not sure how to fix this issue.

Member

qiangxue commented Dec 26, 2014

Reproduced. This is a tricky issue. It happens when data-confirm is used for a submit button in an active form. The first confirm dialog is triggered by the click handler of the button. The second is triggered by active form when validation succeeds (Line 501, $button.click()). Not sure how to fix this issue.

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Dec 27, 2014

Member

wouldn't it be better to apply the confirmation to the form submit event instead of the button click?

Member

cebe commented Dec 27, 2014

wouldn't it be better to apply the confirmation to the form submit event instead of the button click?

pana1990 added a commit to pana1990/yii2 that referenced this issue Dec 30, 2014

@pana1990

This comment has been minimized.

Show comment
Hide comment
@pana1990

pana1990 Dec 30, 2014

Contributor

I am just trying it and it's working, you have to check my pull request, please.

#6698

Contributor

pana1990 commented Dec 30, 2014

I am just trying it and it's working, you have to check my pull request, please.

#6698

@qiangxue qiangxue referenced this issue Dec 31, 2014

Closed

Fix #6642 #6698

@pana1990

This comment has been minimized.

Show comment
Hide comment
@pana1990

pana1990 Jan 6, 2015

Contributor

Hi,

I'm trying to add a hidden input before submitting the form, i do not think that is the best solution but i think it fix issue.

Replace

if ($button.length) {
    $button.click();
} else {
    // no submit button in the form
    $form.submit();
}

with

if ($button.length && $button.prop('name')) {
    $('<input>').attr({
        type: 'hidden',
        name: $button.prop('name'),
        value: $button.prop('value')
     }).appendTo($form);
}

$form.submit();

@qiangxue think about it.

Contributor

pana1990 commented Jan 6, 2015

Hi,

I'm trying to add a hidden input before submitting the form, i do not think that is the best solution but i think it fix issue.

Replace

if ($button.length) {
    $button.click();
} else {
    // no submit button in the form
    $form.submit();
}

with

if ($button.length && $button.prop('name')) {
    $('<input>').attr({
        type: 'hidden',
        name: $button.prop('name'),
        value: $button.prop('value')
     }).appendTo($form);
}

$form.submit();

@qiangxue think about it.

@qiangxue qiangxue modified the milestones: 2.0.2, 2.0.3 Jan 11, 2015

@cicsolutions

This comment has been minimized.

Show comment
Hide comment
@cicsolutions

cicsolutions Jan 19, 2015

I side-stepped this issue by using CSS positioning. I placed the delete button just outside the active form widget, but still in side the form container div. Then I set the form container div to have positioning relative and the delete button to be positioned absolute to the bottom right of the form container div. This approach may not work for everyone, depending on their page structure and layout, but it was a quick solution that worked for my needs.

cicsolutions commented Jan 19, 2015

I side-stepped this issue by using CSS positioning. I placed the delete button just outside the active form widget, but still in side the form container div. Then I set the form container div to have positioning relative and the delete button to be positioned absolute to the bottom right of the form container div. This approach may not work for everyone, depending on their page structure and layout, but it was a quick solution that worked for my needs.

@Faryshta

This comment has been minimized.

Show comment
Hide comment
@Faryshta

Faryshta Jan 20, 2015

Contributor

I think its much easier than that http://api.jquery.com/unbind/ the first
click unbinds the click event and no need to do anything else

2015-01-19 17:52 GMT-06:00 Caleb Crosby notifications@github.com:

I side-stepped this issue by using CSS positioning. I placed the delete
button just outside the active form widget, but still in side the form
container div. Then I set the form container div to have positioning
relative and the delete button to be positioned absolute to the bottom
right of the form container div. This approach may not work for everyone,
depending on their page structure and layout, but it was a quick solution
that worked for my needs.


Reply to this email directly or view it on GitHub
#6642 (comment).

Contributor

Faryshta commented Jan 20, 2015

I think its much easier than that http://api.jquery.com/unbind/ the first
click unbinds the click event and no need to do anything else

2015-01-19 17:52 GMT-06:00 Caleb Crosby notifications@github.com:

I side-stepped this issue by using CSS positioning. I placed the delete
button just outside the active form widget, but still in side the form
container div. Then I set the form container div to have positioning
relative and the delete button to be positioned absolute to the bottom
right of the form container div. This approach may not work for everyone,
depending on their page structure and layout, but it was a quick solution
that worked for my needs.


Reply to this email directly or view it on GitHub
#6642 (comment).

@Faryshta

This comment has been minimized.

Show comment
Hide comment
@Faryshta

Faryshta Jan 20, 2015

Contributor

Correction, unbind is deprecated, the correct functionality is
http://api.jquery.com/off/ since 1.7

2015-01-19 18:18 GMT-06:00 Faryshta angeldelcaos@gmail.com:

I think its much easier than that http://api.jquery.com/unbind/ the first
click unbinds the click event and no need to do anything else

2015-01-19 17:52 GMT-06:00 Caleb Crosby notifications@github.com:

I side-stepped this issue by using CSS positioning. I placed the delete
button just outside the active form widget, but still in side the form
container div. Then I set the form container div to have positioning
relative and the delete button to be positioned absolute to the bottom
right of the form container div. This approach may not work for everyone,
depending on their page structure and layout, but it was a quick solution
that worked for my needs.


Reply to this email directly or view it on GitHub
#6642 (comment).

Contributor

Faryshta commented Jan 20, 2015

Correction, unbind is deprecated, the correct functionality is
http://api.jquery.com/off/ since 1.7

2015-01-19 18:18 GMT-06:00 Faryshta angeldelcaos@gmail.com:

I think its much easier than that http://api.jquery.com/unbind/ the first
click unbinds the click event and no need to do anything else

2015-01-19 17:52 GMT-06:00 Caleb Crosby notifications@github.com:

I side-stepped this issue by using CSS positioning. I placed the delete
button just outside the active form widget, but still in side the form
container div. Then I set the form container div to have positioning
relative and the delete button to be positioned absolute to the bottom
right of the form container div. This approach may not work for everyone,
depending on their page structure and layout, but it was a quick solution
that worked for my needs.


Reply to this email directly or view it on GitHub
#6642 (comment).

@Faryshta

This comment has been minimized.

Show comment
Hide comment
@Faryshta

Faryshta Jan 20, 2015

Contributor

https://github.com/yiisoft/yii2/blob/master/framework/assets/yii.js#L301
the line that attach the events is here. i tried everything i could think
of to dettach an event on my yii2 instalation to no avail.

sorry

2015-01-19 19:29 GMT-06:00 Faryshta angeldelcaos@gmail.com:

Correction, unbind is deprecated, the correct functionality is
http://api.jquery.com/off/ since 1.7

2015-01-19 18:18 GMT-06:00 Faryshta angeldelcaos@gmail.com:

I think its much easier than that http://api.jquery.com/unbind/ the first

click unbinds the click event and no need to do anything else

2015-01-19 17:52 GMT-06:00 Caleb Crosby notifications@github.com:

I side-stepped this issue by using CSS positioning. I placed the delete
button just outside the active form widget, but still in side the form
container div. Then I set the form container div to have positioning
relative and the delete button to be positioned absolute to the bottom
right of the form container div. This approach may not work for everyone,
depending on their page structure and layout, but it was a quick solution
that worked for my needs.


Reply to this email directly or view it on GitHub
#6642 (comment).

Contributor

Faryshta commented Jan 20, 2015

https://github.com/yiisoft/yii2/blob/master/framework/assets/yii.js#L301
the line that attach the events is here. i tried everything i could think
of to dettach an event on my yii2 instalation to no avail.

sorry

2015-01-19 19:29 GMT-06:00 Faryshta angeldelcaos@gmail.com:

Correction, unbind is deprecated, the correct functionality is
http://api.jquery.com/off/ since 1.7

2015-01-19 18:18 GMT-06:00 Faryshta angeldelcaos@gmail.com:

I think its much easier than that http://api.jquery.com/unbind/ the first

click unbinds the click event and no need to do anything else

2015-01-19 17:52 GMT-06:00 Caleb Crosby notifications@github.com:

I side-stepped this issue by using CSS positioning. I placed the delete
button just outside the active form widget, but still in side the form
container div. Then I set the form container div to have positioning
relative and the delete button to be positioned absolute to the bottom
right of the form container div. This approach may not work for everyone,
depending on their page structure and layout, but it was a quick solution
that worked for my needs.


Reply to this email directly or view it on GitHub
#6642 (comment).

@pana1990

This comment has been minimized.

Show comment
Hide comment
@pana1990

pana1990 Jan 20, 2015

Contributor

@Faryshta try this :

Replace in https://github.com/yiisoft/yii2/blob/master/framework/assets/yii.activeForm.js#L498

if ($button.length) {
    $button.click();
} else {
    // no submit button in the form
    $form.submit();
}

with

if ($button.length && $button.prop('name')) {
    $('<input>').attr({
        type: 'hidden',
        name: $button.prop('name'),
        value: $button.prop('value')
     }).appendTo($form);
}

$form.submit();
Contributor

pana1990 commented Jan 20, 2015

@Faryshta try this :

Replace in https://github.com/yiisoft/yii2/blob/master/framework/assets/yii.activeForm.js#L498

if ($button.length) {
    $button.click();
} else {
    // no submit button in the form
    $form.submit();
}

with

if ($button.length && $button.prop('name')) {
    $('<input>').attr({
        type: 'hidden',
        name: $button.prop('name'),
        value: $button.prop('value')
     }).appendTo($form);
}

$form.submit();
@jonvargas

This comment has been minimized.

Show comment
Hide comment
@jonvargas

jonvargas Jan 22, 2015

Facing the same issue here, just subscribed to this thread to track the resolution efforts.

jonvargas commented Jan 22, 2015

Facing the same issue here, just subscribed to this thread to track the resolution efforts.

@matias-yii

This comment has been minimized.

Show comment
Hide comment
@matias-yii

matias-yii Feb 20, 2015

@pana1990 Your code works!!! Thanks!!!

matias-yii commented Feb 20, 2015

@pana1990 Your code works!!! Thanks!!!

@pana1990

This comment has been minimized.

Show comment
Hide comment
@pana1990

pana1990 Feb 21, 2015

Contributor

@matias-yii This is a small patch , developers should seek a more elegant solution, any idea?

Contributor

pana1990 commented Feb 21, 2015

@matias-yii This is a small patch , developers should seek a more elegant solution, any idea?

@xskif

This comment has been minimized.

Show comment
Hide comment
@xskif

xskif Feb 24, 2015

Contributor

Facing the same issue in the GridView delete link.
Now, before send request for deletion, confirm dialog fired twice.

UPD: I'm use ActiveForm with GridView. First confirm dialog window checks validation, and second send request.

Contributor

xskif commented Feb 24, 2015

Facing the same issue in the GridView delete link.
Now, before send request for deletion, confirm dialog fired twice.

UPD: I'm use ActiveForm with GridView. First confirm dialog window checks validation, and second send request.

@pana1990

This comment has been minimized.

Show comment
Hide comment
@pana1990
Contributor

pana1990 commented Feb 25, 2015

@qiangxue qiangxue modified the milestones: 2.0.4, 2.0.3 Mar 1, 2015

@francisberesford

This comment has been minimized.

Show comment
Hide comment
@francisberesford

francisberesford Mar 24, 2015

I think pana1990's solution is good, it also solves another problem where the name attribute of the button does not get submitted when clicked using the current code.

My question is there any important reason the current code is attempting to do $button.click(); rather than just $form.submit();? I believe any javascript click() events would require $button.trigger('click'); to trigger any javascript click events attached to the button, so that can't be the reason can it?

francisberesford commented Mar 24, 2015

I think pana1990's solution is good, it also solves another problem where the name attribute of the button does not get submitted when clicked using the current code.

My question is there any important reason the current code is attempting to do $button.click(); rather than just $form.submit();? I believe any javascript click() events would require $button.trigger('click'); to trigger any javascript click events attached to the button, so that can't be the reason can it?

@pana1990

This comment has been minimized.

Show comment
Hide comment
@pana1990

pana1990 Mar 24, 2015

Contributor

@francisberesford it will make sure the value of the button input is submitted together with the form.

Contributor

pana1990 commented Mar 24, 2015

@francisberesford it will make sure the value of the button input is submitted together with the form.

@francisberesford

This comment has been minimized.

Show comment
Hide comment
@francisberesford

francisberesford Mar 25, 2015

@pana1990 If that's what it's supposed to do it doesn't work. At least in Firefox or Chrome. Your code does work however.

francisberesford commented Mar 25, 2015

@pana1990 If that's what it's supposed to do it doesn't work. At least in Firefox or Chrome. Your code does work however.

@pana1990

This comment has been minimized.

Show comment
Hide comment
@pana1990

pana1990 Mar 25, 2015

Contributor

@francisberesford it work without data-cofirm option. I could trying send pull request with my solution but i think that it is not the best way.

Contributor

pana1990 commented Mar 25, 2015

@francisberesford it work without data-cofirm option. I could trying send pull request with my solution but i think that it is not the best way.

@francisberesford

This comment has been minimized.

Show comment
Hide comment
@francisberesford

francisberesford Mar 25, 2015

I traced the problem with the name attribute not posting with data-confirm to yii.js line 157

            if (method === undefined) {
                if (action && action != '#') {
                    window.location = action;
                } else if ($e.is(':submit') && $form.length) {
                    $form.trigger('submit');
                }
                return;
            }

it is using $form.trigger('submit'); and not a $button.click() in this case (maybe for good reason?)

You'll see that they use your technique in this file quite frequently e.g.

$form.append('<input name="_method" value="' + method + '" type="hidden">');

I see no problem with your solution and it fixes two issues. I think you should do a pull request.

francisberesford commented Mar 25, 2015

I traced the problem with the name attribute not posting with data-confirm to yii.js line 157

            if (method === undefined) {
                if (action && action != '#') {
                    window.location = action;
                } else if ($e.is(':submit') && $form.length) {
                    $form.trigger('submit');
                }
                return;
            }

it is using $form.trigger('submit'); and not a $button.click() in this case (maybe for good reason?)

You'll see that they use your technique in this file quite frequently e.g.

$form.append('<input name="_method" value="' + method + '" type="hidden">');

I see no problem with your solution and it fixes two issues. I think you should do a pull request.

pana1990 added a commit to pana1990/yii2 that referenced this issue Mar 25, 2015

@pana1990

This comment has been minimized.

Show comment
Hide comment
@pana1990

pana1990 Mar 25, 2015

Contributor

@francisberesford done!, thanks for help me ;)

#7872

Contributor

pana1990 commented Mar 25, 2015

@francisberesford done!, thanks for help me ;)

#7872

@francisberesford

This comment has been minimized.

Show comment
Hide comment
@francisberesford

francisberesford Mar 25, 2015

@pana1990 {insert thumbs up emoticon}

francisberesford commented Mar 25, 2015

@pana1990 {insert thumbs up emoticon}

@abassanini

This comment has been minimized.

Show comment
Hide comment
@abassanini

abassanini Mar 26, 2015

Hello
The same behavior using DetailView and/or GridView as mentioned by @xskif
I tried @pana1990 solution, and worked in GridView, but it messed up DetailView. Now the delete button does not delete the record, but nicely ask just once :)

I wish I could help more than just reporting
By the way, using kartik\grid\GridView and kartik\detail\DetailView

abassanini commented Mar 26, 2015

Hello
The same behavior using DetailView and/or GridView as mentioned by @xskif
I tried @pana1990 solution, and worked in GridView, but it messed up DetailView. Now the delete button does not delete the record, but nicely ask just once :)

I wish I could help more than just reporting
By the way, using kartik\grid\GridView and kartik\detail\DetailView

@francisberesford

This comment has been minimized.

Show comment
Hide comment
@francisberesford

francisberesford Mar 26, 2015

@abassanini I could not replicate this problem. I have put my GridView inside an ActiveForm and the delete link in yii\grid\ActionColumn does not fire twice. Also it still deletes the record with the old code and the new provided by @pana1990

Could it be specific to the kartik extension?.

francisberesford commented Mar 26, 2015

@abassanini I could not replicate this problem. I have put my GridView inside an ActiveForm and the delete link in yii\grid\ActionColumn does not fire twice. Also it still deletes the record with the old code and the new provided by @pana1990

Could it be specific to the kartik extension?.

@xskif

This comment has been minimized.

Show comment
Hide comment
@xskif

xskif Mar 26, 2015

Contributor

@abassanini, disable client validation on the tabular form solve the issue.

UPD: @francisberesford kartik extension it's just builder for tabular form inside GridView in one step.

Contributor

xskif commented Mar 26, 2015

@abassanini, disable client validation on the tabular form solve the issue.

UPD: @francisberesford kartik extension it's just builder for tabular form inside GridView in one step.

@abassanini

This comment has been minimized.

Show comment
Hide comment
@abassanini

abassanini Mar 26, 2015

Sorry for asking this, but enableClientValidation property is not in DetailView nor Krajee DetailView

Certainly, I'm in the wrong direction :|

abassanini commented Mar 26, 2015

Sorry for asking this, but enableClientValidation property is not in DetailView nor Krajee DetailView

Certainly, I'm in the wrong direction :|

@abassanini

This comment has been minimized.

Show comment
Hide comment
@abassanini

abassanini Mar 26, 2015

@francisberesford using DetalView from Krajee not Kartik (sorry the misspelling)

view.php:

use kartik\detail\DetailView;

echo DetailView::widget([
    'model' => $model,
    'attributes'=>$attributes,
    'hover'=>true,
    'mode'=>Yii::$app->request->get('edit')=='t' ? DetailView::MODE_EDIT : DetailView::MODE_VIEW,
    'panel'=>[
            'heading'=>$this->title,
            'type'=>DetailView::TYPE_INFO,
    ],

        'deleteOptions'=>[
            'url'=>['delete', 'id' => $model->mac_address],
            'data'=>[
                'confirm'=>Yii::t('app', 'Confirm Deleting'),
                'method'=>'post',
            ],
        ],
]);

abassanini commented Mar 26, 2015

@francisberesford using DetalView from Krajee not Kartik (sorry the misspelling)

view.php:

use kartik\detail\DetailView;

echo DetailView::widget([
    'model' => $model,
    'attributes'=>$attributes,
    'hover'=>true,
    'mode'=>Yii::$app->request->get('edit')=='t' ? DetailView::MODE_EDIT : DetailView::MODE_VIEW,
    'panel'=>[
            'heading'=>$this->title,
            'type'=>DetailView::TYPE_INFO,
    ],

        'deleteOptions'=>[
            'url'=>['delete', 'id' => $model->mac_address],
            'data'=>[
                'confirm'=>Yii::t('app', 'Confirm Deleting'),
                'method'=>'post',
            ],
        ],
]);

@samdark samdark closed this Mar 26, 2015

@samdark samdark reopened this Mar 26, 2015

@xskif

This comment has been minimized.

Show comment
Hide comment
@xskif

xskif Mar 27, 2015

Contributor

@abassanini , try to explore request/response by browser debugger, it's may be internal error.
This issue about form button confirmation dialog, not just button with confirmation of action.

Contributor

xskif commented Mar 27, 2015

@abassanini , try to explore request/response by browser debugger, it's may be internal error.
This issue about form button confirmation dialog, not just button with confirmation of action.

@jotapeserra

This comment has been minimized.

Show comment
Hide comment
@jotapeserra

jotapeserra Jul 1, 2016

I could revolve this by doing:

$confirmTxt = 'confirmation text';
echo Html::hiddenInput('afterSubmit');
echo Html::button('submit',
[
'class' => 'btn btn-warning',
'onclick'=>"if(confirm('".$confirmTxt."')){
$('input[name=\"afterSubmit\"]').val(\"delete\");
$('form#toolbarProductosForm').submit();
}",
]);

jotapeserra commented Jul 1, 2016

I could revolve this by doing:

$confirmTxt = 'confirmation text';
echo Html::hiddenInput('afterSubmit');
echo Html::button('submit',
[
'class' => 'btn btn-warning',
'onclick'=>"if(confirm('".$confirmTxt."')){
$('input[name=\"afterSubmit\"]').val(\"delete\");
$('form#toolbarProductosForm').submit();
}",
]);

@dhimancontact

This comment has been minimized.

Show comment
Hide comment
@dhimancontact

dhimancontact Jan 8, 2017

I am still facing same issue with 2.0.6 version

dhimancontact commented Jan 8, 2017

I am still facing same issue with 2.0.6 version

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Jan 8, 2017

Member

@dhimancontact the latest version is 2.0.10, please try that one and also dev-master, to provide a useful report. Also please open a new issue with more details on your problem. Thanks!

Member

cebe commented Jan 8, 2017

@dhimancontact the latest version is 2.0.10, please try that one and also dev-master, to provide a useful report. Also please open a new issue with more details on your problem. Thanks!

@yupe

This comment has been minimized.

Show comment
Hide comment
@yupe

yupe Jan 9, 2017

Contributor

Have same issue on 2.0.10 gridview with delete buttons is inside the ActiveForm

Contributor

yupe commented Jan 9, 2017

Have same issue on 2.0.10 gridview with delete buttons is inside the ActiveForm

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Jan 9, 2017

Member

@yupe please open a separate issue w/ details.

Member

samdark commented Jan 9, 2017

@yupe please open a separate issue w/ details.

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Jan 9, 2017

Member

please open a new issue and describe what you are doing in more detail please.

Member

cebe commented Jan 9, 2017

please open a new issue and describe what you are doing in more detail please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment