[Form] Improve rendering of file field in bootstrap 4

[apfelbox]

Q	A
Branch?	4.1+
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	—
License	MIT
Doc PR	—

Hi 👋

currently adding a file type to a form looks weird in the bootstrap 4 form themes:

$builder
    ->add("pdfFile", FileType::class, [
        "label" => "PDF",
    ]);

Before

Vertical Form

Horizontal Form

After

Vertical Form

Horizontal Form

Things to consider

There are three labels here:

1. the actual field label. Here the $options["label"] is used.
2. the placeholder. Here $options["attr"]["placeholder"] ?? "" is used new
3. the label on the button. This is set in CSS actually, on an ::after attribute.

There isn't much we can do about 3) because there is no inline HTML-way to overwrite the style of a pseudo element. So if the app developer wants to have localization in this field as well, (s)he has to set it in their CSS file as described in the bootstrap docs

Todo

• WIP because I haven't managed to get the test suite to run locally yet, so we will debug with Travis 🙌  okay, getting them to work is really, really easy, but you just have to get over yourself and do it 🙄

[apfelbox]

I know that this id-thing here is weird, but the check doesn't make sense for this file type (because of the special structure), I guess.

[xabbuh]

WIll this fix #27477?

[apfelbox]

@xabbuh unfortunately not ... that doesn't even work in their own docs in the examples: https://getbootstrap.com/docs/4.0/components/forms/#file-browser

So imo we have two choices regarding #27477

1. Say that the user has to do it themselves

Just like they need to load the CSS by themselves, they would need to write some JS as integration code here. We can maybe make it easier for them by adding a hint in the docs with example code.

2. add the JS functionality with a possibility to opt-out

Opt-out because inline JS and CSP don't mix and because somebody doesn't want custom JS in their app (not sure whether Symfony 2+ ever injected JS somewhere else besides the toolbar).

Would look something like this:

{% block file_widget -%}
    <div class="form-group">
        ...
    </div>
    <script type="text/javascript">
        (function (document) {
            var input = document.getElementById("{{form.vars.id}}");
            var placeholder = input.parentElement.querySelector("custom-file-label.");

            input.addEventListener(
                "change",
                function ()
                {
                    var value = input.value.replace('C:\\fakepath\\', '').trim();
                    placeholder.textContent = "" !== value ? value : (input.getAttribute('placeholder') || '');
                }
            );
        })(document);
    </script>
{% endblock %}

if we want to set it directly on each field or add some JS at the end of the form that does the same thing.
I can send a PR for it either way, we just need to decide.

[apfelbox]

This issue right here is only for fixing the styling of the form row containing the file field.

[vudaltsov]

This should be for 3.4 since it's a bug, I guess?

[vudaltsov]

With these changes we'll have 2 <label> tags for this field, which is not appropriate according to this stackoverflow answer. We can have <legend> for the first one, WDYT?

[apfelbox]

@vudaltsov the file field in the bootstrap 4 Theme was added in 4.1.

[apfelbox]

@vudaltsov I am afraid that you read the StackOverflow answer wrong..

Here is what I read there:

Each LABEL element is associated with exactly one form control.

One label can only have one input.

[...] each form control can be referenced by multiple labels [...]

One input can have multiple labels.

We fall in the second category.
Using a <label> is quite important here, as we need the functionality, that a click on the label focuses the input field and therefore opens the browser file selector. That automatic focusing is key and doesn't work with a <legend>.

[vudaltsov]

Great! I was wrong. Thank you for explanations 👍

[fabpot]

Thank you @apfelbox.

[apfelbox]

@fabpot thanks for the merge!
I forgot to add something, so there is a follow-up PR in #27958 😞

[MrMitch]

Hi @apfelbox, thank you for this and #27958. I just updated to Symfony 4.1.3 in the hope of using it but I still see the 4.1.1 behavior : no label above the field.

In my case the code

$builder->add('name', TextType::class, [
    'label' => 'Nom du partenaire'
])
->add('image', FileType::class, [
    'label' => 'LABEL',
    'attr' => ['placeholder' => 'PLACEHOLDER'],
    'mapped' => false,
]);

renders the following form :

Looking at the source code in my vendor/symfony/twig-bridge/Resouces/views/Form folder does show your version of the code, so I was wondering if there might have been something between 4.1.2 and 4.1.3 that affected the rendering of FileType widget ?

[apfelbox]

Hmm, not that I know of, so it should work.

Do you have any custom form themes loaded and how do you render the form in your template?

[MrMitch]

After further investigation, it looks like something was wrong with my symfony installation.

The toolbar was reporting 4.1.3 :

but most of the pakages were still on 4.1.1 :

$ composer update --dry-run
Loading composer repositories with package information
Updating dependencies (including require-dev)
Package operations: 3 installs, 47 updates, 0 removals
  - Updating symfony/flex (v1.0.80) to symfony/flex (v1.0.89)
  - Updating symfony/dotenv (v4.1.1) to symfony/dotenv (v4.1.3)
  - Updating symfony/expression-language (v4.1.1) to symfony/expression-language (v4.1.3)
  - Updating symfony/inflector (v4.1.1) to symfony/inflector (v4.1.3)
  - Updating symfony/property-access (v4.1.1) to symfony/property-access (v4.1.3)
  - Updating symfony/options-resolver (v4.1.1) to symfony/options-resolver (v4.1.3)
  - Updating symfony/intl (v4.1.1) to symfony/intl (v4.1.3)
  - Updating symfony/form (v4.1.1) to symfony/form (v4.1.3)
  - Updating symfony/routing (v4.1.1) to symfony/routing (v4.1.3)
  - Updating symfony/finder (v4.1.1) to symfony/finder (v4.1.3)
  - Updating symfony/framework-bundle (v4.1.1) to symfony/framework-bundle (v4.1.3)
  - Updating symfony/phpunit-bridge (v4.1.1) to symfony/phpunit-bridge (v4.1.3)
  - Updating symfony/property-info (v4.1.1) to symfony/property-info (v4.1.3)
  - Updating symfony/security (v4.1.1) to symfony/security (v4.1.3)
  - Updating symfony/security-bundle (v4.1.1) to symfony/security-bundle (v4.1.3)
  - Updating twig/twig (v2.4.8) to twig/twig (v2.5.0)
  - Updating symfony/twig-bridge (v4.1.1) to symfony/twig-bridge (v4.1.3)
  - Updating symfony/twig-bundle (v4.1.1) to symfony/twig-bundle (v4.1.3)
  - Updating symfony/translation (v4.1.1) to symfony/translation (v4.1.3)
  - Updating symfony/validator (v4.1.1) to symfony/validator (v4.1.3)
  - Updating symfony/web-link (v4.1.1) to symfony/web-link (v4.1.3)
  - Updating symfony/asset (v4.1.1) to symfony/asset (v4.1.3)
  - Updating symfony/webpack-encore-pack (v1.0.2) to symfony/webpack-encore-pack (v1.0.3)
  - Updating sebastian/comparator (3.0.1) to sebastian/comparator (3.0.2)
  - Updating phar-io/version (1.0.1) to phar-io/version (2.0.1)
  - Updating phar-io/manifest (1.0.1) to phar-io/manifest (1.0.3)
  - Updating phpunit/phpunit (7.2.6) to phpunit/phpunit (7.2.7)
  - Updating symfony/dom-crawler (v4.1.1) to symfony/dom-crawler (v4.1.3)
  - Updating symfony/browser-kit (v4.1.1) to symfony/browser-kit (v4.1.3)
  - Updating symfony/css-selector (v4.1.1) to symfony/css-selector (v4.1.3)
  - Updating symfony/process (v4.1.1) to symfony/process (v4.1.3)
  - Updating symfony/console (v4.1.1) to symfony/console (v4.1.3)
  - Updating symfony/web-server-bundle (v4.1.1) to symfony/web-server-bundle (v4.1.3)
  - Updating symfony/yaml (v4.1.1) to symfony/yaml (v4.1.3)
  - Updating easycorp/easy-log-handler (v1.0.5) to easycorp/easy-log-handler (v1.0.7)
  - Updating symfony/var-dumper (v4.1.1) to symfony/var-dumper (v4.1.3)
  - Updating symfony/debug-bundle (v4.1.1) to symfony/debug-bundle (v4.1.3)
  - Updating symfony/monolog-bridge (v4.1.1) to symfony/monolog-bridge (v4.1.3)
  - Updating symfony/stopwatch (v4.1.1) to symfony/stopwatch (v4.1.3)
  - Updating symfony/web-profiler-bundle (v4.1.1) to symfony/web-profiler-bundle (v4.1.3)
  - Updating nikic/php-parser (v4.0.2) to nikic/php-parser (v4.0.3)
  - Installing doctrine/reflection (v1.0.0)
  - Installing doctrine/event-manager (v1.0.0)
  - Installing doctrine/persistence (v1.0.0)
  - Updating doctrine/common (v2.8.1) to doctrine/common (v2.9.0)
  - Updating doctrine/dbal (v2.7.1) to doctrine/dbal (v2.8.0)
  - Updating doctrine/orm (v2.6.1) to doctrine/orm (v2.6.2)
  - Updating symfony/serializer (v4.1.1) to symfony/serializer (v4.1.3)
  - Updating swiftmailer/swiftmailer (v6.1.1) to swiftmailer/swiftmailer (v6.1.2)
  - Updating symfony/doctrine-bridge (v4.1.1) to symfony/doctrine-bridge (v4.1.3)

Running composer update fixed everything, and my form now renders properly.
Sorry about the false-positive, thank you for your quick reply !

[MrMitch]

@apfelbox a quick follow-up : I noticed that the .custom-file element in wrapped in an extra div.form-group in the file_widget block :

{% block file_widget -%}
    {# this is the wrapper i'm talking about #}
    <div class="form-group">
        <{{ element|default('div') }} class="custom-file">
            {%- set type = type|default('file') -%}
            {{- block('form_widget_simple') -}}
            <label for="{{ form.vars.id }}" class="custom-file-label">
                {%- if attr.placeholder is defined -%}
                    {{- translation_domain is same as(false) ? attr.placeholder : attr.placeholder|trans({}, translation_domain) -}}
                {%- endif -%}
            </label>
        </{{ element|default('div') }}>
    </div>
{% endblock %}

In bootstrap, the .form-group has a bottom margin.

So when the form_row(form.my_file) function is used, the label is followed by a div.form-group and the help text is rendered after this div.form-group. Because of this, the help text on file widgets is further away from the control than on other widgets:

(this might be easier to see without the Chrome console info)

[apfelbox]

@MrMitch ahaha, you were faster than me 😉
Thanks for your investigation, though

[Pesulap]

It works in my project
function getoutput(){ var value = $("#imputID").val().replace('C:\\fakepath\\', '').trim();$("#imputID").next(".custom-file-label").html(value);}
and
input has attribute onChange="getoutput()"