yiisoft / yii2

Yii 2: The Fast, Secure and Professional PHP Framework
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
14.24k stars 6.91k forks source link

Html a with data-confirm inside a form should not submit a form #17624

Closed PowerGamer1 closed 7 months ago

PowerGamer1 commented 5 years ago

What steps will reproduce the problem?

<?php $form = ActiveForm::begin(['action' => ['form-controller/action'], 'method' => 'get']); ?>
<?= Html::a('Click me', ['controller/action'], ['data-confirm' => 'Are you sure?']) ?>
<?= Html::a('Another click me', ['controller/action']) ?>
<?php ActiveForm::end(); ?>

What is the expected result?

Clicking the 'Click me' link should NOT submit the form and should navigate browser to the link URL ('controller/action') as usual (after the confirmation dialog). In other words the 'Click me' link should work exactly like 'Another click me' link but with confirmation dialog.

Such behavior is important for the following use case. There is GridView with checkboxes inside form. Multiple submit buttons outside GridView are used to submit the form with the value of selected checkboxes. Also GridView uses the ActionColumn with simple URLs for per-row actions such as row deletion (which requires confirmation). So the ActionColumn URLs need confirmation but must not submit the form.

What do you get instead?

Clicking the "Click me" link submits the form to 'form-controller/action'.

Additional info

Q A
Yii version 2.0.28
PHP version 7.3.1
s1lver commented 5 years ago

This is an old "mistake".

alex-code commented 5 years ago

If you just need a confirm for a link,

Html::a('Click me', ['controller/action'], ['onclick' => 'return confirm("test")']);

Can you have the GridView outside the form and link the checkboxes using the form attribute? <input type="checkbox" form="my-form" />

PowerGamer1 commented 5 years ago

If you just need a confirm for a link, Html::a('Click me', ['controller/action'], ['onclick' => 'return confirm("test")']);

This was a temporary workaround I used. The drawback: you mix HTML with JavaScript and if you have a lot of such buttons with confirm (GridView with a lot of rows, button for each row) you increase the size of the HTML page (the text onclick='return confirm("test")' is duplicated many times in generated HTML).

Can you have the GridView outside the form and link the checkboxes using the form attribute? <input type="checkbox" form="my-form" />

I don't know how well it is supported by browsers for HTML 5 but HTML 4 does not have such feature at all.

Besides, it would be strange to sometimes use Yii2-like code for confirms:

Html::a('Click me', ['controller/action'], ['data-confirm' => 'Are you sure?']) ?>

and sometimes do it manually like that:

Html::a('Click me', ['controller/action'], ['onclick' => 'return confirm("test")']);

depending on where you put the code in HTML (inside or outside the form).

alex-code commented 5 years ago

The data methods are designed to work with forms so workarounds are limited.

You could always have your own event handler,

$(document).on('click', 'a[data-my-confirm]', e => {
  return confirm($(e.target).data('my-confirm'));
});
Html::a('Click me', ['controller/action'], ['data-my-confirm' => 'Are you sure?']) ?>
PowerGamer1 commented 5 years ago

@alex-code The built-in Yii2 'data-confirm' should work like you show. Afterwards, if there is also a 'data-method' specified the existing form-related submit code of Yii2 should execute.

alex-code commented 5 years ago

The data-confirm option is part of the data methods code which uses forms.

https://github.com/yiisoft/yii2/blob/master/framework/assets/yii.js#L475

PowerGamer1 commented 5 years ago

The data-confirm option is part of the data methods code which uses forms. https://github.com/yiisoft/yii2/blob/master/framework/assets/yii.js#L475

So what?

In the Yii2-based projects I am working on, this Yii2 feature is used alot but exclusively for confirm, not to submit a form using Javascript. I am willing to bet this may be the primary use case in many other projects too. So the design of the feature in Yii2 framework needs to reflect that.

Besides, it doesn't make any sense to allow using confirm ONLY if you want to submit a form. So, as I said previously, the design of this feature in Yii2 should be: 'data-confirm' attribute does NOT dictate if any form will be submitted or not, it just deals with confirm. The 'data-method' attribute, on the other hand, will deal with form submittal.

I don't see any problem with modifying or fully rewriting the code you linked to achieve the above design.

alex-code commented 5 years ago

As an example, how should this work with your proposed changes?

Html::beginForm('/logout');
Html::a('Logout', '#', ['data-confirm' => 'Are you sure?']);
Html::endForm();

Rewriting the data methods could cause BC issues.

PowerGamer1 commented 5 years ago

As an example, how should this work with your proposed changes?

Html::beginForm('/logout');
Html::a('Logout', '#', ['data-confirm' => 'Are you sure?']);
Html::endForm();

As if:

<a href="#" onclick="return confirm('Are you sure?')">Logout</a>

Rewriting the data methods could cause BC issues.

Only if new code functions differently to the correctly documented behavior. And even then it can be easily dealt with in UPGRADE.md.

alex-code commented 5 years ago

Can you link to the docs that mention the data-methods? It could be they are wrong and causing confusion.

As far as i can see from the code the data-methods, inc data-confirm are for submitting data via forms. The confirm option is for convenience and not for separate use.

What you want can easily be done with one line,

$(document).on('click', 'a[data-my-confirm]', e => confirm($(e.target).data('my-confirm')));
PowerGamer1 commented 5 years ago

Can you link to the docs that mention the data-methods? It could be they are wrong and causing confusion.

I am not sure those docs even exist.

As far as i can see from the code the data-methods, inc data-confirm are for submitting data via forms. The confirm option is for convenience and not for separate use.

It does not really matter what and how was initially implemented. What metters is how it is used and how to make it better. See my reasons in previous posts.

What you want can easily be done with one line, $(document).on('click', 'a[data-my-confirm]', e => confirm($(e.target).data('my-confirm')));

Excellent, just as long as it is part of the framework, preferably using existing 'data-confirm' facility.