From 90787f6d7041e49bb25b711a9223384556b151e9 Mon Sep 17 00:00:00 2001 From: Kristijan Husak Date: Tue, 25 Aug 2015 16:32:25 +0200 Subject: [PATCH 1/2] Try to fix multiple select issue. --- src/Kris/LaravelFormBuilder/Fields/FormField.php | 8 ++++---- src/Kris/LaravelFormBuilder/Form.php | 4 ++-- tests/Fields/ChoiceTypeTest.php | 15 +++++++++++++++ 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/Kris/LaravelFormBuilder/Fields/FormField.php b/src/Kris/LaravelFormBuilder/Fields/FormField.php index 3467348b..5139933e 100644 --- a/src/Kris/LaravelFormBuilder/Fields/FormField.php +++ b/src/Kris/LaravelFormBuilder/Fields/FormField.php @@ -219,16 +219,16 @@ protected function prepareOptions(array $options = []) $this->template = array_pull($this->options, 'template'); } + if ($this->getOption('attr.multiple')) { + $this->name = $this->name.'[]'; + } + $options = $this->options = $helper->mergeOptions($this->options, $options); if ($this->parent->haveErrorsEnabled()) { $this->addErrorClass($options); } - if ($this->getOption('attr.multiple')) { - $this->name = $this->name.'[]'; - } - if ($this->getOption('required') === true) { $options['label_attr']['class'] .= ' ' . $this->formHelper ->getConfig('defaults.required_class', 'required'); diff --git a/src/Kris/LaravelFormBuilder/Form.php b/src/Kris/LaravelFormBuilder/Form.php index ba5292b4..93111c05 100644 --- a/src/Kris/LaravelFormBuilder/Form.php +++ b/src/Kris/LaravelFormBuilder/Form.php @@ -832,12 +832,12 @@ protected function checkIfNamedForm() */ protected function setupFieldOptions($name, &$options) { + $options['real_name'] = $name; + if (!$this->getName()) { return; } - $options['real_name'] = $name; - if (!isset($options['label'])) { $options['label'] = $this->formHelper->formatLabel($name); } diff --git a/tests/Fields/ChoiceTypeTest.php b/tests/Fields/ChoiceTypeTest.php index d67b76e0..4f579a77 100644 --- a/tests/Fields/ChoiceTypeTest.php +++ b/tests/Fields/ChoiceTypeTest.php @@ -71,4 +71,19 @@ public function it_creates_choice_as_radio_buttons() $this->assertContainsOnlyInstancesOf('Kris\LaravelFormBuilder\Fields\CheckableType', $choice->getChildren()); } + + /** @test */ + public function it_sets_proper_name_for_multiple() + { + $this->plainForm->add('users', 'select', [ + 'choices' => [1 => 'user1', 2 => 'user2'], + 'attr' => [ + 'multiple' => 'multple' + ] + ]); + + $this->plainForm->renderForm(); + + $this->assertEquals('users[]', $this->plainForm->users->getName()); + } } From cb01fc352c3c758f3cf8de4ee1b3a91198a5699d Mon Sep 17 00:00:00 2001 From: Kristijan Husak Date: Wed, 26 Aug 2015 11:21:25 +0200 Subject: [PATCH 2/2] Refactor options to be merged properly. --- .../LaravelFormBuilder/Fields/FormField.php | 81 +++++++++---------- tests/Fields/ButtonTypeTest.php | 10 ++- tests/Fields/InputTypeTest.php | 1 + 3 files changed, 49 insertions(+), 43 deletions(-) diff --git a/src/Kris/LaravelFormBuilder/Fields/FormField.php b/src/Kris/LaravelFormBuilder/Fields/FormField.php index 5139933e..eb1bec1c 100644 --- a/src/Kris/LaravelFormBuilder/Fields/FormField.php +++ b/src/Kris/LaravelFormBuilder/Fields/FormField.php @@ -121,6 +121,14 @@ protected function setupValue() */ abstract protected function getTemplate(); + /** + * @return string + */ + protected function getViewTemplate() + { + return $this->getOption('template', $this->template); + } + /** * @param array $options * @param bool $showLabel @@ -130,30 +138,20 @@ abstract protected function getTemplate(); */ public function render(array $options = [], $showLabel = true, $showField = true, $showError = true) { - $val = null; - $value = array_get($options, $this->valueProperty); - $defaultValue = array_get($options, $this->defaultValueProperty); + $this->prepareOptions($options); + $value = $this->getValue(); + $defaultValue = $this->getDefaultValue(); if ($showField) { $this->rendered = true; } - // Check if default value is passed to render function from view. - // If it is, we save it to a variable and then override it before - // rendering the view - if ($value) { - $val = $value; - } elseif ($defaultValue && !$this->getOption($this->valueProperty)) { - $val = $defaultValue; - } - - $options = $this->prepareOptions($options); - - if ($val) { - $options[$this->valueProperty] = $val; + // Override default value with value + if (!$value && $defaultValue) { + $this->setOption($this->valueProperty, $defaultValue); } - if (!$this->needsLabel($options)) { + if (!$this->needsLabel()) { $showLabel = false; } @@ -162,12 +160,12 @@ public function render(array $options = [], $showLabel = true, $showField = true } return $this->formHelper->getView()->make( - $this->template, + $this->getViewTemplate(), [ 'name' => $this->name, 'nameKey' => $this->getNameKey(), 'type' => $this->type, - 'options' => $options, + 'options' => $this->options, 'showLabel' => $showLabel, 'showField' => $showField, 'showError' => $showError @@ -214,46 +212,48 @@ protected function transformKey($key) protected function prepareOptions(array $options = []) { $helper = $this->formHelper; + $this->options = $helper->mergeOptions($this->options, $options); - if (array_get($this->options, 'template') !== null) { - $this->template = array_pull($this->options, 'template'); - } - - if ($this->getOption('attr.multiple')) { + if ($this->getOption('attr.multiple') && !$this->getOption('tmp.multipleBracesSet')) { $this->name = $this->name.'[]'; + $this->setOption('tmp.multipleBracesSet', true); } - $options = $this->options = $helper->mergeOptions($this->options, $options); - if ($this->parent->haveErrorsEnabled()) { $this->addErrorClass($options); } if ($this->getOption('required') === true) { - $options['label_attr']['class'] .= ' ' . $this->formHelper - ->getConfig('defaults.required_class', 'required'); - $options['attr']['required'] = 'required'; + $lblClass = $this->getOption('label_attr.class', ''); + $requiredClass = $helper->getConfig('defaults.required_class', 'required'); + if (!str_contains($lblClass, $requiredClass)) { + $lblClass .= ' ' . $requiredClass; + $this->setOption('label_attr.class', $lblClass); + $this->setOption('attr.required', 'required'); + } } if ($this->parent->clientValidationEnabled() && $rules = $this->getOption('rules')) { $rulesParser = new RulesParser($this); - $options['attr'] += $rulesParser->parse($rules); + $attrs = $this->getOption('attr') + $rulesParser->parse($rules); + $this->setOption('attr', $attrs); } - $options['wrapperAttrs'] = $helper->prepareAttributes($options['wrapper']); - $options['errorAttrs'] = $helper->prepareAttributes($options['errors']); + $this->setOption('wrapperAttrs', $helper->prepareAttributes($this->getOption('wrapper'))); + $this->setOption('errorAttrs', $helper->prepareAttributes($this->getOption('errors'))); - if ($options['is_child']) { - $options['labelAttrs'] = $helper->prepareAttributes($options['label_attr']); + if ($this->getOption('is_child')) { + $this->setOption('labelAttrs', $helper->prepareAttributes($this->getOption('label_attr'))); } - if ($options['help_block']['text']) { - $options['help_block']['helpBlockAttrs'] = $helper->prepareAttributes( - $options['help_block']['attr'] + if ($this->getOption('help_block.text')) { + $this->setOption( + 'help_block.helpBlockAttrs', + $helper->prepareAttributes($this->getOption('help_block.attr')) ); } - return $options; + return $this->options; } /** @@ -493,13 +493,12 @@ protected function setDefaultOptions(array $options = []) /** * Check if fields needs label * - * @param array $options * @return bool */ - protected function needsLabel(array $options = []) + protected function needsLabel() { // If field is