Skip to content

Commit

Permalink
dont add required class on wrapper div
Browse files Browse the repository at this point in the history
  • Loading branch information
euromark committed Apr 26, 2013
1 parent 03e5207 commit 67cc7f6
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 3 deletions.
19 changes: 18 additions & 1 deletion lib/Cake/Test/Case/View/Helper/FormHelperTest.php
View file
Expand Up @@ -8697,7 +8697,7 @@ public function testRequiredOnCreate() {
$this->Form->inputDefaults(array('required' => false));
$result = $this->Form->input('Contact.imrequired');
$expected = array(
'div' => array('class' => 'input text required'),
'div' => array('class' => 'input text'),
'label' => array('for' => 'ContactImrequired'),
'Imrequired',
'/label',
Expand All @@ -8711,6 +8711,23 @@ public function testRequiredOnCreate() {

$result = $this->Form->input('Contact.imrequired', array('required' => false));
$this->assertTags($result, $expected);

$result = $this->Form->input('Contact.imrequired', array('required' => true));
$expected = array(
'div' => array('class' => 'input text required'),
'label' => array('for' => 'ContactImrequired'),
'Imrequired',
'/label',
'input' => array(
'required' => 'required', 'type' => 'text', 'name' => 'data[Contact][imrequired]',
'id' => 'ContactImrequired'
),
'/div'
);
$this->assertTags($result, $expected);

$result = $this->Form->input('Contact.imrequired', array('required' => null));
$this->assertTags($result, $expected);
}

/**
Expand Down
6 changes: 4 additions & 2 deletions lib/Cake/View/Helper/FormHelper.php
View file
Expand Up @@ -1271,8 +1271,10 @@ protected function _divOptions($options) {
} elseif (is_array($div)) {
$divOptions = array_merge($divOptions, $div);
}

if ($this->_introspectModel($this->model(), 'validates', $this->field())) {
if (
$this->_extractOption('required', $options) !== false &&
$this->_introspectModel($this->model(), 'validates', $this->field())
) {
$divOptions = $this->addClass($divOptions, 'required');
}
if (!isset($divOptions['tag'])) {
Expand Down

4 comments on commit 67cc7f6

@kimegede
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the reason not to have the "required" class on the wrapper div?

@ADmad
Copy link
Member

@ADmad ADmad commented on 67cc7f6 Apr 29, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kimegede The "required" class is removed only If you specify the html5's required attribute to false for an input.

@kimegede
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ADmad Sorry, should read the code also, and not only the commit message :)
Sounds good ;)

@ADmad
Copy link
Member

@ADmad ADmad commented on 67cc7f6 Apr 29, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kimegede No probs, I agree the commit message could have been better 😄

Please sign in to comment.