Add Form\Element labelOptions property w/ implemented use case

[stefanotorresi]

Having a labelOptions array of dedicated properties for the form labels makes custom rendering of the form a bit easier.

One use case is the possibilty to disable HTML escaping of the label text.

A common use case for this is an hyper link inside the label, which pretty much any html form with a 'privacy terms' checkbox needs.

Since Form\View\Helper\FormRow escapes the label content, the only way of doing this at the moment is using custom view templates using a wide range of form view helpers.

That denies the purpose of Form\View\Helper\Form and Form\View\Helper\FormRow.

This PR adds labelOptions property and accessors to the Form\Element class, and handles the disable_html_escape option in Form\View\Helper\FormRow.

I reckon that adding an array to the Form\Element class just to handle a view helper option may seem a bit overkill in this case, but I'm looking from a wider angle: being able to tie custom options to the form element label itself gives much more flexibility when developing custom view templates and helpers; the example above is just the simplest I had to deal with.

[EvanDotPro]

Well, I'm not 100% sure on that use-case, but as far as the tests go, I'd say you should leave testSpecificOptionsSetLabelAttributes() as-is, and just add another test, instead of modifying the existing one. If you have to modify an existing test when adding functionality, that makes it look like there could be a BC break from a casual review perspective, even if that's not necessarily the case.

[EvanDotPro]

Maybe put an @TODO to clean up this duck typing for 3.0 when we can break BC.

[localheinz]

PhpStorm user, eh?

Indentation is messed up.

[stefanotorresi]

Hmmm I replicated the same indentation of the previous method. I thought I had to line the array keys to the opening parenthesis above. Should it be just one level instead?

[edit] np, looking at the other methods answered the question. Not PhpStorm's fault btw :-P

[mwillbanks]

Instead of a method exists; it is far preferable to provide an additional interface. This is done in several areas of the framework to provide additional behavior. This way when an element implements the behavior you can check to see if it is an instance of rather than doing an internal hash table lookup on the object itself.

IMO this makes it easier to extend the framework further as you can see even in the ModuleManager which provides interfaces for certain extension points.

[weierophinney]

There's a BC problem with this, and not what you would expect: PHP 5.3.3 does not allow implementing multiple interfaces that define the same method, even if the signatures are the same. This means that setLabel() and getLabel() cannot be in the LabelAwareInterface. As such, on merge, I'm going to rename it to LabelOptionsAwareInterface and will remove those methods.

[chludwig]

Thanks for the update, but I am missing this option for button elements. Buttons are rendered in a different way.

See line 97 in FormButton.php

return $openTag . $escape($buttonContent) . $this->closeTag();

[franz-deleon]

i have a problem with this change and it breaks a lot of my forms and i assume most people will too.
I cannot successfully turn this switch off/on (disable_html_escape)

//inside form
    public function init()
    {
        $this->setAttribute('method', 'post');
        $this->setLabelOptions(array(
            'disable_html_escape' => true,
        ));

        $this->add(array(
            'type' => 'Text',
            'name' => 'displayname',
            'options' => array(
                'label' => 'Display Name:<span>*</span>',
            ),
            'attributes' => array(
                'size' => 70,
                'id' => 'name',
            ),
            'label_options' => array(
                'disable_html_escape' => true,
            ),
        ));
}

[stefanotorresi]

label_options is still an element option, so the correct spec is this:

$form->add(array(
    'type' => 'Text',
    'name' => 'displayname',
    'options' => array(
        'label' => 'Display Name:<span>*</span>',
        'label_options' => array(
            'disable_html_escape' => true,
        ),
    ),
    'attributes' => array(
        'size' => 70,
        'id' => 'name',
    ),
));

Now that I notice it, Form\Factory::configureElement() may use a little tweak to allow the notation you tried, which is certainly more intuitive.

By the way, if your existent forms were affected or not by the change depends on what view helpers you are using, because rather than this PR, the breaking change you may refer to has been introduced in #5101, involving the FormLabel view helper and not the Form and FormRow ones, which behaviour has not been changed (they escaped the labels already, this PR only added the opt-out option).

[franz-deleon]

ahh i see now that is how to implement this. i have to change a bunch of forms when this change occurs

[stefanotorresi]

No. ;)
´FormRow´, ´FormButton´ and ´FormLabel´ are the only helpers involved with the element label. ´FormText´ renders just the input. That can let me assume that you are also using ´FormLabel´ to render the label, which is indeed the one with the breaking change.

[franz-deleon]

yes i stand corrected thats why i deleted that comment :D . and yes indeed i am using formLabel for rendering,