Emagister / zend-form-decorators-bootstrap

Zend_Form decorators for Twitter's Bootstrap UI
http://emagister.github.com/zend-form-decorators-bootstrap
168 stars 79 forks source link

Call parent constructor after the element decorators have been set #109

Open jimmykane opened 10 years ago

jimmykane commented 10 years ago

This fixes issue #102

jimmykane commented 10 years ago

To be more specific the issue realates to how Zend_Form is using setElementDecorators

 /**
     * Set all element decorators as specified
     *
     * @param  array $decorators
     * @param  array|null $elements Specific elements to decorate or exclude from decoration
     * @param  bool $include Whether $elements is an inclusion or exclusion list
     * @return Zend_Form
     */
    public function setElementDecorators(array $decorators, array $elements = null, $include = true)
    {
        if (is_array($elements)) {
            if ($include) {
                $elementObjs = array();
                foreach ($elements as $name) {
                    if (null !== ($element = $this->getElement($name))) {
                        $elementObjs[] = $element;
                    }
                }
            } else {
                $elementObjs = $this->getElements();
                foreach ($elements as $name) {
                    if (array_key_exists($name, $elementObjs)) {
                        unset($elementObjs[$name]);
                    }
                }
            }
        } else {
            $elementObjs = $this->getElements();
        }

        foreach ($elementObjs as $element) {
            $element->setDecorators($decorators);
        }

        $this->_elementDecorators = $decorators;

        return $this;
    }

Basically if you notice this has a double purpose.

jimmykane commented 10 years ago

Anyone there on this ?

jimmykane commented 10 years ago

It would be nice if it's merged because we are using ZFTB3 as an dependency installed via composer.

jakejohns commented 10 years ago

this does appear to fix the issue. seems like it should be pulled unless theres some other implication I'm missing.

jimmykane commented 10 years ago

Sorry wrong reading....

twisty commented 10 years ago

Hi, does anyone at @Emagister have an opinion on this PR?

theUniC commented 10 years ago

Ping @eberhm

Can you merge the PR?