merged branch helmer/readonly (PR #3193)

Commits ------- de253dd [Form] read_only and disabled attributes Discussion ---------- [Form] read_only and disabled attributes (closes #1974) 1. Removed ``readOnly`` property from ``Form``, as it is no longer required 2. Introduced ``disabled`` property to ``Form``, behaves exactly like ``readOnly`` used to 3. Added ``disabled`` property to fields, defaults to ``false``, renders as ``disabled="disabled"`` 4. A field with positive ``read_only`` property now renders as ``readonly="readonly"`` --------------------------------------------------------------------------- by helmer at 2012-01-26T17:46:17Z I changed ``Form`` and ``FormBuilder`` property ``readOnly`` to ``disabled``. On second thought, this is perhaps not such good change - while readOnly somewhat implied the use-case, disabled no longer does. Perhaps something else, like ``bindable`` (as not to confuse with read_only attribute of Fields)? @bschussek, others, any thoughts? --------------------------------------------------------------------------- by bschussek at 2012-01-31T06:53:59Z Please prefix commits with the affected component, if applicable. --------------------------------------------------------------------------- by helmer at 2012-01-31T08:41:03Z @bschussek Prefixed. Please also see see to [this question](https://github.com/symfony/symfony/pull/3193#issuecomment-3673074)
2012-02-02 10:03:00 +01:00 · 2012-02-02 10:03:00 +01:00 · e71d1579d1
commit e71d1579d1
parent 253a8ff016 de253dd3dd
11 changed files with 86 additions and 64 deletions
--- a/CHANGELOG-2.1.md
+++ b/CHANGELOG-2.1.md
@ -158,6 +158,7 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c

 ### Form

+ * [BC BREAK] ``read_only`` field attribute now renders as ``readonly="readonly"``, use ``disabled`` instead
 * [BC BREAK] child forms now aren't validated anymore by default
 * made validation of form children configurable (new option: cascade_validation)
 * added support for validation groups as callbacks
--- a/src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig
+++ b/src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig
@ -280,7 +280,7 @@

 {% block widget_attributes %}
 {% spaceless %}
-    id="{{ id }}" name="{{ full_name }}"{% if read_only %} disabled="disabled"{% endif %}{% if required %} required="required"{% endif %}{% if max_length %} maxlength="{{ max_length }}"{% endif %}{% if pattern %} pattern="{{ pattern }}"{% endif %}
+    id="{{ id }}" name="{{ full_name }}"{% if read_only %} readonly="readonly"{% endif %}{% if disabled %} disabled="disabled"{% endif %}{% if required %} required="required"{% endif %}{% if max_length %} maxlength="{{ max_length }}"{% endif %}{% if pattern %} pattern="{{ pattern }}"{% endif %}
    {% for attrname,attrvalue in attr %}{{attrname}}="{{attrvalue}}" {% endfor %}
 {% endspaceless %}
 {% endblock widget_attributes %}
--- a/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/attributes.html.php
+++ b/src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/attributes.html.php
@ -1,6 +1,7 @@
 id="<?php echo $view->escape($id) ?>"
 name="<?php echo $view->escape($full_name) ?>"
-<?php if ($read_only): ?>disabled="disabled" <?php endif ?>
+<?php if ($read_only): ?>readonly="readonly" <?php endif ?>
+<?php if ($disabled): ?>disabled="disabled" <?php endif ?>
 <?php if ($required): ?>required="required" <?php endif ?>
 <?php if ($max_length): ?>maxlength="<?php echo $view->escape($max_length) ?>" <?php endif ?>
 <?php if ($pattern): ?>pattern="<?php echo $view->escape($pattern) ?>" <?php endif ?>
--- a/src/Symfony/Component/Form/Extension/Core/Type/FieldType.php
+++ b/src/Symfony/Component/Form/Extension/Core/Type/FieldType.php
@ -44,9 +44,10 @@ class FieldType extends AbstractType

        $builder
            ->setRequired($options['required'])
-            ->setReadOnly($options['read_only'])
+            ->setDisabled($options['disabled'])
            ->setErrorBubbling($options['error_bubbling'])
            ->setEmptyData($options['empty_data'])
+            ->setAttribute('read_only', $options['read_only'])
            ->setAttribute('by_reference', $options['by_reference'])
            ->setAttribute('property_path', $options['property_path'])
            ->setAttribute('error_mapping', $options['error_mapping'])
@ -107,7 +108,8 @@ class FieldType extends AbstractType
            ->set('full_name', $fullName)
            ->set('errors', $form->getErrors())
            ->set('value', $form->getClientData())
-            ->set('read_only', $form->isReadOnly())
+            ->set('read_only', $form->getAttribute('read_only'))
+            ->set('disabled', $form->isDisabled())
            ->set('required', $form->isRequired())
            ->set('max_length', $form->getAttribute('max_length'))
            ->set('pattern', $form->getAttribute('pattern'))
@ -131,6 +133,7 @@ class FieldType extends AbstractType
            'trim'              => true,
            'required'          => true,
            'read_only'         => false,
+            'disabled'          => false,
            'max_length'        => null,
            'pattern'           => null,
            'property_path'     => null,
--- a/src/Symfony/Component/Form/Form.php
+++ b/src/Symfony/Component/Form/Form.php
@ -166,7 +166,7 @@ class Form implements \IteratorAggregate, FormInterface
     * Whether this form may only be read, but not bound
     * @var Boolean
     */
-    private $readOnly = false;
+    private $disabled = false;

    /**
     * The dispatcher for distributing events of this form
@ -190,7 +190,7 @@ class Form implements \IteratorAggregate, FormInterface
        array $types = array(), array $clientTransformers = array(),
        array $normTransformers = array(),
        DataMapperInterface $dataMapper = null, array $validators = array(),
-        $required = false, $readOnly = false, $errorBubbling = false,
+        $required = false, $disabled = false, $errorBubbling = false,
        $emptyData = null, array $attributes = array())
    {
        $name = (string) $name;
@ -223,7 +223,7 @@ class Form implements \IteratorAggregate, FormInterface
        $this->dataMapper = $dataMapper;
        $this->validators = $validators;
        $this->required = (Boolean) $required;
-        $this->readOnly = (Boolean) $readOnly;
+        $this->disabled = (Boolean) $disabled;
        $this->errorBubbling = (Boolean) $errorBubbling;
        $this->emptyData = $emptyData;
        $this->attributes = $attributes;
@ -270,7 +270,6 @@ class Form implements \IteratorAggregate, FormInterface
    public function isRequired()
    {
        if (null === $this->parent || $this->parent->isRequired()) {
-
            return $this->required;
        }

@ -278,21 +277,12 @@ class Form implements \IteratorAggregate, FormInterface
    }

    /**
-     * Returns whether this form is read only.
-     *
-     * The content of a read-only form is displayed, but not allowed to be
-     * modified. The validation of modified read-only forms should fail.
-     *
-     * Fields whose parents are read-only are considered read-only regardless of
-     * their own state.
-     *
-     * @return Boolean
+     * {@inheritDoc}
     */
-    public function isReadOnly()
+    public function isDisabled()
    {
-        if (null === $this->parent || !$this->parent->isReadOnly()) {
-
-            return $this->readOnly;
+        if (null === $this->parent || !$this->parent->isDisabled()) {
+            return $this->disabled;
        }

        return true;
@ -461,7 +451,7 @@ class Form implements \IteratorAggregate, FormInterface
     */
    public function bind($clientData)
    {
-        if ($this->readOnly) {
+        if ($this->isDisabled()) {
            $this->bound = true;

            return $this;
@ -678,7 +668,6 @@ class Form implements \IteratorAggregate, FormInterface
    {
        foreach ($this->children as $child) {
            if (!$child->isEmpty()) {
-
                return false;
            }
        }
@ -701,10 +690,9 @@ class Form implements \IteratorAggregate, FormInterface
            return false;
        }

-        if (!$this->readOnly) {
+        if (!$this->isDisabled()) {
            foreach ($this->children as $child) {
                if (!$child->isValid()) {
-
                    return false;
                }
            }
@ -879,7 +867,6 @@ class Form implements \IteratorAggregate, FormInterface
    public function get($name)
    {
        if (isset($this->children[$name])) {
-
            return $this->children[$name];
        }

--- a/src/Symfony/Component/Form/FormBuilder.php
+++ b/src/Symfony/Component/Form/FormBuilder.php
@ -46,7 +46,7 @@ class FormBuilder
    /**
     * @var Boolean
     */
-    private $readOnly;
+    private $disabled;

    /**
     * @var Boolean
@ -178,27 +178,27 @@ class FormBuilder
    }

    /**
-     * Set whether the form is read only
+     * Set whether the form is disabled
     *
-     * @param Boolean $readOnly Whether the form is read only
+     * @param Boolean $disabled Whether the form is disabled
     *
     * @return FormBuilder The current builder
     */
-    public function setReadOnly($readOnly)
+    public function setDisabled($disabled)
    {
-        $this->readOnly = (Boolean) $readOnly;
+        $this->disabled = (Boolean) $disabled;

        return $this;
    }

    /**
-     * Returns whether the form is read only.
+     * Returns whether the form is disabled.
     *
-     * @return Boolean Whether the form is read only
+     * @return Boolean Whether the form is disabled
     */
-    public function getReadOnly()
+    public function getDisabled()
    {
-        return $this->readOnly;
+        return $this->disabled;
    }

    /**
@ -649,7 +649,7 @@ class FormBuilder
            $this->getDataMapper(),
            $this->getValidators(),
            $this->getRequired(),
-            $this->getReadOnly(),
+            $this->getDisabled(),
            $this->getErrorBubbling(),
            $this->getEmptyData(),
            $this->getAttributes()
--- a/src/Symfony/Component/Form/FormInterface.php
+++ b/src/Symfony/Component/Form/FormInterface.php
@ -178,17 +178,17 @@ interface FormInterface extends \ArrayAccess, \Traversable, \Countable
    function isRequired();

    /**
-     * Returns whether this form can be read only.
+     * Returns whether this form is disabled.
     *
-     * The content of a read-only form is displayed, but not allowed to be
-     * modified. The validation of modified read-only forms should fail.
+     * The content of a disabled form is displayed, but not allowed to be
+     * modified. The validation of modified disabled forms should fail.
     *
-     * Fields whose parents are read-only are considered read-only regardless of
+     * Fields whose parents are disabled are considered disabled regardless of
     * their own state.
     *
     * @return Boolean
     */
-    function isReadOnly();
+    function isDisabled();

    /**
     * Returns whether the form is empty.
--- a/tests/Symfony/Tests/Component/Form/AbstractLayoutTest.php
+++ b/tests/Symfony/Tests/Component/Form/AbstractLayoutTest.php
@ -1231,6 +1231,36 @@ abstract class AbstractLayoutTest extends \PHPUnit_Framework_TestCase
        );
    }

+    public function testReadOnly()
+    {
+        $form = $this->factory->createNamed('text', 'name', null, array(
+            'read_only' => true,
+        ));
+
+        $this->assertWidgetMatchesXpath($form->createView(), array(),
+'/input
+    [@type="text"]
+    [@name="name"]
+    [@readonly="readonly"]
+'
+        );
+    }
+
+    public function testDisabled()
+    {
+        $form = $this->factory->createNamed('text', 'name', null, array(
+            'disabled' => true,
+        ));
+
+        $this->assertWidgetMatchesXpath($form->createView(), array(),
+'/input
+    [@type="text"]
+    [@name="name"]
+    [@disabled="disabled"]
+'
+        );
+    }
+
    public function testInteger()
    {
        $form = $this->factory->createNamed('integer', 'name', 123);
--- a/tests/Symfony/Tests/Component/Form/Extension/Core/Type/FieldTypeTest.php
+++ b/tests/Symfony/Tests/Component/Form/Extension/Core/Type/FieldTypeTest.php
@ -70,11 +70,11 @@ class FieldTypeTest extends TypeTestCase
        $this->assertTrue($form->isRequired());
    }

-    public function testPassReadOnlyAsOption()
+    public function testPassDisabledAsOption()
    {
-        $form = $this->factory->create('field', null, array('read_only' => true));
+        $form = $this->factory->create('field', null, array('disabled' => true));

-        $this->assertTrue($form->isReadOnly());
+        $this->assertTrue($form->isDisabled());
    }

    public function testBoundDataIsTrimmedBeforeTransforming()
--- a/tests/Symfony/Tests/Component/Form/FormFactoryTest.php
+++ b/tests/Symfony/Tests/Component/Form/FormFactoryTest.php
@ -492,7 +492,7 @@ class FormFactoryTest extends \PHPUnit_Framework_TestCase

        $this->setExpectedException('Symfony\Component\Form\Exception\CreationException',
            'The options "invalid", "unknown" do not exist. Known options are: "data", "data_class", ' .
-            '"trim", "required", "read_only", "max_length", "pattern", "property_path", "by_reference", ' .
+            '"trim", "required", "read_only", "disabled", "max_length", "pattern", "property_path", "by_reference", ' .
            '"error_bubbling", "error_mapping", "label", "attr", "invalid_message", "invalid_message_parameters", ' .
            '"translation_domain", "empty_data"'
        );
@ -507,7 +507,7 @@ class FormFactoryTest extends \PHPUnit_Framework_TestCase

        $this->setExpectedException('Symfony\Component\Form\Exception\CreationException',
            'The option "unknown" does not exist. Known options are: "data", "data_class", ' .
-            '"trim", "required", "read_only", "max_length", "pattern", "property_path", "by_reference", ' .
+            '"trim", "required", "read_only", "disabled", "max_length", "pattern", "property_path", "by_reference", ' .
            '"error_bubbling", "error_mapping", "label", "attr", "invalid_message", "invalid_message_parameters", ' .
            '"translation_domain", "empty_data"'
        );
--- a/tests/Symfony/Tests/Component/Form/FormTest.php
+++ b/tests/Symfony/Tests/Component/Form/FormTest.php
@ -212,10 +212,10 @@ class FormTest extends \PHPUnit_Framework_TestCase
        $this->form->bind(array());
    }

-    public function testBindIsIgnoredIfReadOnly()
+    public function testBindIsIgnoredIfDisabled()
    {
        $form = $this->getBuilder()
-            ->setReadOnly(true)
+            ->setDisabled(true)
            ->setData('initial')
            ->getForm();

@ -255,34 +255,34 @@ class FormTest extends \PHPUnit_Framework_TestCase
        $this->assertFalse($child->isRequired());
    }

-    public function testAlwaysReadOnlyIfParentReadOnly()
+    public function testAlwaysDisabledIfParentDisabled()
    {
-        $parent = $this->getBuilder()->setReadOnly(true)->getForm();
-        $child = $this->getBuilder()->setReadOnly(false)->getForm();
+        $parent = $this->getBuilder()->setDisabled(true)->getForm();
+        $child = $this->getBuilder()->setDisabled(false)->getForm();

        $child->setParent($parent);

-        $this->assertTrue($child->isReadOnly());
+        $this->assertTrue($child->isDisabled());
    }

-    public function testReadOnly()
+    public function testDisabled()
    {
-        $parent = $this->getBuilder()->setReadOnly(false)->getForm();
-        $child = $this->getBuilder()->setReadOnly(true)->getForm();
+        $parent = $this->getBuilder()->setDisabled(false)->getForm();
+        $child = $this->getBuilder()->setDisabled(true)->getForm();

        $child->setParent($parent);

-        $this->assertTrue($child->isReadOnly());
+        $this->assertTrue($child->isDisabled());
    }

-    public function testNotReadOnly()
+    public function testNotDisabled()
    {
-        $parent = $this->getBuilder()->setReadOnly(false)->getForm();
-        $child = $this->getBuilder()->setReadOnly(false)->getForm();
+        $parent = $this->getBuilder()->setDisabled(false)->getForm();
+        $child = $this->getBuilder()->setDisabled(false)->getForm();

        $child->setParent($parent);

-        $this->assertFalse($child->isReadOnly());
+        $this->assertFalse($child->isDisabled());
    }

    public function testCloneChildren()
@ -361,15 +361,15 @@ class FormTest extends \PHPUnit_Framework_TestCase
        $this->assertTrue($this->form->isValid());
    }

-    public function testValidIfBoundAndReadOnly()
+    public function testValidIfBoundAndDisabled()
    {
-        $form = $this->getBuilder()->setReadOnly(true)->getForm();
+        $form = $this->getBuilder()->setDisabled(true)->getForm();
        $form->bind('foobar');

        $this->assertTrue($form->isValid());
    }

-    public function testValidIfBoundAndReadOnlyWithChildren()
+    public function testValidIfBoundAndDisabledWithChildren()
    {
        $this->factory->expects($this->once())
            ->method('createNamedBuilder')
@ -377,7 +377,7 @@ class FormTest extends \PHPUnit_Framework_TestCase
            ->will($this->returnValue($this->getBuilder('name')));

        $form = $this->getBuilder('person')
-            ->setReadOnly(true)
+            ->setDisabled(true)
            ->add('name', 'text')
            ->getForm();
        $form->bind(array('name' => 'Jacques Doe'));