Skip to content

Commit

Permalink
Selectmenu: Don't use active items at all
Browse files Browse the repository at this point in the history
Menus only use active items for nested menus which selectmenu doesn't support.
Selectmenu should only be working with focused items.

Ref gh-1224
  • Loading branch information
scottgonzalez committed Apr 18, 2014
1 parent 1849655 commit 1272fca
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 23 deletions.
26 changes: 11 additions & 15 deletions tests/unit/selectmenu/selectmenu_core.js
Expand Up @@ -185,36 +185,36 @@ $.each([
}); });


asyncTest( "item focus and active state - " + settings.type, function () { asyncTest( "item focus and active state - " + settings.type, function () {
expect( 8 ); expect( 4 );


var element = $( settings.selector ).selectmenu(), var items, focusedItem, activeItem,
element = $( settings.selector ).selectmenu(),
button = element.selectmenu( "widget" ), button = element.selectmenu( "widget" ),
menu = element.selectmenu( "menuWidget" ), menu = element.selectmenu( "menuWidget" );
links, focusedItem, activeItem;


// init menu // Initialize menu
button.simulate( "focus" ); button.simulate( "focus" );


setTimeout(function() { setTimeout(function() {
links = menu.find( "li.ui-menu-item" ); items = menu.find( "li.ui-menu-item" );


button.trigger( "click" ); button.trigger( "click" );
setTimeout(function() { setTimeout(function() {
checkItemClasses(); checkItemClasses();


links.eq( 3 ).simulate( "mouseover" ).trigger( "click" ); items.eq( 3 ).simulate( "mouseover" ).trigger( "click" );


button.trigger( "click" ); button.trigger( "click" );
links.eq( 2 ).simulate( "mouseover" ); items.eq( 2 ).simulate( "mouseover" );
$( document ).trigger( "click" ); $( document ).trigger( "click" );


button.trigger( "click" ); button.trigger( "click" );
links.eq( 1 ).simulate( "mouseover" ); items.eq( 1 ).simulate( "mouseover" );
$( document ).trigger( "click" ); $( document ).trigger( "click" );


button.trigger( "click" ); button.trigger( "click" );
setTimeout(function() { setTimeout(function() {
checkItemClasses(); checkItemClasses();
start(); start();
}, 350 ); }, 350 );
}, 350 ); }, 350 );
Expand All @@ -223,11 +223,7 @@ $.each([
function checkItemClasses() { function checkItemClasses() {
focusedItem = menu.find( "li.ui-state-focus" ); focusedItem = menu.find( "li.ui-state-focus" );
equal( focusedItem.length, 1, "only one item has ui-state-focus class" ); equal( focusedItem.length, 1, "only one item has ui-state-focus class" );
equal( focusedItem.attr( "id" ), links.eq( element[ 0 ].selectedIndex ).attr( "id" ), "selected item has ui-state-focus class" ); equal( focusedItem.attr( "id" ), items.eq( element[ 0 ].selectedIndex ).attr( "id" ), "selected item has ui-state-focus class" );

activeItem = menu.find( "li.ui-state-active" );
equal( activeItem.length, 1, "only one item has ui-state-active class" );
equal( activeItem.attr( "id" ), links.eq( element[ 0 ].selectedIndex ).attr( "id" ), "selected item has ui-state-active class" );
} }
}); });


Expand Down
10 changes: 2 additions & 8 deletions ui/selectmenu.js
Expand Up @@ -223,11 +223,10 @@ return $.widget( "ui.selectmenu", {
if ( !this.menuItems ) { if ( !this.menuItems ) {
this._refreshMenu(); this._refreshMenu();
} else { } else {
// TODO: Why is this necessary?
// Shouldn't the underlying menu always have accurate state? // Menu clears focus on close, reset focus to selected item
this.menu.find( ".ui-state-focus" ).removeClass( "ui-state-focus" ); this.menu.find( ".ui-state-focus" ).removeClass( "ui-state-focus" );
this.menuInstance.focus( null, this._getSelectedItem() ); this.menuInstance.focus( null, this._getSelectedItem() );
this.menuItems.eq( this.element[ 0 ].selectedIndex ).addClass( "ui-state-active" );
} }


this.isOpen = true; this.isOpen = true;
Expand All @@ -252,11 +251,6 @@ return $.widget( "ui.selectmenu", {
this.isOpen = false; this.isOpen = false;
this._toggleAttr(); this._toggleAttr();


// Check if we have an item to select
if ( this.menuItems ) {
this.menuInstance.active = this._getSelectedItem();
}

this._off( this.document ); this._off( this.document );


this._trigger( "close", event ); this._trigger( "close", event );
Expand Down

0 comments on commit 1272fca

Please sign in to comment.