Skip to content

[Form][TwigBridge] Add row_attr to form theme #30320

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

Merged
merged 1 commit into from
Mar 31, 2019

Conversation

alexander-schranz
Copy link
Contributor

@alexander-schranz alexander-schranz commented Feb 20, 2019

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? waiting for confirmation to implement
Fixed tickets
License MIT
Doc PR symfony/symfony-docs#...

Currently you need to copy the whole form_row block if you just want to add a class to the form_row div. Instead we could introduce a row_attr which can then simple be set the following in the theme e.g.:

{% block form_row %}
    {% set row_attr = { class: 'form__field' } %}
    {{ parent() }}
{% endblock %}

or in php:

$builder->add('test', TextType::class, ['row_attr' => ['class' => 'form__field']]);

@OskarStark
Copy link
Contributor

I like the idea 👍

@nicolas-grekas nicolas-grekas added this to the next milestone Feb 23, 2019
@nicolas-grekas nicolas-grekas changed the title Add row_attr to form theme [Form][TwigBridge] Add row_attr to form theme Feb 23, 2019
@xabbuh
Copy link
Member

xabbuh commented Mar 8, 2019

I am not sure if this is a common enough use case or if it's enough to let users add this in their own form type extensions.

@alexander-schranz
Copy link
Contributor Author

alexander-schranz commented Mar 8, 2019

@xabbuh it would make it easer to create reusable themes. Currently you need to check if the installed symfony version has a form_help block or not.

Another fix to this issue could be adding a new block to the form_div_layout:

{% block form_row %}
    <div>
          {{ block('form_row_inner') }}
    </div>
{% endblock %}


{% block form_row_inner %}
    ....
{% endblock %}

@HeahDude
Copy link
Contributor

I would not be in favor of adding yet a new option for the view level unless we can workaround another way.

I think we could just change that line https://github.com/symfony/symfony/blob/master/src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig#L324 to set it only when it's not done before, that would allow you to reset only that value in your block. WDYT?

@stof
Copy link
Member

stof commented Mar 11, 2019

I have an equivalent of this option (I named it wrapper_attr, but row_attr makes more sense as it goes on the div generated by form_row) in the Incenteev codebase since years.
Adding a type extension for that is not the painful part. Overriding all the *_row blocks in the theme to add support for the option is.

I vote 👍 for adding this in core.

@HeahDude
Copy link
Contributor

I don't get why this should be an option? you just need to pass it as context in form_row calls if we allow it by changing the line I link, no?

{{ form(some_form, { widget_attr: ... }) }}

?

@alexander-schranz
Copy link
Contributor Author

@HeahDude but widget_attr is totally different what we want to change. We want to add attributes to the div of the form_row not the widget.

@HeahDude
Copy link
Contributor

HeahDude commented Mar 11, 2019

Yes but we can do both if we add raw_attr there. That would make sense to have all levels configurable the same way, with the template context. Options are overhead also in the submission process.

@alexander-schranz
Copy link
Contributor Author

@HeahDude now I get you. For me ok just add it to the theme and not to the FormType.

What do the others think? /cc @stof @OskarStark

@xabbuh
Copy link
Member

xabbuh commented Mar 11, 2019

@HeahDude's suggestion sounds better to me than adding another option for this.

@HeahDude
Copy link
Contributor

To add an argument on this, ppl wanting an option can still rely on form type extension, and define their own targets which may not be FormType.

@fabpot
Copy link
Member

fabpot commented Mar 31, 2019

Thank you @alexander-schranz.

@fabpot fabpot merged commit 7ab1b00 into symfony:master Mar 31, 2019
fabpot added a commit that referenced this pull request Mar 31, 2019
…er-schranz)

This PR was squashed before being merged into the 4.3-dev branch (closes #30320).

Discussion
----------

[Form][TwigBridge] Add row_attr to form theme

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | waiting for confirmation to implement
| Fixed tickets |
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

Currently you need to copy the whole form_row block if you just want to add a class to the form_row div. Instead we could introduce a `row_attr` which can then simple be set the following in the theme e.g.:

```twig
{% block form_row %}
    {% set row_attr = { class: 'form__field' } %}
    {{ parent() }}
{% endblock %}
```

or in php:

```
$builder->add('test', TextType::class, ['row_attr' => ['class' => 'form__field']]);
```

Commits
-------

7ab1b00 [Form][TwigBridge] Add row_attr to form theme
@alexander-schranz alexander-schranz deleted the patch-2 branch March 31, 2019 13:54
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019
@tehplague
Copy link

Could you add this to the bootstrap form themes, too? Currently only the form_div_layout features the row_attr argument.

@jkabat
Copy link

jkabat commented Aug 30, 2019

Also this is not working out of the box, option has to be defined first:

$builder->add('test', TextType::class, ['row_attr' => ['class' => 'form__field']]);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants