From 020828e7ffd5e7830d979b4c5cf8412f04223973 Mon Sep 17 00:00:00 2001 From: Michał Gołębiowski-Owczarek Date: Wed, 10 May 2023 10:55:01 +0200 Subject: Selectmenu: Fix selecting options following hidden ones Change a2b25ef6caae3e1a272214839b815a6387618124 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 Closes gh-2144 Ref a2b25ef6caae3e1a272214839b815a6387618124 --- tests/unit/selectmenu/core.js | 65 +++++++++++++++++++++++++++++++++++++++++-- ui/widgets/selectmenu.js | 12 ++++---- 2 files changed, 69 insertions(+), 8 deletions(-) diff --git a/tests/unit/selectmenu/core.js b/tests/unit/selectmenu/core.js index f32cec9b9..f8daf4db0 100644 --- a/tests/unit/selectmenu/core.js +++ b/tests/unit/selectmenu/core.js @@ -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 ); diff --git a/ui/widgets/selectmenu.js b/ui/widgets/selectmenu.js index 79e1eacc9..c80e39f60 100644 --- a/ui/widgets/selectmenu.js +++ b/ui/widgets/selectmenu.js @@ -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 ); }, @@ -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; @@ -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" ) }; -- cgit v1.2.3