Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Allow additions to view helper valid attributes #194

Merged
merged 6 commits into from
May 15, 2018

Conversation

Brindster
Copy link
Contributor

Currently, the list of valid attributes on the form view helpers is fixed. This pull request adds the ability to add additional valid attributes to view helpers.

My use case is that certain javascript frameworks use non standard attributes (such as ng-, v-) when binding on form elements. It would simplify development if Zend/Form were able to specify these attributes on form inputs.

@froschdesign
Copy link
Member

@Brindster

This pull request adds the ability to add additional valid attributes to view helpers.

Your commit includes something more:

/**
 * Attribute prefixes valid for all tags
 *
 * @var array
 */
protected $validTagAttributePrefixes = [
    'data-' => true,
    'aria-' => true,
    'x-'    => true,
];

https://github.com/Brindster/zend-form/blob/81f282411c9b202c2a483d310c2a81e14eb9bdd6/src/View/Helper/AbstractHelper.php#L164-L173

/**
 * Adds a prefix to the list of valid attribute prefixes
 *
 * @param string $prefix
 *
 * @return AbstractHelper
 */
public function addValidAttributePrefix($prefix)
{
    $this->validTagAttributePrefixes[$prefix] = true;
    return $this;
}

https://github.com/Brindster/zend-form/blob/81f282411c9b202c2a483d310c2a81e14eb9bdd6/src/View/Helper/AbstractHelper.php#L475-L486

Please don't mix different topics in one commit. That would help a lot!

*/
public function addValidAttribute($attribute)
{
$this->validTagAttributes[$attribute] = true;
Copy link
Member

Choose a reason for hiding this comment

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

No validation of the given parameter?

*/
public function addValidAttributePrefix($prefix)
{
$this->validTagAttributePrefixes[$prefix] = true;
Copy link
Member

Choose a reason for hiding this comment

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

No validation of the given parameter? (This should be an extra commit.)

@@ -60,6 +60,26 @@ public function testWillNotEncodeValueAttributeValuesCorrectly()
);
}

public function testWillIncludeAdditionalAttributes()
Copy link
Member

Choose a reason for hiding this comment

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

This test covers only the optimal case. The problematic data types are not tested. (see my comments above)

);
}

public function testWillIncludeAdditionalAttributesByPrefix()
Copy link
Member

Choose a reason for hiding this comment

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

Same here. (This should be an extra commit.)

@froschdesign
Copy link
Member

For the moment I added the label "work in progress". I have to look for side effects.

@froschdesign
Copy link
Member

Related to #50

@Brindster
Copy link
Contributor Author

Apologies for the mixed initial commit, I'll try and be more careful in future. Validation and non-optimal test cases have been added.

@weierophinney weierophinney merged commit a13331e into zendframework:develop May 15, 2018
weierophinney added a commit that referenced this pull request May 15, 2018
Allow additions to view helper valid attributes

Conflicts:
	src/View/Helper/AbstractHelper.php
weierophinney added a commit that referenced this pull request May 15, 2018
- Updates docblocks of changed files to match new standards.
- Updates docblocks of new properties, methods to match formatting of
  others. Adds `@throws` annotations to methods throwing exceptions.
- Adds labels to each datum in a data provider.
weierophinney added a commit that referenced this pull request May 15, 2018
weierophinney added a commit that referenced this pull request May 15, 2018
@weierophinney
Copy link
Member

Thanks, @Brindster!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants