Skip to content

Commit 378dacf

Browse files
scottgonzalezscottgonzalez
committed
Menu: Cleanup.
1 parent 4f3c1e9 commit 378dacf

File tree

1 file changed

+58
-46
lines changed

1 file changed

+58
-46
lines changed

‎ui/jquery.ui.menu.js

Lines changed: 58 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
* jquery.ui.core.js
1212
* jquery.ui.widget.js
1313
*/
14-
(function($) {
14+
(function( $, undefined ) {
1515

1616
var currentEventTarget = null;
1717

@@ -32,6 +32,7 @@ $.widget( "ui.menu", {
3232
focus: null,
3333
select: null
3434
},
35+
3536
_create: function() {
3637
this.activeMenu = this.element;
3738
this.element
@@ -69,7 +70,10 @@ $.widget( "ui.menu", {
6970
var target = $( event.target );
7071
if ( target[0] !== currentEventTarget ) {
7172
currentEventTarget = target[0];
72-
target.one( "click.menu", function( event ) {
73+
// TODO: What are we trying to accomplish with this check?
74+
// Clicking a menu item twice results in a select event with
75+
// an empty ui.item.
76+
target.one( "click" + this.eventNamespace, function( event ) {
7377
currentEventTarget = null;
7478
});
7579
// Don't select disabled menu items
@@ -94,20 +98,27 @@ $.widget( "ui.menu", {
9498
mouseleave: "collapseAll",
9599
"mouseleave .ui-menu": "collapseAll",
96100
focus: function( event ) {
97-
var menu = this.element,
98-
firstItem = menu.children( ".ui-menu-item" ).eq( 0 );
99-
if ( this._hasScroll() && !this.active ) {
101+
var menuTop,
102+
menu = this.element,
103+
// Default to focusing the first item
104+
item = menu.children( ".ui-menu-item" ).eq( 0 );
105+
106+
// If there's already an active item, keep it active
107+
if ( this.active ) {
108+
item = this.active;
109+
// If there's no active item and the menu is scrolled,
110+
// then find the first visible item
111+
} else if ( this._hasScroll() ) {
112+
menuTop = menu.offset().top;
100113
menu.children().each(function() {
101114
var currentItem = $( this );
102-
if ( currentItem.offset().top - menu.offset().top >= 0 ) {
103-
firstItem = currentItem;
115+
if ( currentItem.offset().top - menuTop >= 0 ) {
116+
item = currentItem;
104117
return false;
105118
}
106119
});
107-
} else if ( this.active ) {
108-
firstItem = this.active;
109120
}
110-
this.focus( event, firstItem );
121+
this.focus( event, item );
111122
},
112123
blur: function( event ) {
113124
this._delay(function() {
@@ -121,7 +132,7 @@ $.widget( "ui.menu", {
121132

122133
this.refresh();
123134

124-
// TODO: We probably shouldn't bind to document for each menu.
135+
// Clicks outside of a menu collapse any open menus
125136
this._on( this.document, {
126137
click: function( event ) {
127138
if ( !$( event.target ).closest( ".ui-menu" ).length ) {
@@ -132,7 +143,7 @@ $.widget( "ui.menu", {
132143
},
133144

134145
_destroy: function() {
135-
// destroy (sub)menus
146+
// Destroy (sub)menus
136147
this.element
137148
.removeAttr( "aria-activedescendant" )
138149
.find( ".ui-menu" ).andSelf()
@@ -146,7 +157,7 @@ $.widget( "ui.menu", {
146157
.removeUniqueId()
147158
.show();
148159

149-
// destroy menu items
160+
// Destroy menu items
150161
this.element.find( ".ui-menu-item" )
151162
.removeClass( "ui-menu-item" )
152163
.removeAttr( "role" )
@@ -164,10 +175,10 @@ $.widget( "ui.menu", {
164175
}
165176
});
166177

167-
// destroy menu dividers
178+
// Destroy menu dividers
168179
this.element.find( ".ui-menu-divider" ).removeClass( "ui-menu-divider ui-widget-content" );
169180

170-
// unbind currentEventTarget click event handler
181+
// Unbind currentEventTarget click event handler
171182
this._off( $( currentEventTarget ), "click" );
172183
},
173184

@@ -207,8 +218,6 @@ $.widget( "ui.menu", {
207218
}
208219
break;
209220
case $.ui.keyCode.ENTER:
210-
this._activate( event );
211-
break;
212221
case $.ui.keyCode.SPACE:
213222
this._activate( event );
214223
break;
@@ -242,7 +251,7 @@ $.widget( "ui.menu", {
242251
if ( !match.length ) {
243252
character = String.fromCharCode( event.keyCode );
244253
match = this.activeMenu.children( ".ui-menu-item" ).filter(function() {
245-
return new RegExp( "^" + escape(character), "i" )
254+
return new RegExp( "^" + escape( character ), "i" )
246255
.test( $( this ).children( "a" ).text() );
247256
});
248257
}
@@ -278,7 +287,7 @@ $.widget( "ui.menu", {
278287
},
279288

280289
refresh: function() {
281-
// initialize nested menus
290+
// Initialize nested menus
282291
var menus,
283292
submenus = this.element.find( this.options.menus + ":not(.ui-menu)" )
284293
.addClass( "ui-menu ui-widget ui-widget-content ui-corner-all" )
@@ -289,7 +298,7 @@ $.widget( "ui.menu", {
289298
"aria-expanded": "false"
290299
});
291300

292-
// don't refresh list items that are already adapted
301+
// Don't refresh list items that are already adapted
293302
menus = submenus.add( this.element );
294303

295304
menus.children( ":not( .ui-menu-item ):has( a )" )
@@ -303,22 +312,24 @@ $.widget( "ui.menu", {
303312
role: this._itemRole()
304313
});
305314

306-
// initialize unlinked menu-items containing spaces and/or dashes only as dividers
307-
menus.children( ":not(.ui-menu-item)" ).each( function() {
315+
// Initialize unlinked menu-items containing spaces and/or dashes only as dividers
316+
menus.children( ":not(.ui-menu-item)" ).each(function() {
308317
var item = $( this );
309318
// hyphen, em dash, en dash
310319
if ( !/[^\-—–\s]/.test( item.text() ) ) {
311320
item.addClass( "ui-widget-content ui-menu-divider" );
312321
}
313322
});
314323

315-
// add aria-disabled attribute to any disabled menu item
324+
// Add aria-disabled attribute to any disabled menu item
316325
menus.children( ".ui-state-disabled" ).attr( "aria-disabled", "true" );
317326

318327
submenus.each(function() {
319328
var menu = $( this ),
320329
item = menu.prev( "a" ),
321-
submenuCarat = $( '<span class="ui-menu-icon ui-icon ui-icon-carat-1-e"></span>' ).data( "ui-menu-submenu-carat", true );
330+
submenuCarat = $( "<span>" )
331+
.addClass( "ui-menu-icon ui-icon ui-icon-carat-1-e" )
332+
.data( "ui-menu-submenu-carat", true );
322333

323334
item
324335
.attr( "aria-haspopup", "true" )
@@ -342,13 +353,13 @@ $.widget( "ui.menu", {
342353

343354
this.active = item.first();
344355
focused = this.active.children( "a" ).addClass( "ui-state-focus" );
345-
// only update aria-activedescendant if there's a role
356+
// Only update aria-activedescendant if there's a role
346357
// otherwise we assume focus is managed elsewhere
347358
if ( this.options.role ) {
348359
this.element.attr( "aria-activedescendant", focused.attr( "id" ) );
349360
}
350361

351-
// highlight active parent menu item, if any
362+
// Highlight active parent menu item, if any
352363
this.active
353364
.parent()
354365
.closest( ".ui-menu-item" )
@@ -363,7 +374,7 @@ $.widget( "ui.menu", {
363374
}, this.delay );
364375
}
365376

366-
nested = $( "> .ui-menu", item );
377+
nested = item.children( ".ui-menu" );
367378
if ( nested.length && ( /^mouse/.test( event.type ) ) ) {
368379
this._startOpening(nested);
369380
}
@@ -429,7 +440,7 @@ $.widget( "ui.menu", {
429440
);
430441

431442
clearTimeout( this.timer );
432-
this.element.find( ".ui-menu" ).not( submenu.parents() )
443+
this.element.find( ".ui-menu" ).not( submenu.parents( ".ui-menu" ) )
433444
.hide()
434445
.attr( "aria-hidden", "true" );
435446

@@ -443,11 +454,11 @@ $.widget( "ui.menu", {
443454
collapseAll: function( event, all ) {
444455
clearTimeout( this.timer );
445456
this.timer = this._delay(function() {
446-
// if we were passed an event, look for the submenu that contains the event
457+
// If we were passed an event, look for the submenu that contains the event
447458
var currentMenu = all ? this.element :
448459
$( event && event.target ).closest( this.element.find( ".ui-menu" ) );
449460

450-
// if we found no valid submenu ancestor, use the main menu to close all sub menus anyway
461+
// If we found no valid submenu ancestor, use the main menu to close all sub menus anyway
451462
if ( !currentMenu.length ) {
452463
currentMenu = this.element;
453464
}
@@ -496,7 +507,7 @@ $.widget( "ui.menu", {
496507
if ( newItem && newItem.length ) {
497508
this._open( newItem.parent() );
498509

499-
// timeout so Firefox will not hide activedescendant change in expanding submenu from AT
510+
// Delay so Firefox will not hide activedescendant change in expanding submenu from AT
500511
this._delay(function() {
501512
this.focus( event, newItem );
502513
}, 20 );
@@ -541,47 +552,48 @@ $.widget( "ui.menu", {
541552
},
542553

543554
nextPage: function( event ) {
555+
var item, base, height;
556+
544557
if ( !this.active ) {
545-
this._move( "next", "first", event );
558+
this.next( event );
546559
return;
547560
}
548561
if ( this.isLastItem() ) {
549562
return;
550563
}
551564
if ( this._hasScroll() ) {
552-
var base = this.active.offset().top,
553-
height = this.element.height(),
554-
result;
565+
base = this.active.offset().top;
566+
height = this.element.height();
555567
this.active.nextAll( ".ui-menu-item" ).each(function() {
556-
result = $( this );
557-
return $( this ).offset().top - base - height < 0;
568+
item = $( this );
569+
return item.offset().top - base - height < 0;
558570
});
559571

560-
this.focus( event, result );
572+
this.focus( event, item );
561573
} else {
562574
this.focus( event, this.activeMenu.children( ".ui-menu-item" )
563575
[ !this.active ? "first" : "last" ]() );
564576
}
565577
},
566578

567579
previousPage: function( event ) {
580+
var item, base, height;
568581
if ( !this.active ) {
569-
this._move( "next", "first", event );
582+
this.next( event );
570583
return;
571584
}
572585
if ( this.isFirstItem() ) {
573586
return;
574587
}
575588
if ( this._hasScroll() ) {
576-
var base = this.active.offset().top,
577-
height = this.element.height(),
578-
result;
589+
base = this.active.offset().top;
590+
height = this.element.height();
579591
this.active.prevAll( ".ui-menu-item" ).each(function() {
580-
result = $( this );
581-
return $(this).offset().top - base + height > 0;
592+
item = $( this );
593+
return item.offset().top - base + height > 0;
582594
});
583595

584-
this.focus( event, result );
596+
this.focus( event, item );
585597
} else {
586598
this.focus( event, this.activeMenu.children( ".ui-menu-item" ).first() );
587599
}
@@ -592,7 +604,7 @@ $.widget( "ui.menu", {
592604
},
593605

594606
select: function( event ) {
595-
// save active reference before collapseAll triggers blur
607+
// Save active reference before collapseAll triggers blur
596608
var ui = {
597609
item: this.active
598610
};

0 commit comments

Comments
 (0)