Skip to content

Commit

Permalink
Selectmenu: Fix selecting options following hidden ones
Browse files Browse the repository at this point in the history
Change a2b25ef made options with
the `hidden` attribute skipped when rendering. However, that makes
indexes misaligned with native options as hidden ones maintain their
index values. Instead, don't skip hidden options but add the `hidden`
attribute to the respective jQuery UI elements as well.

Fixes gh-2082
Ref a2b25ef
  • Loading branch information
mgol committed Jan 15, 2023
1 parent 3286792 commit ce06e9c
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 8 deletions.
65 changes: 62 additions & 3 deletions tests/unit/selectmenu/core.js
Expand Up @@ -394,16 +394,75 @@ QUnit.test( "Options with hidden attribute should not be rendered", function( as
setTimeout( function() {
button.trigger( "click" );
options = menu.children()
.map( function() {
return $( this ).text();
.get()
.filter( function( item ) {
return $( item ).is( ":visible" );
} )
.get();
.map( function( item ) {
return $( item ).text();
} );
assert.deepEqual( options, [ "Slower", "Medium", "Fast", "Faster" ], "correct elements" );

ready();
} );
} );

QUnit.test( "Options with hidden attribute should not break the widget (gh-2082)",
function( assert ) {
var ready = assert.async();
assert.expect( 1 );

var button;
var element = $( "#speed" );

element.find( "option" ).slice( 0, 2 ).prop( "hidden", true );
element.val( "Faster" );
element.selectmenu();

button = element.selectmenu( "widget" );
button.simulate( "focus" );
setTimeout( function() {
try {
button.trigger( "click" );
assert.strictEqual( button.text(), "Faster", "Selected value is correct" );
} catch ( e ) {
assert.ok( false, "Clicking on the select box crashed" );
}

ready();
} );
} );

QUnit.test( "Optgroups with hidden attribute should not break the widget (gh-2082)",
function( assert ) {
var ready = assert.async();
assert.expect( 1 );

var button;
var element = $( "#files" );

element.find( "optgroup" ).first().prop( "hidden", true );
element
.find( "optgroup" ).eq( 1 )
.find( "option" ).first()
.prop( "hidden", true );
element.val( "someotherfile" );
element.selectmenu();

button = element.selectmenu( "widget" );
button.simulate( "focus" );
setTimeout( function() {
try {
button.trigger( "click" );
assert.strictEqual( button.text(), "Some other file", "Selected option is correct" );
} catch ( e ) {
assert.ok( false, "Clicking on the select box crashed" );
}

ready();
} );
} );

QUnit.test( "extra listeners created after selection (trac-15078, trac-15152)", function( assert ) {
assert.expect( 3 );

Expand Down
12 changes: 7 additions & 5 deletions ui/widgets/selectmenu.js
Expand Up @@ -354,7 +354,12 @@ return $.widget( "ui.selectmenu", [ $.ui.formResetMixin, {
if ( item.disabled ) {
this._addClass( li, null, "ui-state-disabled" );
}
this._setText( wrapper, item.label );

if ( item.hidden ) {
li.prop( "hidden", true );
} else {
this._setText( wrapper, item.label );
}

return li.append( wrapper ).appendTo( ul );
},
Expand Down Expand Up @@ -658,10 +663,6 @@ return $.widget( "ui.selectmenu", [ $.ui.formResetMixin, {
var that = this,
data = [];
options.each( function( index, item ) {
if ( item.hidden ) {
return;
}

data.push( that._parseOption( $( item ), index ) );
} );
this.items = data;
Expand All @@ -675,6 +676,7 @@ return $.widget( "ui.selectmenu", [ $.ui.formResetMixin, {
index: index,
value: option.val(),
label: option.text(),
hidden: optgroup.prop( "hidden" ) || option.prop( "hidden" ),
optgroup: optgroup.attr( "label" ) || "",
disabled: optgroup.prop( "disabled" ) || option.prop( "disabled" )
};
Expand Down

0 comments on commit ce06e9c

Please sign in to comment.