Add Attributes helper

[aleksip]

Signed-off-by: Aleksi Peebles aleksi@iki.fi

Q	A
New Feature	yes

Description

A new helper to print out HTML tag attributes as discussed in the forum.

This is a minimal implementation, basically just moving existing methods from AbstractHtmlElement to a trait, used by a separate helper for more general use.

[froschdesign]

At the moment this is a very basic helper without any further functions. I missed translation and validation of attributes like in Laminas\Form\View\Helper\AbstractHelper.

[aleksip]

@froschdesign This could indeed be developed a bit further. I have some ideas I am working on, so marked this as draft for now.

[aleksip]

@froschdesign @weierophinney I have now added an AttributeStore class which I have been dogfooding successfully. This is still basic functionality for use cases where that is all that is needed. Validation and translation could be added, of course. Just checking at this point whether AttributeStore and the implementation are acceptable?

[froschdesign]

@aleksip
Thanks for your suggestion!

Some short comments:

• the duplication in the naming: AttributeStore::setAttributes() (see also in the forum)
• Can and should the new class AttributeStore be used for other things? Or are there other reasons why the functionality is extracted?

[aleksip]

@froschdesign

• The methods for single attributes could indeed be shortened to just set(), add() etc.! Should the methods for all attributes then be setAll() etc.?
• Unless I have misunderstood something there is only one instance of a specific helper per request? So a separate store class is useful when a template needs to process attributes for several tags. I thought this might be a more straightforward approach than managing several sets of attributes in the helper itself. AttributeStore could basically be a rather independent class, but if we want to make use of the helper plugin system, there needs to be at least an optional coupling.

[froschdesign]

So a separate store class is useful when a template needs to process attributes for several tags.

A simple array or PHP's ArrayObject can be used here.

I still think that a real added value would be the functions from the abstract view helper of laminas-form. Enhanced with the support of application wide configuration for custom attributes.

[aleksip]

My original implementation was array-based, but an $attributes->add('class', 'active') seems much cleaner to me than $attributes = $this->attributes->add($attributes, 'class', 'active'). Or are you thinking of some other way?

I can assure that just this simple functionality brings huge value for the style of templates that are common with Drupal, at least! But the functions from laminas-form would be a great addition too.

[froschdesign]

…an $attributes->add('class', 'active') seems much cleaner to me than $attributes = $this->attributes->add($attributes, 'class', 'active'). Or are you thinking of some other way?

No it is but the storing for multiple usage of the same attributes can be done via array or ArrayObject.

[aleksip]

Do you mean something like $this->attributes->add('someId', 'class', 'active') and then $this->attributes->toString('someId')?

I considered that style of approach but it does add extra logic to the helper, and does not seem as object-oriented as the store based approach. I also thought about the possible risk of ID collisions. Is this type of storage already implemented in some existing helper?

[froschdesign]

Do you mean something like $this->attributes->add('someId', 'class', 'active') and then $this->attributes->toString('someId')?

No no!

Instead of:

$attributes = new AttributeStore(['class' => '…', 'title' => '…']);
echo $this->htmlAttributes($attributes);
echo $this->htmlAttributes($attributes);

this:

$attributes = ['class' => '…', 'title' => '…'];
echo $this->htmlAttributes($attributes);
echo $this->htmlAttributes($attributes);

or:

$attributes = new ArrayObject(['class' => '…', 'title' => '…']);
echo $this->htmlAttributes($attributes);
echo $this->htmlAttributes($attributes);

No need for extra utility classes.

[aleksip]

That was basically the first version submitted in this PR. But dogfooding it I soon noticed that I am constantly repeating checks whether class is already set etc. in my templates.

So I find that the set(), add() and other methods in the utility class really help making the templates much more simple and the codebase more DRY.

Example:

$attributes = $attributes ?? [];
$attributes['class'] = (array)($attributes['class'] ?? []);
$attributes['class'][] = 'nav';
$attributes['class'][] = 'navbar-nav';
echo $this->attributes($attributes);

with AttributeStore:

$attributes = $this->attributes($attributes ?? []);
$attributes->addValue('class', ['nav', 'navbar-nav']);
echo $attributes;

There might be a more elegant way to do the first version, but the main point is that with the utility class this logic is in just one place, and additional features like checks can also be added at already the individual attribute stage, instead of only for the entire set in the end.

[froschdesign]

but the main point is that with the utility class this logic is in just one place

And my main point is: add this checks to the helper. No need for utility class. 😄

[aleksip]

I'm afraid I don't understand. In the examples you provided, the checking is done to the entire array? But it needs be done when adding attributes to the array. And if $attributes = $this->attributes->add($attributes, 'class', 'active') or $this->attributes->add('someId', 'class', 'active') are not the way to do it, what is?

[aleksip]

__construct() and setAttributes() allow setting the entire array without checks in the current version of AttributeStore. This would need to be fixed.

[froschdesign]

I have a few ideas, but I need some time for a short elaboration.

[aleksip]

Ok, so meanwhile I have made an attempt to improve the class-based approach. Attributes is now an independent class extending from ArrayObject. I have removed the trait and instead refactored AbstractHtmlElement to use the new Attributes class.

I am not sure if two different escaper variables are actually required, but chose to be on the safe side. AbstractHtmlElement::normalizeId() would logically also belong to Attributes, but I have not moved it there yet because it needs to be overriden in some cases I am not too familiar with.

Based on a quick look at laminas-form, it might be possible to just "drop in" Attributes to ElementInterface elements, since it extends from ArrayObject. And then as a next step perhaps further develop and/or extend Attributes so that each element could have an element-specific Attributes configuration etc... I mention this just as a possibility, since even in their current form Attributes and AttributesHelper can be extremely useful.

[weierophinney]

Looking good; I've suggested some improvements to make the various code paths more clear, and to reduce conditional nesting.

Ping me when you're ready for merge!

[aleksip]

@weierophinney Thanks! I have now applied your suggestions.

Just to be sure: the class names and packages are fine, and there should be two variables for the escaper?

This is still a very minimal implementation and many useful features could be added, including features from the abstract view helper of laminas-form. More features could also be added in a subclass, of course. If this were to be merged in the current state, would it still be easy to add non-BC-breaking features? Or would it be better to "dogfood" this for a while longer?

[weierophinney]

This is really looking great, and I hate to provide more changes, but it's all in the interest of polishing it up and getting it ready for release.

As noted below: this branch requires a minimum supported PHP version of 7.3, so new functionality can make full use of parameter and return typehints, including scalar hints and the iterable pseudo-type.

Finally: please create a document in the docs/book/helpers/ folder detailing the new helper and its usage; you can use one of the others in that directory as a template. When done, add it to the list in the mkdocs.yml file.

At that point, this will be ready to go!

[froschdesign]

A few small changes are required. The most important is the name of the helper.