Removed 'type' => 'submit' by default for FormHelper::button() #365

Closed
wants to merge 1 commit into
from

Projects

None yet

2 participants

@ProLoser
ProLoser commented Dec 8, 2011

Why? Because:

  • The type attribute is not required for buttons, so there's no real reason to include
  • 'Submit' is a very unsafe default behavior. When people don't explicitly wish to use submit buttons, such as for cancel, expand, js-related, etc, submitting the form is not something people will want to override. Especially because of the first point.
  • Follow the rule of design to expectation. The button should do what people expect it to. If no type is set, don't add one.
  • When calling $this->Form->input('Call JS', array('type' => 'button')) the type is always 'submit' and cannot be changed. This is the main reason for this fix. Since type is already passed (to input()) it is impossible to correct this behavior.
@markstory
Member

I guess since this is slated for 2.1 the change is reasonable.

Dean Sofer Removed 'type' => 'submit' by default for FormHelper::button()
Why? Because:

* The type attribute is not required for buttons, so there's no real reason to include
* 'Submit' is a very unsafe default behavior. When people __don't__ explicitly wish to use submit buttons, such as for cancel, expand, js-related, etc, submitting the form is not something people will want to override. Especially because of the first point.
* Follow the rule of design to expectation. The button should do what people expect it to. If no type is set, don't add one.
* __When calling `$this->Form->input('Call JS', array('type' => 'button'))` the type is always 'submit' and cannot be changed.__ This is the main reason for this fix. Since type is already passed (to `input()`) it is impossible to correct this behavior.
18b2c88
@ProLoser
ProLoser commented Feb 5, 2012

Any update on this?

@markstory
Member

I think creating submit buttons with input() is a bit silly, but that's just me. input() isn't meant as a facade to every other method on the helper. I can see the utility in not having type set for buttons, but I'm a bit worried about the impact this has on the effect this would have on people upgrading. Where buttons that used to submit no longer do. I guess that is something that can be documented.

@markstory
Member

Since this change would introduce what could be construed as a backwards compatibility break, I made a different change in [afecb71]. The type is now nullable, but still defaults to 'submit'. We can make the other change in a future release.

@markstory markstory closed this Feb 14, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment