phalcon / cphalcon

High performance, full-stack PHP framework delivered as a C extension.
https://phalcon.io
BSD 3-Clause "New" or "Revised" License
10.79k stars 1.97k forks source link

Default checked attribute for a checkbox #16374

Open ahashesht opened 1 year ago

ahashesht commented 1 year ago

Referencing this line:

https://github.com/phalcon/cphalcon/blob/e01ae9b5a2412be4bcac0772f2ec7463b2f2f960/phalcon/Html/Helper/Input/Checkbox.zep#L117

Assuming I create my checkbox like this:

$checkbox = new Check('check', ['value' => 1]);
$checkbox->setDefault(1);

Then the rendered checkbox will not be checked by default! Is this by design? In Phalconv4 there was a method called prepareAttributes that handled assigned the correct checked attribute. I believe checked should be set in this case.

s-ohnishi commented 1 year ago

Since setDefault() is for setting the initial value of the value attribute, it will not work as expected. (However, I can understand your expectations...)

With the idea of reversal, if you do the following, it will work even in the current version. I found it while trying various things.

$elm = new Element\Check('testcb', ['checked' => '1',]);
$elm->setDefault(1);
$this->add($elm);

However, this was no good.

$elm = new Element\Check('testcb', ['checked' => 1,]);
$elm->setDefault(1);
$this->add($elm);

And although this result seems strange,

$elm = new Element\Check('testcb', ['value' => 1,]);
$this->add($elm);

{{ form.render('testcb', ['checked' : 1]) }} is OK but {{ form.render('testcb', ['checked' : '1']) }} is NG.

noone-silent commented 11 months ago

There are some Problems with the checkbox and radio Tags (like always)

https://github.com/phalcon/cphalcon/blob/e01ae9b5a2412be4bcac0772f2ec7463b2f2f960/phalcon/Html/Helper/Input/Checkbox.zep#L111

  1. If the user sets the checked attribute to checked, this should be honored before anything else. Currently there is no check for this at all.
  2. If the checkbox is checked is a strict check. So it needs not only be of the same value, it also needs to be of the same type. If we talk about database values, values could be parsed as string or integers, depending on a lot of different settings. And a natural match is nearly impossible.

Proposal:

  1. Check if the attribute checked is set to checked and treat it as such. There is no need to check anything anymore.
  2. Change the strict check to strcmp and cast both the values to string, if it's not an object or array, to increase the matching.