-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Form] deprecate read_only option #14403
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
f1ea295
to
83db940
Compare
6d6edaa
to
40f4440
Compare
@symfony/deciders ready |
|
||
$readOnlyNormalizer = function (Options $options, $readOnly) { | ||
if (null !== $readOnly) { | ||
trigger_error('The form option "read_only" is deprecated since version 2.7 and will be removed in 3.0. Use "attr[\'readonly\']" instead.', E_USER_DEPRECATED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Must be 2.8?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch
Shouldn't we bump the dev dependency for the Form component in the FrameworkBundle to |
yes |
Fixed |
Looks good! 👍 |
Thank you @Tobion. |
This PR was squashed before being merged into the 2.8 branch (closes #14403). Discussion ---------- [Form] deprecate read_only option | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #10658 | License | MIT | Doc PR | symfony/symfony-docs#3782 Replaces #10676 with a slightly different implementation. - fixes BC break when 'read_only' => true option and at the same time custom attributes are used by using a normalizer - adds deprecation notice - keeps legacy tests Commits ------- 53330e1 [Form] deprecate read_only option
)); | ||
|
||
$html = $this->renderWidget($form->createView()); | ||
|
||
// compare plain HTML to check the whitespace | ||
$this->assertSame('<input type="text" id="text" name="text" readonly="readonly" disabled="disabled" required="required" maxlength="10" pattern="\d+" class="foobar" data-foo="bar" value="value" />', $html); | ||
$this->assertSame('<input type="text" id="text" name="text" disabled="disabled" required="required" readonly="readonly" maxlength="10" pattern="\d+" class="foobar" data-foo="bar" value="value" />', $html); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand, this doesn't preserve BC here? The test was changed to support the new way (through attributes), but the old way is not tested anymore.
This created a BC break for us when migrating from 2.7 to 2.8. We have to switch from the read_only
variable to the attr
key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mnapoli what exactly is breaking except the position in the string ? One would just expect to get the attribute here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before (2.7) passing read_only
as a variable was working, after (2.8) it isn't. We had to put readonly
in the attr
array instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the change we had to do:
{% block _form_category_name_widget %}
- {% set read_only = true %}
{% set attr = {
'data-target-show': '.choose-categories',
+ 'readonly': true,
} %}
{{ block('form_widget_simple') }}
{% endblock %}
This PR was merged into the 2.8 branch. Discussion ---------- [Form] Fix BC break introduced in #14403 | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | - Commits ------- 5f48c6a [Form] Fix BC break introduced in #14403
* 2.8: improved comment [FileSystem] Add support for Google App Engine. [2.8] fix mocks [Form] Fix BC break introduced in #14403
* 3.0: bumped Symfony version to 2.8.5 updated VERSION for 2.8.4 updated CHANGELOG for 2.8.4 Revert "bug #18275 [Form] Fix BC break introduced in #14403 (HeahDude)" improved comment [FileSystem] Add support for Google App Engine. [2.8] fix mocks bumped Symfony version to 2.7.12 [Form] Fix BC break introduced in #14403 updated VERSION for 2.7.11 updated CHANGELOG for 2.7.11 [Request] Fix support of custom mime types with parameters fix mocks fix mocks [Validator] do not treat payload as callback
Replaces #10676 with a slightly different implementation.