[Form] Improve rendering of `file` field in bootstrap 4 #27919

apfelbox · 2018-07-10T19:45:12Z

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:

the actual field label. Here the $options["label"] is used.
the placeholder. Here $options["attr"]["placeholder"] ?? "" is used new
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 · 2018-07-11T09:41:28Z

src/Symfony/Component/Form/Tests/AbstractBootstrap4LayoutTest.php

-        $this->assertWidgetMatchesXpath($form->createView(), array('attr' => array('class' => 'my&class form-control-file')),
-'/input
-    [@type="file"]
+        $this->assertWidgetMatchesXpath($form->createView(), array('id' => 'nope', 'attr' => array('class' => 'my&class form-control-file')),


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 · 2018-07-11T11:39:41Z

WIll this fix #27477?

apfelbox · 2018-07-11T13:11:38Z

@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 · 2018-07-11T13:12:25Z

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

vudaltsov · 2018-07-12T12:23:14Z

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 · 2018-07-12T12:32:20Z

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

apfelbox · 2018-07-12T12:46:42Z

@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 · 2018-07-12T12:49:39Z

Great! I was wrong. Thank you for explanations 👍

fabpot · 2018-07-15T07:27:00Z

Thank you @apfelbox.

…pfelbox) This PR was squashed before being merged into the 4.1 branch (closes #27919). Discussion ---------- [Form] Improve rendering of `file` field in bootstrap 4 | 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: ```php $builder ->add("pdfFile", FileType::class, [ "label" => "PDF", ]); ``` ## Before ### Vertical Form ![2018-07-10 at 21 36](https://user-images.githubusercontent.com/1032411/42533175-6b88e33a-8489-11e8-927a-8e987f362913.png) ### Horizontal Form ![2018-07-10 at 21 37](https://user-images.githubusercontent.com/1032411/42533197-7d45944c-8489-11e8-970f-79b18a273366.png) ## After ### Vertical Form ![2018-07-10 at 21 38](https://user-images.githubusercontent.com/1032411/42533229-8ebe44a8-8489-11e8-94e0-891403063476.png) ### Horizontal Form ![2018-07-10 at 21 38](https://user-images.githubusercontent.com/1032411/42533247-99782db4-8489-11e8-9f66-30a5dccdaa9d.png) ## Things to consider There are three labels here: ![2018-07-10 at 21 40](https://user-images.githubusercontent.com/1032411/42533370-ecbaf63c-8489-11e8-9be8-1c8c3342c459.png) 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](https://getbootstrap.com/docs/4.0/components/forms/#file-browser) ## Todo - [x] ~~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 🙄 Commits ------- 32ad5d9 [Form] Improve rendering of `file` field in bootstrap 4

apfelbox · 2018-07-15T15:53:43Z

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

…lbox) This PR was merged into the 4.1 branch. Discussion ---------- [Form] Remaining changes for bootstrap 4 file fields | 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 again, This is a follow-up PR for #27919 Apparently I talked about it in the previous PR, but forgot to actually push the change. Sorry about that! 😞 This PR no actually adds this instead of just talking about it: ![2018-07-15 at 17 52](https://user-images.githubusercontent.com/1032411/42735630-e19b22be-8857-11e8-85b8-6d64e17c2be2.png) Sorry again, just forgot to actually update the last PR. Commits ------- 3cd2eef Add placeholder support in bootstrap 4 file fields

…ap 4 (apfelbox) This PR was squashed before being merged into the 4.1 branch (closes symfony#27919). Discussion ---------- [Form] Improve rendering of `file` field in bootstrap 4 | 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: ```php $builder ->add("pdfFile", FileType::class, [ "label" => "PDF", ]); ``` ## Before ### Vertical Form ![2018-07-10 at 21 36](https://user-images.githubusercontent.com/1032411/42533175-6b88e33a-8489-11e8-927a-8e987f362913.png) ### Horizontal Form ![2018-07-10 at 21 37](https://user-images.githubusercontent.com/1032411/42533197-7d45944c-8489-11e8-970f-79b18a273366.png) ## After ### Vertical Form ![2018-07-10 at 21 38](https://user-images.githubusercontent.com/1032411/42533229-8ebe44a8-8489-11e8-94e0-891403063476.png) ### Horizontal Form ![2018-07-10 at 21 38](https://user-images.githubusercontent.com/1032411/42533247-99782db4-8489-11e8-9f66-30a5dccdaa9d.png) ## Things to consider There are three labels here: ![2018-07-10 at 21 40](https://user-images.githubusercontent.com/1032411/42533370-ecbaf63c-8489-11e8-9be8-1c8c3342c459.png) 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](https://getbootstrap.com/docs/4.0/components/forms/#file-browser) ## Todo - [x] ~~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 🙄 Commits ------- 32ad5d9 [Form] Improve rendering of `file` field in bootstrap 4

…s (apfelbox) This PR was merged into the 4.1 branch. Discussion ---------- [Form] Remaining changes for bootstrap 4 file fields | 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 again, This is a follow-up PR for symfony#27919 Apparently I talked about it in the previous PR, but forgot to actually push the change. Sorry about that! 😞 This PR no actually adds this instead of just talking about it: ![2018-07-15 at 17 52](https://user-images.githubusercontent.com/1032411/42735630-e19b22be-8857-11e8-85b8-6d64e17c2be2.png) Sorry again, just forgot to actually update the last PR. Commits ------- 3cd2eef Add placeholder support in bootstrap 4 file fields

MrMitch · 2018-08-01T23:26:08Z

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 · 2018-08-02T06:16:50Z

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 · 2018-08-02T10:18:36Z

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 · 2018-08-02T10:59:53Z

@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 · 2018-08-02T11:37:14Z

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

@apfelbox

… in bootstrap 4 (MrMitch) This PR was merged into the 4.1 branch. Discussion ---------- [Form] Remove extra .form-group wrapper around file widget in bootstrap 4 | Q | A | ------------- | --- | Branch? | 4.1 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a This is a follow-up to #27958 and #27919 by @apfelbox . It fixes an extra space between the help text of a FileType widget and the widget itself. The extra space was caused by a `.form-group` wrapper in the `file_widget` block. Commits ------- 01e7fe4 [Form] Remove extra .form-group wrapper around file widget in bootstrap 4

…ap 4 This is a follow-up to symfony/symfony#27958 and symfony/symfony#27919. It fixes an extra space between the help text of a FileType widget and the widget itself. The extra space was cause by a .form-group wrapper in the file_widget block.

Pesulap · 2019-02-06T19:13:14Z

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()"

carsonbot added Status: Needs Review Form Bug labels Jul 10, 2018

apfelbox changed the title ~~[WIP][Form] Improve rendering of file field in bootstrap 4~~ [Form] Improve rendering of file field in bootstrap 4 Jul 10, 2018

OskarStark approved these changes Jul 11, 2018

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Jul 11, 2018

apfelbox commented Jul 11, 2018

View reviewed changes

xabbuh added the TwigBridge label Jul 11, 2018

vudaltsov reviewed Jul 12, 2018

View reviewed changes

vudaltsov approved these changes Jul 12, 2018

View reviewed changes

fabpot approved these changes Jul 13, 2018

View reviewed changes

xabbuh approved these changes Jul 14, 2018

View reviewed changes

[Form] Improve rendering of file field in bootstrap 4

32ad5d9

fabpot merged commit 32ad5d9 into symfony:4.1 Jul 15, 2018

apfelbox deleted the bootstrap-file-field-styling branch July 15, 2018 15:38

apfelbox mentioned this pull request Jul 15, 2018

[Form] Remaining changes for bootstrap 4 file fields #27958

Merged

fabpot mentioned this pull request Jul 23, 2018

Release v4.1.2 #28043

Merged

MrMitch mentioned this pull request Aug 2, 2018

[Form] Remove extra .form-group wrapper around file widget in bootstrap 4 #28115

Merged

apfelbox mentioned this pull request Aug 2, 2018

[Form] Remove duplicate .form-group in bootstrap 4 file field #28116

Closed

apfelbox mentioned this pull request Dec 4, 2018

[Twig] Form - Bootstrap 4 layout - custom-file-input - not able to see which file are selected in a input file type #27477

Closed

jean-gerard mentioned this pull request Apr 18, 2019

[TWIG] File input, visible but no text #31159

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Form] Improve rendering of `file` field in bootstrap 4 #27919

[Form] Improve rendering of `file` field in bootstrap 4 #27919

apfelbox commented Jul 10, 2018 •

edited

Loading

apfelbox Jul 11, 2018

xabbuh commented Jul 11, 2018

apfelbox commented Jul 11, 2018 •

edited

Loading

apfelbox commented Jul 11, 2018

vudaltsov commented Jul 12, 2018

vudaltsov left a comment

apfelbox commented Jul 12, 2018

apfelbox commented Jul 12, 2018

vudaltsov commented Jul 12, 2018

fabpot commented Jul 15, 2018

apfelbox commented Jul 15, 2018

MrMitch commented Aug 1, 2018

apfelbox commented Aug 2, 2018

MrMitch commented Aug 2, 2018

MrMitch commented Aug 2, 2018 •

edited

Loading

apfelbox commented Aug 2, 2018

Pesulap commented Feb 6, 2019 •

edited

Loading

[Form] Improve rendering of file field in bootstrap 4 #27919

[Form] Improve rendering of file field in bootstrap 4 #27919

Conversation

apfelbox commented Jul 10, 2018 • edited Loading

Before

Vertical Form

Horizontal Form

After

Vertical Form

Horizontal Form

Things to consider

Todo

apfelbox Jul 11, 2018

Choose a reason for hiding this comment

xabbuh commented Jul 11, 2018

apfelbox commented Jul 11, 2018 • edited Loading

1. Say that the user has to do it themselves

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

apfelbox commented Jul 11, 2018

vudaltsov commented Jul 12, 2018

vudaltsov left a comment

Choose a reason for hiding this comment

apfelbox commented Jul 12, 2018

apfelbox commented Jul 12, 2018

vudaltsov commented Jul 12, 2018

fabpot commented Jul 15, 2018

apfelbox commented Jul 15, 2018

MrMitch commented Aug 1, 2018

apfelbox commented Aug 2, 2018

MrMitch commented Aug 2, 2018

MrMitch commented Aug 2, 2018 • edited Loading

apfelbox commented Aug 2, 2018

Pesulap commented Feb 6, 2019 • edited Loading

[Form] Improve rendering of `file` field in bootstrap 4 #27919

[Form] Improve rendering of `file` field in bootstrap 4 #27919

apfelbox commented Jul 10, 2018 •

edited

Loading

apfelbox commented Jul 11, 2018 •

edited

Loading

MrMitch commented Aug 2, 2018 •

edited

Loading

Pesulap commented Feb 6, 2019 •

edited

Loading