Skip to content
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.

Sign up for GitHub

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

Jump to bottom

function(dataprovider) does not handle needed attributes correctly - text and disabled missing #469

Closed
landreww opened this issue Jan 14, 2015 · 1 comment
Closed

function(dataprovider) does not handle needed attributes correctly - text and disabled missing #469

landreww opened this issue Jan 14, 2015 · 1 comment
Labels
bug enhancement

Comments

@landreww
Copy link

When reading options from an array the code (beginning at line 1074) is missing two attributes, text and disabled. I would also submit that the code should not presuppose the desire of the data provider to want the label attribute to be set with contents of the value attribute (should the label be omitted). The definition of the label attribute indicates that if omitted, it should contain the 'text' value (done in the browser). In addition, Firefox requires the text attribute as it will not display the label.
If anything (although I would argue against it), if the text value is omitted, it should be set to the label (if exists and value exists) or to the value (if exists and no label supplied).

Also, the disabled attribute is missing as it is a proper attribute in both the optiongroup and option tags.

Here is my suggested correction:

                if ($.isArray(option.children)) { // create optiongroup tag
                    groupCounter++;
                    tag = $('<optgroup/>').attr({
                        label: option.label || 'Group ' + groupCounter,
                        disabled: !!option.disabled
                    });
                    forEach(option.children, function(subOption) { // add children option tags
                        tag.append($('<option/>').attr({
                            value: subOption.value,
                            label: subOption.label,
                            title: subOption.title,
                            selected: !!subOption.selected,
                            disabled: !!subOption.disabled
                        }).text(subOption.text));
                    });

                    optionDOM += '</optgroup>';
                }
                else { // create option tag
                    tag = $('<option/>').attr({
                        value: option.value,
                        label: option.label,
                        title: option.title,
                        selected: !!option.selected,
                        disabled: !!option.disabled
                    }).text(option.text);
                }

The data used for my testing was as follows:

     var optgroups = [
        {

            label: 'Dade', disabled: 'disabled', children: [
                {label: 'Miami', value: 'Miami'},
                {label: 'HollyWood', value: 'Hollywood', title: 'this is a group'}
            ]
        },
        {
            label: 'West Central', children: [
                {label: 'This is the label - no value', text: 'This is text - no value', selected: 'selected'},
                {label: 'This is the label - no text', value: 'this is value - no text', selected: 'selected'},             
                {value: 'this is the value - no label', text: 'this is the text - no label', selected: 'selected'},             
                {value: 'this is the value - no label - no text', selected: 'selected'},                
                {label: 'This is the label - no value - no text', selected: 'selected'},                
                {text: 'this is the text - no label - no value', selected: 'selected'},
                {label: 'This is the label', value: 'this is the value', title: 'this is the title', text: 'This is text', selected: 'selected'},
                {label: 'This is the label - no value', text: 'this is text - no value', selected: 'selected'}              
            ]
        }
    ];

This was the resulting selection group in IE11 (will show the label if no text is supplied). Firefox will have a blank line if text is not provided:

<select id="CityList" class="multiselect form-control" name="CityList[]" multiple="multiple" style="display: none;">
    <optgroup label="Dade" disabled="disabled">
        <option value="Miami" label="Miami"></option>
        <option value="Hollywood" label="HollyWood" title="this is a group"></option>
    </optgroup>
    <optgroup label="West Central">
        <option label="This is the label - no value" selected="selected">This is text - no value</option> = LABEL IS DISPLAYED IN LIST
        <option value="this is value - no text" label="This is the label - no text" selected="selected"></option> = LABEL IS DISPLAYED IN LIST
        <option value="this is the value - no label" selected="selected">this is the text - no label</option> = TEXT IS DISPLAYED IN LIST
        <option value="this is the value - no label - no text" selected="selected"></option> = BLANK DISPLAYED IN LIST
        <option label="This is the label - no value - no text" selected="selected"></option> = LABEL IS DISPLAYED IN LIST (no value passed)
        <option selected="selected">this is the text - no label - no value</option> = TEXT IS DISPLAYED IN LIST
        <option value="this is the value" label="This is the label" title="this is the title" selected="selected">This is text</option> = LABEL IS DISPLAYED IN LIST
        <option label="This is the label - no value" selected="selected">this is text - no value</option> = LABEL IS DISPLAYED IN LIST
    </optgroup>
</select>

Please consider updating the code. Let the developer be responsible for providing the correct data in the passed array. Maybe you could update the instructions to indicate that different browsers handle the options differently. However, the text attribute is a MUST!

Thanks.
@davidstutz
Copy link
Owner

Sounds reasonable, thanks! I will have a look and definitely update the documentation.

@davidstutz davidstutz mentioned this issue Feb 12, 2015
Closed
davidstutz added a commit that referenced this issue Feb 17, 2015
@davidstutz
Added #469.
2a65b4b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@davidstutz @landreww