Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Feature/datetime factory format #3437

Merged
merged 6 commits into from Jan 16, 2013

Conversation

Projects
None yet
3 participants
Contributor

pdobrigkeit commented Jan 15, 2013

Added the setOptions function to the DateTime form element, which allows to set the accepted format, i.e. when using the array notation to define your elements and the factory.

@Maks3w Maks3w commented on an outdated diff Jan 15, 2013

library/Zend/Form/Element/DateTime.php
@@ -52,6 +52,24 @@ class DateTime extends Element implements InputProviderInterface
protected $validators;
/**
+ * Accepted options for DateTime:
+ * - format: A PHP date() compatible string
+ *
+ * @param array|\Traversable $options
+ * @return DateSelect
@Maks3w

Maks3w Jan 15, 2013

Member

DateTime

@Maks3w Maks3w and 1 other commented on an outdated diff Jan 15, 2013

library/Zend/Form/Element/DateTime.php
@@ -52,6 +52,24 @@ class DateTime extends Element implements InputProviderInterface
protected $validators;
/**
+ * Accepted options for DateTime:
+ * - format: A PHP date() compatible string
+ *
+ * @param array|\Traversable $options
+ * @return DateSelect
+ */
+ public function setOptions($options)
+ {
+ parent::setOptions($options);
+
+ if (isset($options['format'])) {
@Maks3w

Maks3w Jan 15, 2013

Member

$options can be array or Traversable accord to the docblock but this code only will work with arrays. It's necessary convert the iterator to array first (http://php.net/manual/en/function.iterator-to-array.php)

@pdobrigkeit

pdobrigkeit Jan 15, 2013

Contributor

This code block is copied directly from an already present form element (i.e. captcha). Then the same problem is present in those other form elements as well, but I will fix it here.

@pdobrigkeit pdobrigkeit Fixed usage of Traverseable for $options.
Did not want to duplicate the check done in Element, but rather used to already converted $this->options instead
c1bd400

@Ocramius Ocramius commented on the diff Jan 16, 2013

library/Zend/Form/Element/DateTime.php
@@ -52,6 +52,24 @@ class DateTime extends Element implements InputProviderInterface
protected $validators;
/**
+ * Accepted options for DateTime:
@Ocramius

Ocramius Jan 16, 2013

Member

Use {@inheritDoc} to inherit parent method documentation too

@Ocramius Ocramius commented on the diff Jan 16, 2013

library/Zend/Form/Element/DateTime.php
@@ -52,6 +52,24 @@ class DateTime extends Element implements InputProviderInterface
protected $validators;
/**
+ * Accepted options for DateTime:
+ * - format: A \DateTime compatible string
+ *
+ * @param array|\Traversable $options
+ * @return DateSelect
+ */
+ public function setOptions($options)
+ {
+ parent::setOptions($options);
+
+ if (isset($this->options['format'])) {
@Ocramius

Ocramius Jan 16, 2013

Member

Shouldn't this be if (isset($options['format'])) { ?

@Maks3w

Maks3w Jan 16, 2013

Member

$options can be a Traversable object but $this->options is always array.

Member

Ocramius commented Jan 16, 2013

@pdobrigkeit missing tests?

@Maks3w Maks3w commented on an outdated diff Jan 16, 2013

library/Zend/Form/Element/DateTime.php
@@ -52,6 +52,24 @@ class DateTime extends Element implements InputProviderInterface
protected $validators;
/**
+ * Accepted options for DateTime:
+ * - format: A \DateTime compatible string
+ *
+ * @param array|\Traversable $options
+ * @return DateSelect
@Maks3w

Maks3w Jan 16, 2013

Member

return type should be DateTime or Zend\Form\Element

Contributor

pdobrigkeit commented Jan 16, 2013

Added a test. But someone should check the other form elements for
a) the same $options can be Traversable
b) there are no tests either

Member

Maks3w commented Jan 16, 2013

That is tested in ZendTest\Form\ElementTest

Contributor

pdobrigkeit commented Jan 16, 2013

Take a look at for example Zend\Form\Element\Captcha. it also has its own setOptions and it uses $options['captcha']

@Maks3w Maks3w added a commit that referenced this pull request Jan 16, 2013

@Maks3w Maks3w Merge pull request #3437 from pdobrigkeit/feature/datetime_factory_fo…
…rmat

Feature/datetime factory format
89adefa

@Maks3w Maks3w merged commit 89adefa into zendframework:master Jan 16, 2013

1 check was pending

default The Travis build is in progress
Details

@Maks3w Maks3w added a commit that referenced this pull request Jan 16, 2013

@Maks3w Maks3w Forward #3437 64d00bb

@danizord danizord added a commit to danizord/zf2 that referenced this pull request Jan 16, 2013

@danizord danizord Fix #3437: Fixed usage of Traverseable for $options. 59313e8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment