sonata-project / SonataAdminBundle

The missing Symfony Admin Generator
https://docs.sonata-project.org/projects/SonataAdminBundle
MIT License
2.11k stars 1.26k forks source link

Required sonata_type_model_autocomplete causing "An invalid form control with name='' is not focusable" #4022

Closed enekochan closed 3 years ago

enekochan commented 8 years ago

Environment

Sonata packages

$ composer show sonata-project/*
sonata-project/admin-bundle              3.4.0  The missing Symfony Admin G...
sonata-project/block-bundle              3.1.1  Symfony SonataBlockBundle
sonata-project/cache                     1.0.7  Cache library
sonata-project/classification-bundle     3.1.0  Symfony SonataClassificatio...
sonata-project/core-bundle               3.0.3  Symfony SonataCoreBundle
sonata-project/datagrid-bundle           2.2    Symfony SonataDatagridBundle
sonata-project/doctrine-extensions       1.0.2  Doctrine2 behavioral extens...
sonata-project/doctrine-orm-admin-bundle 3.0.5  Symfony Sonata / Integrate ...
sonata-project/easy-extends-bundle       2.1.10 Symfony SonataEasyExtendsBu...
sonata-project/exporter                  1.5.0  Lightweight Exporter library
sonata-project/intl-bundle               2.2.4  Symfony SonataIntlBundle
sonata-project/media-bundle              3.1.0  Symfony SonataMediaBundle
sonata-project/notification-bundle       3.0.0  Symfony SonataNotificationB...

Symfony packages

$ composer show symfony/*
symfony/assetic-bundle     v2.8.0  Integrates Assetic into Symfony2
symfony/monolog-bundle     2.11.1  Symfony MonologBundle
symfony/polyfill-apcu      v1.2.0  Symfony polyfill backporting apcu_* func...
symfony/polyfill-intl-icu  v1.2.0  Symfony polyfill for intl's ICU-related ...
symfony/polyfill-mbstring  v1.2.0  Symfony polyfill for the Mbstring extension
symfony/polyfill-php54     v1.2.0  Symfony polyfill backporting some PHP 5....
symfony/polyfill-php55     v1.2.0  Symfony polyfill backporting some PHP 5....
symfony/polyfill-php56     v1.2.0  Symfony polyfill backporting some PHP 5....
symfony/polyfill-php70     v1.2.0  Symfony polyfill backporting some PHP 7....
symfony/polyfill-util      v1.2.0  Symfony utilities for portability of PHP...
symfony/security-acl       v3.0.0  Symfony Security Component - ACL (Access...
symfony/swiftmailer-bundle v2.3.11 Symfony SwiftmailerBundle
symfony/symfony            v2.8.8  The Symfony PHP framework

PHP version

$ php -v
PHP 7.0.9 (cli) (built: Jul 21 2016 08:19:58) ( NTS )
Copyright (c) 1997-2016 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2016 Zend Technologies
    with Zend OPcache v7.0.9, Copyright (c) 1999-2016, by Zend Technologies
    with Xdebug v2.4.0, Copyright (c) 2002-2016, by Derick Rethans

Subject

First of all this could be related to #1994 and #3446 but the first one was never marked as fixed and the former has a commit that should have fix it but still getting the same error message.

I have some sonata_type_model_autocomplete fields in an admin entity that fail to validate correctly in Chrome (52 right now) when trying to create a new entity from Sonata Admin. This problem only happens with fields marked as required, for example:

class BusinessAdmin extends BaseAdmin
{
    // Fields to be shown on create/edit forms
    protected function configureFormFields(FormMapper $formMapper)
    {
        $formMapper
...
                ->add(
                    'city',
                    'sonata_type_model_autocomplete',
                    array(
                        'label' => 'admin.label.city',
                        'required' => true,
                        'multiple' => false,
                        'property' => 'name',
                    )
                )
...
                ->add(
                    'categories',
                    'sonata_type_model_autocomplete',
                    array(
                        'required' => true,
                        'multiple' => true,
                        'property' => 'name',
                        'to_string_callback' => function($entity, $property) {
                            return $entity->getName().' (ID: '.$entity->getId().')';
                        },
                        'callback' => function ($admin, $property, $value) use ($parentCategory) {
                            // Show only child categories from "Businesses" category
                            $datagrid = $admin->getDatagrid();
                            $queryBuilder = $datagrid->getQuery();
                            $queryBuilder
                                ->andWhere($queryBuilder->getRootAlias() . '.parent=:parentCategory')
                                ->setParameter('parentCategory', $parentCategory->getId())
                            ;
                            $datagrid->setValue($property, null, $value);
                        },
                    )
                )
    }
}

There is another not required (and multiple) sonata_type_model_autocomplete field in the same admin entity that does not arise the issue:

            ->add(
                'users',
                'sonata_type_model_autocomplete',
                array(
                    'required' => false,
                    'multiple' => true,
                    'label' => 'admin.label.users',
                    'property'=>'username',
                )
            )

This is the generated HTML+Java Script in the form for one off the affected fields (I know it's messy and I'm sorry but it may help debug the issue):

<div class="form-group" id="sonata-ba-field-container-s5798adf90f5a1_city">

                    <label class="control-label required" for="s5798adf90f5a1_city">
                            Ciudad
                    </label>

        <div class="sonata-ba-field sonata-ba-field-standard-natural">
                <div class="select2-container form-control" id="s2id_s5798adf90f5a1_city_autocomplete_input"><a href="javascript:void(0)" class="select2-choice" tabindex="-1">   <span class="select2-chosen" id="select2-chosen-1">Vitoria-Gasteiz</span><abbr class="select2-search-choice-close"></abbr>   <span class="select2-arrow" role="presentation"><b role="presentation"></b></span></a><label for="s2id_autogen1" class="select2-offscreen"></label><input class="select2-focusser select2-offscreen" type="text" aria-haspopup="true" role="button" aria-labelledby="select2-chosen-1" id="s2id_autogen1"></div><input type="text" id="s5798adf90f5a1_city_autocomplete_input" value="" required="required" tabindex="-1" title="" style="display: none;"><div id="s5798adf90f5a1_city_hidden_inputs_wrap"><input type="hidden" name="s5798adf90f5a1[city]" value="2715"></div><script>
        (function ($) {
            var autocompleteInput = $('#s5798adf90f5a1_city_autocomplete_input');
            autocompleteInput.select2({placeholder: '', // allowClear needs placeholder to work properly
                allowClear: false,
                enable: true,
                readonly: false,
                minimumInputLength: 3,
                multiple: false,
                width: '',
                dropdownAutoWidth: false,
                containerCssClass: ' form-control',
                dropdownCssClass: '',
                ajax: {
                    url:  '\x2Fapp_dev.php\x2Fadmin\x2Fcore\x2Fget\x2Dautocomplete\x2Ditems',
                    dataType: 'json',
                    quietMillis: 100,
                    cache: false,
                    data: function (term, page) { // page is the one-based page number tracked by Select2
                                                return {
                                //search term
                                'q': term,

                                // page size
                                '_per_page': 10,

                                // page number
                                '_page': page,

                                // admin
                                                                    'uniqid': 's5798adf90f5a1',
                                    'admin_code': 'sonata.admin.business',

                                 // subclass

                                                                    'field':  'city'

                                // other parameters
                                                        };
                                            },
                    results: function (data, page) {
                        // notice we return the value of more so Select2 knows if more results can be loaded
                        return {results: data.items, more: data.more};
                    }
                },
                formatResult: function (item) {
                    return '<div class="">'+item.label+'<\/div>';// format of one dropdown item
                },
                formatSelection: function (item) {
                    return item.label;// format selected item '<b>'+item.label+'</b>';
                },
                escapeMarkup: function (m) { return m; } // we do not want to escape markup since we are displaying html in results
            });

            autocompleteInput.on('change', function(e) {

                // console.log('change '+JSON.stringify({val:e.val, added:e.added, removed:e.removed}));

                // remove input
                if (undefined !== e.removed && null !== e.removed) {
                    var removedItems = e.removed;

                    $('#s5798adf90f5a1_city_hidden_inputs_wrap input:hidden').val('');                }

                // add new input
                var el = null;
                if (undefined !== e.added) {

                    var addedItems = e.added;

                    $('#s5798adf90f5a1_city_hidden_inputs_wrap input:hidden').val(addedItems.id);                }
            });

            // Initialise the autocomplete
            var data = [];if (undefined==data.length || 0<data.length) { // Leave placeholder if no data set
                autocompleteInput.select2('data', data);
            }

            // remove unneeded autocomplete text input before form submit
            $('#s5798adf90f5a1_city_autocomplete_input').closest('form').submit(function()
            {
                $('#s5798adf90f5a1_city_autocomplete_input').remove();
                return true;
            });
        })(jQuery);
    </script> 

                    </div>
    </div>

Steps to reproduce

Add a sonata_type_model_autocomplete field linked to another entity with required set to true. Open the create new entity form in Sonata Admin and don't select a related object for that field. Click on "Create".

Expected results

The required fields should be focused and labeled with the browsers own validate message "Please fill this out this field" (if you are using Chrome, other browser will have another text message).

Actual results

A JavaScript error with this message in the console:

[Sonata.Admin] [core|show_form_first_tab_with_errors] show first tab with errors, [object Object]
create:1 An invalid form control with name='' is not focusable.
greg0ire commented 8 years ago

create:1

This is one of the reasons why we should stop with inlined javascript… it's very hard to debug.

Thanks for reporting this. Can you set a breakpoint and do step by step debugging to find the actual line that is responsible for this error message?

enekochan commented 8 years ago

Yes sure, I'll do a step by step debugging as soon as I have a break at work. Just so I can go straight to the point, do you know which JavaScript file and/or function is called when any of the "Create" buttons are clicked?

greg0ire commented 8 years ago

Just so I can go straight to the point, do you know which JavaScript file and/or function is called when any of the "Create" buttons are clicked?

I don't know, but I think you can find out easily with recent versions of firefox.

enekochan commented 8 years ago

I think I've narrowed it down a little bit. The autocomplete input is being rendered with select2, and it has a non visible input field (with a style="display:none") were the actual selected value is placed, AFAIK this is how select2 works internaly. As I made my field in the form required the browser tries to validate it but as it is non visible it's also non focusable, thus the error. If I make this input field visible (removing the style with the inspector once the page is loaded) it shows the usual "Please fill out this field" when "Create" is clicked and the error is gone, but of course the input field is shown bellow the select2 widget.

screen shot 2016-07-29 at 11 23 32

Here you can see the input that gives the problem selected in blue (with the style already removed):

screen shot 2016-07-29 at 11 24 27
BenjaminBeck commented 7 years ago

I needed a quick solution:

.sonata-ba-field.sonata-ba-field-standard-natural select[tabindex='-1']{
    display:block !important;
    width: 0.1px !important;
    height: 0.1px !important;
    opacity: 0.01 !important;
}
lalop commented 7 years ago

this seems to be fixed into select2 v4 https://github.com/select2/select2/issues/128

OskarStark commented 7 years ago

@Soullivaneuh started to implement Select 2 4.0 in this PR https://github.com/sonata-project/SonataCoreBundle/pull/147, maybe someone can support him or overtake this PR

EDIT but this is against master branch and would only affect the new major version of core-bundle 😢

apapacy commented 7 years ago

Please, fix this truble. For validation reason only input type text must be required. But select "required" have some conflict with html5.

<input type="text" id="{{ id }}_autocomplete_input" value=""
{%- if read_only is defined and read_only %} readonly="readonly"{% endif -%}
{%- if disabled %} disabled="disabled"{% endif -%}
{%- if required %} required="required"{% endif %}
/>

<select id="{{ id }}_autocomplete_input_v4" data-sonata-select2="false"
    {%- if read_only is defined and read_only %} readonly="readonly"{% endif -%}
    {%- if disabled %} disabled="disabled"{% endif -%}
    {######  This is a fix only need %- if required %} required="required"{% endif %######}
>
</select>
core23 commented 7 years ago

Feel free to contribute and fix this problem @apapacy

apapacy commented 7 years ago

OK

eschultz-magix commented 7 years ago

I have the same error and this is my workaround:

Create a custom template called sonata_type_model_autocomplete.html.twig in the Resources/Form/Type folder of your bundle with this as content:

{% include 'SonataAdminBundle:Form/Type:sonata_type_model_autocomplete.html.twig' %}

{% spaceless %}
{% if required %}
    <script>
        (function ($) {
            if (window.Select2) {
                var usedInputRef = '#{{ id }}_autocomplete_input';
                var unusedInputRef = '#{{ id }}_autocomplete_input_v4';
                $(unusedInputRef).removeAttr('required');

                var autocompleteInput = $(usedInputRef);
                autocompleteInput.on('change', function(e) {
                    var selected = $('#{{ id }}_hidden_inputs_wrap input:hidden');
                    if (selected.length > 0) {
                        autocompleteInput.removeAttr('required');
                    } else {
                        autocompleteInput.attr('required', 'required');
                    }
                });
            }
        })(jQuery);
    </script>
{% endif %}
{% endspaceless %}

And then use it in your admin class:

$formMapper->add('field', 'Sonata\AdminBundle\Form\Type\ModelAutocompleteType', array(
    'property' => 'id',
    'template' => 'AcmeDemoBundle:Form/Type:sonata_type_model_autocomplete.html.twig'
));

The script is also compatible with Select2 v4 (it simply does nothing then).

OskarStark commented 7 years ago

@eschultz-magix could you fix it and provide a PR?

enekochan commented 7 years ago

This problem still persists even Select2 v4 compatibility for model autocomplete has been introduced with the release of 3.18.1 and 3.18.2

greg0ire commented 7 years ago

@enekochan even with #4512 ?

enekochan commented 7 years ago

Yes, I'm using 3.19.0 right now but still have the same problem.

composer show sonata-project/*
sonata-project/admin-bundle              3.19.0          The missing Symfon...
sonata-project/block-bundle              3.3.2           Symfony SonataBloc...
sonata-project/cache                     1.0.7           Cache library
sonata-project/classification-bundle     3.x-dev ba4f0cb Symfony SonataClas...
sonata-project/core-bundle               3.4.0           Symfony SonataCore...
sonata-project/datagrid-bundle           2.2.1           Symfony SonataData...
sonata-project/doctrine-extensions       1.0.2           Doctrine2 behavior...
sonata-project/doctrine-orm-admin-bundle 3.x-dev 884233a Symfony Sonata / I...
sonata-project/easy-extends-bundle       2.2.0           Symfony SonataEasy...
sonata-project/exporter                  1.7.1           Lightweight Export...
sonata-project/intl-bundle               2.3.0           Symfony SonataIntl...
sonata-project/media-bundle              3.x-dev d7c4934 Symfony SonataMedi...
sonata-project/notification-bundle       3.1.0           Symfony SonataNoti...
$ composer show symfony/*
symfony/assetic-bundle     v2.8.1  Integrates Assetic into Symfony2
symfony/monolog-bundle     v2.12.1 Symfony MonologBundle
symfony/polyfill-apcu      v1.4.0  Symfony polyfill backporting apcu_* func...
symfony/polyfill-intl-icu  v1.4.0  Symfony polyfill for intl's ICU-related ...
symfony/polyfill-mbstring  v1.4.0  Symfony polyfill for the Mbstring extension
symfony/polyfill-php54     v1.4.0  Symfony polyfill backporting some PHP 5....
symfony/polyfill-php55     v1.4.0  Symfony polyfill backporting some PHP 5....
symfony/polyfill-php56     v1.4.0  Symfony polyfill backporting some PHP 5....
symfony/polyfill-php70     v1.4.0  Symfony polyfill backporting some PHP 7....
symfony/polyfill-util      v1.4.0  Symfony utilities for portability of PHP...
symfony/security-acl       v3.0.0  Symfony Security Component - ACL (Access...
symfony/swiftmailer-bundle v2.6.2  Symfony SwiftmailerBundle
symfony/symfony            v2.8.22 The Symfony PHP framework
greg0ire commented 7 years ago

@enekochan what do you think is happening? Is the unused input ref not removed? Or is there another hidden input that needs to be removed?

enekochan commented 7 years ago

There is an input field with required="true" and style"display: none;", and just bellow it the actual input type="hidden" that stores the value that is sent with the form.

sonata-admin-input-not-focusable

I've tried deleting that first input (via devtools) and I get no errors, just that the select is not validated before submit, so if it's empty I would get and error message saying that field is mandatory after the submit and the page is reloaded.

greg0ire commented 7 years ago

Are you sure? IMO only the second should be mandatory, because it has a name

peter-gribanov commented 7 years ago

@enekochan i had this problem and #4512 is decided it.

enekochan commented 7 years ago

I'm using 3.19.0 which has that merged and still. I've created a mini symfony app that reproduces the error:

https://github.com/enekochan/sonata-model-autocomplete

If you try to create a new Business object within SonataAdmin setting a name for it but not choosing a City the error will be shown in the JavaScript console.

greg0ire commented 7 years ago

What about my question @enekochan ? The field that should be validated server side is the second one, isn't it?

enekochan commented 7 years ago

@greg0ire Sorry. Yes, I think it is, when I just deleted the first one the form was correctly validated server side.

greg0ire commented 7 years ago

Ok so now let's try to answer some more questions:

  1. What version of select2 are you using?
  2. is the first input necessary for the version of select2 you are currently using?
    1. if yes, how come it is empty when you validate?
    2. if not, why wasn't it removed by the piece of code in my PR?
BrianWalters commented 6 years ago

I ran into this problem. Same situation. A ManyToMany association, using ModelAutocompleteType, and a "required" => true in the form type options.

From what I can tell, the input that is selected from the '#{{ id }}_autocomplete_input_v4' is correctly removed. There is another input that is '#{{ id }}_autocomplete_input' (same but without the v4) that is required and is unfocusable. Built-in browser validation can't focus the required form element, which prevents the form from being submitted and doesn't provide a useful error message.

VincentLanglet commented 4 years ago

I think I have this issue for every EntityType::class field (and maybe more Type, maybe ChoiceType too). The User can't submit the page without having any error message, since the required field has display: none.

I have to put all my field as required: false and use validation constraint. That's not the best.

I'll take a look if I can fix this.

github-actions[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

VincentLanglet commented 3 years ago

Instead of display: none,

display: block;
position: absolute;
height: 0px;
pointer-events: none;
border-color: transparent;
background: transparent;
appearance: none;

works fine (from https://github.com/select2/select2/issues/128#issuecomment-749418742)