Skip to content
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.

Sign up for GitHub

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

Jump to bottom

3.0 form idprefix #2980

Merged
merged 5 commits into from Mar 10, 2014
Merged

3.0 form idprefix #2980

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ac25629
Move FormHelper::_domId() to IdGeneratorTrait.
ADmad Mar 9, 2014
78c8b3e
Add ID attribute prefixing feature.
ADmad Mar 9, 2014
3084a02
Implement id prefixing for radio and multicheckbox widgets.
ADmad Mar 9, 2014
825eab3
Remove extra newline.
ADmad Mar 9, 2014
17bcaf3
Move View/Widget/IdGeneratorTrait to View/Helper.
ADmad Mar 10, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 9 additions & 11 deletions src/View/Helper/FormHelper.php
View file
Open in desktop
Expand Up @@ -29,6 +29,7 @@
use Cake\View\Helper\StringTemplateTrait;
use Cake\View\StringTemplate;
use Cake\View\View;
use Cake\View\Widget\IdGeneratorTrait;
use Cake\View\Widget\WidgetRegistry;
use DateTime;
use Traversable;
Expand All @@ -43,6 +44,7 @@
*/
class FormHelper extends Helper {

use IdGeneratorTrait;
use StringTemplateTrait;

/**
Expand Down Expand Up @@ -260,6 +262,7 @@ protected function _isRequiredField($validationRules) {
* - `encoding` Set the accept-charset encoding for the form. Defaults to `Configure::read('App.encoding')`
* - `context` Additional options for the context class. For example the EntityContext accepts a 'table'
* option that allows you to set the specific Table class the form should be based on.
* - `idPrefix` Prefix for generated ID attributes.
*
* @param mixed $model The context for which the form is being defined. Can
* be an ORM entity, ORM resultset, or an array of meta data. You can use false or null
Expand All @@ -286,10 +289,12 @@ public function create($model = null, $options = []) {
'url' => null,
'default' => true,
'encoding' => strtolower(Configure::read('App.encoding')),
'idPrefix' => null
];

$this->_idPrefix = $options['idPrefix'];
$action = $this->url($this->_formUrl($context, $options));
unset($options['url'], $options['action']);
unset($options['url'], $options['action'], $options['idPrefix']);

$htmlAttributes = [];
switch (strtolower($options['type'])) {
Expand Down Expand Up @@ -426,6 +431,7 @@ public function end($secureAttributes = []) {

$this->requestType = null;
$this->_context = null;
$this->_idPrefix = null;
return $out;
}

Expand Down Expand Up @@ -708,16 +714,6 @@ public function label($fieldName, $text = null, $options = array()) {
return $this->widget('label', $attrs);
}

/**
* Generate an ID suitable for use in an ID attribute.
*
* @param string $value The value to convert into an ID.
* @return string The generated id.
*/
protected function _domId($value) {
return mb_strtolower(Inflector::slug($value, '-'));
}

/**
* Generate a set of inputs for `$fields`. If $fields is null the fields of current model
* will be used.
Expand Down Expand Up @@ -1200,6 +1196,7 @@ public function radio($fieldName, $options = [], $attributes = []) {
]);
}
$attributes['options'] = $options;
$attributes['idPrefix'] = $this->_idPrefix;

return $hidden . $this->widget('radio', $attributes);
}
Expand Down Expand Up @@ -1656,6 +1653,7 @@ public function multiCheckbox($fieldName, $options, $attributes = []) {
];
$attributes = $this->_initInputField($fieldName, $attributes);
$attributes['options'] = $options;
$attributes['idPrefix'] = $this->_idPrefix;

$hidden = '';
if ($attributes['hiddenField']) {
Expand Down
2 changes: 1 addition & 1 deletion src/View/StringTemplate.php
View file
Open in desktop
Expand Up @@ -170,7 +170,7 @@ public function formatAttributes($options, $exclude = null) {
$exclude = [];
}

$exclude = ['escape' => true] + array_flip($exclude);
$exclude = ['escape' => true, 'idPrefix' => true] + array_flip($exclude);
$escape = $options['escape'];
$attributes = [];

Expand Down
27 changes: 25 additions & 2 deletions src/View/Widget/IdGeneratorTrait.php
View file
Open in desktop
Expand Up @@ -22,6 +22,13 @@
*/
trait IdGeneratorTrait {

/**
* Prefix for id attribute.
*
* @var string
*/
protected $_idPrefix = null;

/**
* A list of id suffixes used in the current rendering.
*
Expand All @@ -39,7 +46,7 @@ protected function _clearIds() {
}

/**
* Generate an ID attribute for a radio button.
* Generate an ID attribute for an element.
*
* Ensures that id's for a given set of fields are unique.
*
Expand All @@ -48,7 +55,8 @@ protected function _clearIds() {
* @return string Generated id.
*/
protected function _id($name, $val) {
$name = mb_strtolower(Inflector::slug($name, '-'));
$name = $this->_domId($name);

$idSuffix = mb_strtolower(str_replace(array('/', '@', '<', '>', ' ', '"', '\''), '-', $val));
$count = 1;
$check = $idSuffix;
Expand All @@ -58,4 +66,19 @@ protected function _id($name, $val) {
$this->_idSuffixes[] = $check;
return trim($name . '-' . $check, '-');
}

/**
* Generate an ID suitable for use in an ID attribute.
*
* @param string $value The value to convert into an ID.
* @return string The generated id.
*/
protected function _domId($value) {
$domId = mb_strtolower(Inflector::slug($value, '-'));
if (!empty($this->_idPrefix)) {
$domId = $this->_idPrefix . '-' . $domId;
}
return $domId;
}

}
3 changes: 3 additions & 0 deletions src/View/Widget/MultiCheckbox.php
View file
Open in desktop
Expand Up @@ -71,6 +71,7 @@ public function __construct($templates, $label) {
* - `disabled` Either a boolean or an array of checkboxes to disable.
* - `escape` Set to false to disable HTML escaping.
* - `options` An associative array of value=>labels to generate options for.
* - `idPrefix` Prefix for generated ID attributes.
*
* ### Options format
*
Expand Down Expand Up @@ -105,8 +106,10 @@ public function render(array $data) {
'options' => [],
'disabled' => null,
'val' => null,
'idPrefix' => null
];
$out = [];
$this->_idPrefix = $data['idPrefix'];
$this->_clearIds();
foreach ($data['options'] as $key => $val) {
$checkbox = [
Expand Down
3 changes: 3 additions & 0 deletions src/View/Widget/Radio.php
View file
Open in desktop
Expand Up @@ -75,6 +75,7 @@ public function __construct($templates, $label) {
* an array of attributes for all labels.
* - `required` - Set to true to add the required attribute
* on all generated radios.
* - `idPrefix` Prefix for generated ID attributes.
*
* @param array $data The data to build radio buttons with.
* @return string
Expand All @@ -88,6 +89,7 @@ public function render(array $data) {
'escape' => true,
'label' => true,
'empty' => false,
'idPrefix' => null
];
if ($data['options'] instanceof Traversable) {
$options = iterator_to_array($data['options']);
Expand All @@ -101,6 +103,7 @@ public function render(array $data) {
}
unset($data['empty']);

$this->_idPrefix = $data['idPrefix'];
$this->_clearIds();
$opts = [];
foreach ($options as $val => $text) {
Expand Down
136 changes: 136 additions & 0 deletions tests/TestCase/View/Helper/FormHelperTest.php
View file
Open in desktop
Expand Up @@ -2013,6 +2013,100 @@ public function testInputCustomization() {
$this->assertTags($result, $expected);
}

/**
* Test id prefix
*
* @return void
*/
public function testCreateIdPrefix() {
$this->Form->create(false, ['idPrefix' => 'prefix']);

$result = $this->Form->input('field');
$expected = [
'div' => ['class' => 'input text'],
'label' => ['for' => 'prefix-field'],
'Field',
'/label',
'input' => ['type' => 'text', 'name' => 'field', 'id' => 'prefix-field'],
'/div'
];
$this->assertTags($result, $expected);

$result = $this->Form->input('field', ['id' => 'custom-id']);
$expected = [
'div' => ['class' => 'input text'],
'label' => ['for' => 'custom-id'],
'Field',
'/label',
'input' => ['type' => 'text', 'name' => 'field', 'id' => 'custom-id'],
'/div'
];
$this->assertTags($result, $expected);

$result = $this->Form->radio('Model.field', ['option A']);
$expected = [
'input' => ['type' => 'hidden', 'name' => 'Model[field]', 'value' => ''],
['input' => [
'type' => 'radio',
'name' => 'Model[field]',
'value' => '0',
'id' => 'prefix-model-field-0'
]],
'label' => ['for' => 'prefix-model-field-0'],
'option A',
'/label'
];
$this->assertTags($result, $expected);

$result = $this->Form->radio('Model.field', ['option A', 'option']);
$expected = [
'input' => ['type' => 'hidden', 'name' => 'Model[field]', 'value' => ''],
['input' => [
'type' => 'radio',
'name' => 'Model[field]',
'value' => '0',
'id' => 'prefix-model-field-0'
]],
'label' => ['for' => 'prefix-model-field-0'],
'option A',
'/label'
];
$this->assertTags($result, $expected);

$result = $this->Form->select(
'Model.multi_field',
['first'],
['multiple' => 'checkbox']
);
$expected = [
'input' => [
'type' => 'hidden', 'name' => 'Model[multi_field]', 'value' => ''
],
['div' => ['class' => 'checkbox']],
['input' => [
'type' => 'checkbox', 'name' => 'Model[multi_field][]',
'value' => '0', 'id' => 'prefix-model-multi-field-0'
]],
['label' => ['for' => 'prefix-model-multi-field-0']],
'first',
'/label',
'/div',
];
$this->assertTags($result, $expected);

$this->Form->end();
$result = $this->Form->input('field');
$expected = [
'div' => ['class' => 'input text'],
'label' => ['for' => 'field'],
'Field',
'/label',
'input' => ['type' => 'text', 'name' => 'field', 'id' => 'field'],
'/div'
];
$this->assertTags($result, $expected);
}

/**
* Test that inputs with 0 can be created.
*
Expand Down Expand Up @@ -2128,6 +2222,48 @@ public function testInputDatetime() {
$this->assertTags($result, $expected);
}

/**
* test form->input() with datetime with id prefix
*
* @return void
*/
public function testInputDatetimeIdPrefix() {
$this->Form = $this->getMock(
'Cake\View\Helper\FormHelper',
['datetime'],
[new View(null)]
);

$this->Form->create(false, ['idPrefix' => 'prefix']);

$this->Form->expects($this->once())->method('datetime')
Copy link
Member

Choose a reason for hiding this comment

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

The datetime widget doesn't use the idPrefix option. Shoud it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Datetime widget doesn't use the IdGeneratorTrait itself.

I believe there was a discussion earlier to just skip id generation for datetime selects for now since we wouldn't know for which select the label's for attribute should be set if selects for all datetime parts are not generated.

Copy link
Member

Choose a reason for hiding this comment

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

Good point, that is the one remaining incomplete test for formhelper.

Copy link
Member Author

Choose a reason for hiding this comment

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

Anything else needs to be done before this can be merged? Datetime can be dealt with later on.

Is it ok to keep IdGeneratorTrait under Widget now that it's used for helper too?

Copy link
Member

Choose a reason for hiding this comment

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

We might want to move the trait to View/Helper.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

->with('prueba', [
'type' => 'datetime',
'timeFormat' => 24,
'minYear' => 2008,
'maxYear' => 2011,
'interval' => 15,
'options' => null,
'empty' => false,
'id' => 'prefix-prueba',
'required' => false,
])
->will($this->returnValue('This is it!'));
$result = $this->Form->input('prueba', array(
'type' => 'datetime', 'timeFormat' => 24, 'minYear' => 2008,
'maxYear' => 2011, 'interval' => 15
));
$expected = array(
'div' => array('class' => 'input datetime'),
'label' => array('for' => 'prefix-prueba'),
'Prueba',
'/label',
'This is it!',
'/div'
);
$this->assertTags($result, $expected);
}

/**
* Test generating checkboxes with disabled elements.
*
Expand Down