[TwigBridge] Set Form ID on form element, prevent duplicate form element attributes #48013

ker0x · Oct 27, 2022

Q	A
Branch?	7.1
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	Fix #42075
License	MIT
Doc PR	-

As explained in #42075, when a form is rendered, its ID is displayed in the child div of the <form> element.

This prevents to use the form_attr option on FormType.

Moreover, attributes defined on <form> element are duplicated in the child div

For example, the following form

return $this
    ->createFormBuilder(null, [
        'attr' => [
            'class' => 'foo-bar',
            'data-foo' => 'bar'
        ],
    ])
    ->add('input', TextType::class)
    // ...
;

has the following HTML rendering

<form name="form" method="post" class="foo-bar" data-foo="bar">
    <div id="form" class="foo-bar" data-model="form">
         // ... fields
    </div>
</form>

This PR provides the following fixes

A new computed variable form_id is available in the view. It is the concatenation of form_ and the ID of the form.
The <form> element uses the row_attr option instead of attr to display attributes. The row_attr option was never used on the root form and this avoids duplicating attributes.
The form_attr option now only accepts a boolean value.
- If set on the root form, each child will have a form attribute with the form_id as value.
- If set on a child, it will have a form attribute with the form_id as value.

src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig

ker0x · Nov 2, 2022

I just updated the PR with the following changes:

A new computed variable form_id is available in the view. It is the concatenation of form_ and the ID of the form.
The <form> element uses the row_attr option instead of attr to display attributes. The row_attr option was never used on the root form and this avoids duplicating attributes.
The form_attr option now only accepts a boolean value.
- If set on the root form, each child will have a form attribute with the form_id as value.
- If set on a child, it will have a form attribute with the form_id as value.

nicolas-grekas · Apr 17, 2023

Looks like a rebase is needed. Still up to do it?

ker0x · Apr 18, 2023

@nicolas-grekas PR has been rebase, however unit tests are failing with high and low deps and I don't know why!

xabbuh · Apr 21, 2023

looks like you need to update the composer.json file of the Twig bridge to make sure that the Form component is not older than 6.3 (in the require-dev and conflict sections)

nicolas-grekas · Jul 13, 2023

I pushed the fix to the composer.json.
Can you please update the description of the PR? I feel like it's a bit out of sync.

HeahDude

Since this issue has been there for a long time already, maybe this should target 6.4 to prevent side effects in the LTS.

ker0x · Jul 26, 2023

@nicolas-grekas description update 👍

@HeahDude I agree to target 6.4 instead of 5.4

nicolas-grekas · Jul 26, 2023

target 6.4 instead of 5.4

Would be better I agree. Can you update the target of this PR and rebase on 6.4 please?

xabbuh

you will need to update the version constraints similarly to what Nicolas did in 3f3d763#diff-33dca46fccec7efe765aa6cf249fddcdde0d15fa2d01adc052a5e1da01b423c5

src/Symfony/Bridge/Twig/composer.json

xabbuh · Jul 26, 2023

src/Symfony/Bridge/Twig/Resources/views/Form/bootstrap_3_horizontal_layout.html.twig

@@ -1,7 +1,7 @@
 {% use "bootstrap_3_layout.html.twig" %}

 {% block form_start -%}
-    {% set attr = attr|merge({class: (attr.class|default('') ~ ' form-horizontal')|trim}) %}
+    {% set row_attr = row_attr|merge({class: (row_attr.class|default('') ~ ' form-horizontal')|trim}) %}


If we have to make this change here, it seems to me that we need to adapt for custom form types that do not provide the row_attr variable (and that we need to fall back to attr then).

…ent attributes

ker0x requested review from xabbuh and yceruto as code owners October 27, 2022 13:35

carsonbot added Status: Needs Review Bug TwigBridge labels Oct 27, 2022

carsonbot added this to the 6.2 milestone Oct 27, 2022

ker0x changed the base branch from 6.2 to 5.4 October 27, 2022 13:36

ker0x mentioned this pull request Oct 27, 2022

[Form] Form ID is not set on form element #42075

Closed

ker0x commented Oct 27, 2022

View reviewed changes

src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig Outdated Show resolved Hide resolved

ker0x changed the title ~~[TwigBridge] Set Form ID on form element, prevent duplicate form elem…~~ [TwigBridge] Set Form ID on form element, prevent duplicate form element attributes Oct 27, 2022

fabpot modified the milestones: 6.2, 5.4 Nov 1, 2022

ker0x force-pushed the bugfix/form-id branch from 1d5884d to 4da3951 Compare November 2, 2022 11:25

ker0x force-pushed the bugfix/form-id branch from 4da3951 to 3fd0ed7 Compare November 2, 2022 11:42

ker0x force-pushed the bugfix/form-id branch 2 times, most recently from 28f61d7 to 8dd761e Compare April 18, 2023 10:23

nicolas-grekas force-pushed the bugfix/form-id branch from 8dd761e to 3f3d763 Compare July 13, 2023 14:10

ker0x force-pushed the bugfix/form-id branch from 3f3d763 to d54e174 Compare July 15, 2023 14:23

HeahDude approved these changes Jul 20, 2023

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Jul 20, 2023

nicolas-grekas modified the milestones: 5.4, 6.4 Jul 26, 2023

ker0x changed the base branch from 5.4 to 6.4 July 26, 2023 10:04

ker0x force-pushed the bugfix/form-id branch from d54e174 to fe12153 Compare July 26, 2023 10:10

xabbuh requested changes Jul 26, 2023

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Reviewed labels Jul 26, 2023

ker0x force-pushed the bugfix/form-id branch 2 times, most recently from 8d6067d to abb301b Compare July 26, 2023 15:25

xabbuh reviewed Jul 26, 2023

View reviewed changes

src/Symfony/Bridge/Twig/composer.json Outdated Show resolved Hide resolved

xabbuh reviewed Jul 26, 2023

View reviewed changes

nicolas-grekas modified the milestones: 6.4, 7.1 Nov 15, 2023

ker0x force-pushed the bugfix/form-id branch 3 times, most recently from 42ed263 to 16d3589 Compare December 31, 2023 09:35

[TwigBridge] Set Form ID on form element, prevent duplicate form elem…

2e1e273

…ent attributes

ker0x force-pushed the bugfix/form-id branch from 16d3589 to 2e1e273 Compare March 27, 2024 07:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TwigBridge] Set Form ID on form element, prevent duplicate form element attributes #48013

[TwigBridge] Set Form ID on form element, prevent duplicate form element attributes #48013

ker0x commented Oct 27, 2022 •

edited

Loading

ker0x commented Nov 2, 2022 •

edited

Loading

nicolas-grekas commented Apr 17, 2023

ker0x commented Apr 18, 2023

xabbuh commented Apr 21, 2023

nicolas-grekas commented Jul 13, 2023

HeahDude left a comment

ker0x commented Jul 26, 2023

nicolas-grekas commented Jul 26, 2023

xabbuh left a comment

xabbuh Jul 26, 2023

Search code, repositories, users, issues, pull requests...

[TwigBridge] Set Form ID on form element, prevent duplicate form element attributes #48013

Are you sure you want to change the base?

[TwigBridge] Set Form ID on form element, prevent duplicate form element attributes #48013

Conversation

ker0x commented Oct 27, 2022 • edited Loading

ker0x commented Nov 2, 2022 • edited Loading

nicolas-grekas commented Apr 17, 2023

ker0x commented Apr 18, 2023

xabbuh commented Apr 21, 2023

nicolas-grekas commented Jul 13, 2023

HeahDude left a comment

Choose a reason for hiding this comment

ker0x commented Jul 26, 2023

nicolas-grekas commented Jul 26, 2023

xabbuh left a comment

Choose a reason for hiding this comment

xabbuh Jul 26, 2023

Choose a reason for hiding this comment

ker0x commented Oct 27, 2022 •

edited

Loading

ker0x commented Nov 2, 2022 •

edited

Loading