Skip to content

Commit c9d3147

Browse files
committed
Selectmenu: Fix selecting options following hidden ones
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
1 parent 827abdf commit c9d3147

File tree

2 files changed

+69
-8
lines changed

2 files changed

+69
-8
lines changed

tests/unit/selectmenu/core.js

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -394,16 +394,75 @@ QUnit.test( "Options with hidden attribute should not be rendered", function( as
394394
setTimeout( function() {
395395
button.trigger( "click" );
396396
options = menu.children()
397-
.map( function() {
398-
return $( this ).text();
397+
.get()
398+
.filter( function( item ) {
399+
return $( item ).is( ":visible" );
399400
} )
400-
.get();
401+
.map( function( item ) {
402+
return $( item ).text();
403+
} );
401404
assert.deepEqual( options, [ "Slower", "Medium", "Fast", "Faster" ], "correct elements" );
402405

403406
ready();
404407
} );
405408
} );
406409

410+
QUnit.test( "Options with hidden attribute should not break the widget (gh-2082)",
411+
function( assert ) {
412+
var ready = assert.async();
413+
assert.expect( 1 );
414+
415+
var button;
416+
var element = $( "#speed" );
417+
418+
element.find( "option" ).slice( 0, 2 ).prop( "hidden", true );
419+
element.val( "Faster" );
420+
element.selectmenu();
421+
422+
button = element.selectmenu( "widget" );
423+
button.simulate( "focus" );
424+
setTimeout( function() {
425+
try {
426+
button.trigger( "click" );
427+
assert.strictEqual( button.text(), "Faster", "Selected value is correct" );
428+
} catch ( e ) {
429+
assert.ok( false, "Clicking on the select box crashed" );
430+
}
431+
432+
ready();
433+
} );
434+
} );
435+
436+
QUnit.test( "Optgroups with hidden attribute should not break the widget (gh-2082)",
437+
function( assert ) {
438+
var ready = assert.async();
439+
assert.expect( 1 );
440+
441+
var button;
442+
var element = $( "#files" );
443+
444+
element.find( "optgroup" ).first().prop( "hidden", true );
445+
element
446+
.find( "optgroup" ).eq( 1 )
447+
.find( "option" ).first()
448+
.prop( "hidden", true );
449+
element.val( "someotherfile" );
450+
element.selectmenu();
451+
452+
button = element.selectmenu( "widget" );
453+
button.simulate( "focus" );
454+
setTimeout( function() {
455+
try {
456+
button.trigger( "click" );
457+
assert.strictEqual( button.text(), "Some other file", "Selected option is correct" );
458+
} catch ( e ) {
459+
assert.ok( false, "Clicking on the select box crashed" );
460+
}
461+
462+
ready();
463+
} );
464+
} );
465+
407466
QUnit.test( "extra listeners created after selection (trac-15078, trac-15152)", function( assert ) {
408467
assert.expect( 3 );
409468

ui/widgets/selectmenu.js

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,12 @@ return $.widget( "ui.selectmenu", [ $.ui.formResetMixin, {
354354
if ( item.disabled ) {
355355
this._addClass( li, null, "ui-state-disabled" );
356356
}
357-
this._setText( wrapper, item.label );
357+
358+
if ( item.hidden ) {
359+
li.prop( "hidden", true );
360+
} else {
361+
this._setText( wrapper, item.label );
362+
}
358363

359364
return li.append( wrapper ).appendTo( ul );
360365
},
@@ -658,10 +663,6 @@ return $.widget( "ui.selectmenu", [ $.ui.formResetMixin, {
658663
var that = this,
659664
data = [];
660665
options.each( function( index, item ) {
661-
if ( item.hidden ) {
662-
return;
663-
}
664-
665666
data.push( that._parseOption( $( item ), index ) );
666667
} );
667668
this.items = data;
@@ -675,6 +676,7 @@ return $.widget( "ui.selectmenu", [ $.ui.formResetMixin, {
675676
index: index,
676677
value: option.val(),
677678
label: option.text(),
679+
hidden: optgroup.prop( "hidden" ) || option.prop( "hidden" ),
678680
optgroup: optgroup.attr( "label" ) || "",
679681
disabled: optgroup.prop( "disabled" ) || option.prop( "disabled" )
680682
};

0 commit comments

Comments
 (0)