Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Form] Improve rendering offile field in bootstrap 4#27919
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
file field in bootstrap 4file field in bootstrap 4| $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')), |
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 know that thisid-thing here is weird, but the check doesn't make sense for this file type (because of the special structure), I guess.
WIll this fix#27477? |
apfelbox commentedJul 11, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@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 themselvesJust 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-outOpt-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: {%blockfile_widget -%} <divclass="form-group"> ... </div> <scripttype="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. |
This issue right here is only for fixing the styling of the form row containing the file field. |
This should be for 3.4 since it's a bug, I guess? |
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.
With these changes we'll have 2<label> tags for this field, which is not appropriate according tothis stackoverflow answer. We can have<legend> for the first one, WDYT?
@vudaltsov the file field in the bootstrap 4 Theme was added in 4.1. |
@vudaltsov I am afraid that you read the StackOverflow answer wrong.. Here is what I read there:
One label can only have one input.
One input can have multiple labels. We fall in the second category. |
Great! I was wrong. Thank you for explanations 👍 |
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### Horizontal Form## After### Vertical Form### Horizontal Form## Things to considerThere 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](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
…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 <!-- please add some, will be required by reviewers -->| Fixed tickets | —| License | MIT| Doc PR | —Hi again,This is a follow-up PR for#27919Apparently 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: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 (closessymfony#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### Horizontal Form## After### Vertical Form### Horizontal Form## Things to considerThere 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](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 <!-- please add some, will be required by reviewers -->| Fixed tickets | —| License | MIT| Doc PR | —Hi again,This is a follow-up PR forsymfony#27919Apparently 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:Sorry again, just forgot to actually update the last PR.Commits-------3cd2eef Add placeholder support in bootstrap 4 file fields
MrMitch commentedAug 1, 2018
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 |
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 commentedAug 2, 2018
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-runLoading composer repositories with package informationUpdating 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 |
MrMitch commentedAug 2, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@apfelbox a quick follow-up : I noticed that the {%blockfile_widget -%}{# this is the wrapper i'm talking about#} <divclass="form-group"><{{element|default('div') }}> {%-settype=type|default('file') -%} {{-block('form_widget_simple') -}} <labelfor="{{form.vars.id }}"class="custom-file-label"> {%-ifattr.placeholderisdefined -%} {{-translation_domainissame as(false)?attr.placeholder:attr.placeholder|trans({},translation_domain) -}} {%-endif -%} </label></{{element|default('div') }}> </div>{%endblock %}In bootstrap, the So when the
(this might be easier to see without the Chrome console info) |
@MrMitch ahaha, you were faster than me 😉 |
… 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/aThis 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 4This is a follow-up tosymfony/symfony#27958 andsymfony/symfony#27919.It fixes an extra space between the help text of a FileType widget and thewidget itself. The extra space was cause by a .form-group wrapper in thefile_widget block.
Pesulap commentedFeb 6, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
It works in my project |
Uh oh!
There was an error while loading.Please reload this page.
Hi 👋
currently adding a file type to a form looks weird in the bootstrap 4 form themes:
Before
Vertical Form
Horizontal Form
After
Vertical Form
Horizontal Form
Things to consider
There are three labels here:
$options["label"]is used.$options["attr"]["placeholder"] ?? ""is usednew::afterattribute.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 fileas 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 🙄