Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

Menu: Cleanup.

  • Loading branch information...
commit 378dacfda142a7f4681e79601924a47c362dcee1 1 parent 4f3c1e9
Scott González authored

Showing 1 changed file with 58 additions and 46 deletions. Show diff stats Hide diff stats

  1. 104  ui/jquery.ui.menu.js
104  ui/jquery.ui.menu.js
@@ -11,7 +11,7 @@
11 11
  *	jquery.ui.core.js
12 12
  *	jquery.ui.widget.js
13 13
  */
14  
-(function($) {
  14
+(function( $, undefined ) {
15 15
 
16 16
 var currentEventTarget = null;
17 17
 
@@ -32,6 +32,7 @@ $.widget( "ui.menu", {
32 32
 		focus: null,
33 33
 		select: null
34 34
 	},
  35
+
35 36
 	_create: function() {
36 37
 		this.activeMenu = this.element;
37 38
 		this.element
@@ -69,7 +70,10 @@ $.widget( "ui.menu", {
69 70
 				var target = $( event.target );
70 71
 				if ( target[0] !== currentEventTarget ) {
71 72
 					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 ) {
73 77
 						currentEventTarget = null;
74 78
 					});
75 79
 					// Don't select disabled menu items
@@ -94,20 +98,27 @@ $.widget( "ui.menu", {
94 98
 			mouseleave: "collapseAll",
95 99
 			"mouseleave .ui-menu": "collapseAll",
96 100
 			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;
100 113
 					menu.children().each(function() {
101 114
 						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;
104 117
 							return false;
105 118
 						}
106 119
 					});
107  
-				} else if ( this.active ) {
108  
-					firstItem = this.active;
109 120
 				}
110  
-				this.focus( event, firstItem );
  121
+				this.focus( event, item );
111 122
 			},
112 123
 			blur: function( event ) {
113 124
 				this._delay(function() {
@@ -121,7 +132,7 @@ $.widget( "ui.menu", {
121 132
 
122 133
 		this.refresh();
123 134
 
124  
-		// TODO: We probably shouldn't bind to document for each menu.
  135
+		// Clicks outside of a menu collapse any open menus
125 136
 		this._on( this.document, {
126 137
 			click: function( event ) {
127 138
 				if ( !$( event.target ).closest( ".ui-menu" ).length ) {
@@ -132,7 +143,7 @@ $.widget( "ui.menu", {
132 143
 	},
133 144
 
134 145
 	_destroy: function() {
135  
-		// destroy (sub)menus
  146
+		// Destroy (sub)menus
136 147
 		this.element
137 148
 			.removeAttr( "aria-activedescendant" )
138 149
 			.find( ".ui-menu" ).andSelf()
@@ -146,7 +157,7 @@ $.widget( "ui.menu", {
146 157
 				.removeUniqueId()
147 158
 				.show();
148 159
 
149  
-		// destroy menu items
  160
+		// Destroy menu items
150 161
 		this.element.find( ".ui-menu-item" )
151 162
 			.removeClass( "ui-menu-item" )
152 163
 			.removeAttr( "role" )
@@ -164,10 +175,10 @@ $.widget( "ui.menu", {
164 175
 					}
165 176
 				});
166 177
 
167  
-		// destroy menu dividers
  178
+		// Destroy menu dividers
168 179
 		this.element.find( ".ui-menu-divider" ).removeClass( "ui-menu-divider ui-widget-content" );
169 180
 
170  
-		// unbind currentEventTarget click event handler
  181
+		// Unbind currentEventTarget click event handler
171 182
 		this._off( $( currentEventTarget ), "click" );
172 183
 	},
173 184
 
@@ -207,8 +218,6 @@ $.widget( "ui.menu", {
207 218
 			}
208 219
 			break;
209 220
 		case $.ui.keyCode.ENTER:
210  
-			this._activate( event );
211  
-			break;
212 221
 		case $.ui.keyCode.SPACE:
213 222
 			this._activate( event );
214 223
 			break;
@@ -242,7 +251,7 @@ $.widget( "ui.menu", {
242 251
 			if ( !match.length ) {
243 252
 				character = String.fromCharCode( event.keyCode );
244 253
 				match = this.activeMenu.children( ".ui-menu-item" ).filter(function() {
245  
-					return new RegExp( "^" + escape(character), "i" )
  254
+					return new RegExp( "^" + escape( character ), "i" )
246 255
 						.test( $( this ).children( "a" ).text() );
247 256
 				});
248 257
 			}
@@ -278,7 +287,7 @@ $.widget( "ui.menu", {
278 287
 	},
279 288
 
280 289
 	refresh: function() {
281  
-		// initialize nested menus
  290
+		// Initialize nested menus
282 291
 		var menus,
283 292
 			submenus = this.element.find( this.options.menus + ":not(.ui-menu)" )
284 293
 				.addClass( "ui-menu ui-widget ui-widget-content ui-corner-all" )
@@ -289,7 +298,7 @@ $.widget( "ui.menu", {
289 298
 					"aria-expanded": "false"
290 299
 				});
291 300
 
292  
-		// don't refresh list items that are already adapted
  301
+		// Don't refresh list items that are already adapted
293 302
 		menus = submenus.add( this.element );
294 303
 
295 304
 		menus.children( ":not( .ui-menu-item ):has( a )" )
@@ -303,8 +312,8 @@ $.widget( "ui.menu", {
303 312
 					role: this._itemRole()
304 313
 				});
305 314
 
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() {
308 317
 			var item = $( this );
309 318
 			// hyphen, em dash, en dash
310 319
 			if ( !/[^\-—–\s]/.test( item.text() ) ) {
@@ -312,13 +321,15 @@ $.widget( "ui.menu", {
312 321
 			}
313 322
 		});
314 323
 
315  
-		// add aria-disabled attribute to any disabled menu item
  324
+		// Add aria-disabled attribute to any disabled menu item
316 325
 		menus.children( ".ui-state-disabled" ).attr( "aria-disabled", "true" );
317 326
 
318 327
 		submenus.each(function() {
319 328
 			var menu = $( this ),
320 329
 				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 );
322 333
 
323 334
 			item
324 335
 				.attr( "aria-haspopup", "true" )
@@ -342,13 +353,13 @@ $.widget( "ui.menu", {
342 353
 
343 354
 		this.active = item.first();
344 355
 		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
346 357
 		// otherwise we assume focus is managed elsewhere
347 358
 		if ( this.options.role ) {
348 359
 			this.element.attr( "aria-activedescendant", focused.attr( "id" ) );
349 360
 		}
350 361
 
351  
-		// highlight active parent menu item, if any
  362
+		// Highlight active parent menu item, if any
352 363
 		this.active
353 364
 			.parent()
354 365
 			.closest( ".ui-menu-item" )
@@ -363,7 +374,7 @@ $.widget( "ui.menu", {
363 374
 			}, this.delay );
364 375
 		}
365 376
 
366  
-		nested = $( "> .ui-menu", item );
  377
+		nested = item.children( ".ui-menu" );
367 378
 		if ( nested.length && ( /^mouse/.test( event.type ) ) ) {
368 379
 			this._startOpening(nested);
369 380
 		}
@@ -429,7 +440,7 @@ $.widget( "ui.menu", {
429 440
 		);
430 441
 
431 442
 		clearTimeout( this.timer );
432  
-		this.element.find( ".ui-menu" ).not( submenu.parents() )
  443
+		this.element.find( ".ui-menu" ).not( submenu.parents( ".ui-menu" ) )
433 444
 			.hide()
434 445
 			.attr( "aria-hidden", "true" );
435 446
 
@@ -443,11 +454,11 @@ $.widget( "ui.menu", {
443 454
 	collapseAll: function( event, all ) {
444 455
 		clearTimeout( this.timer );
445 456
 		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
447 458
 			var currentMenu = all ? this.element :
448 459
 				$( event && event.target ).closest( this.element.find( ".ui-menu" ) );
449 460
 
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
451 462
 			if ( !currentMenu.length ) {
452 463
 				currentMenu = this.element;
453 464
 			}
@@ -496,7 +507,7 @@ $.widget( "ui.menu", {
496 507
 		if ( newItem && newItem.length ) {
497 508
 			this._open( newItem.parent() );
498 509
 
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
500 511
 			this._delay(function() {
501 512
 				this.focus( event, newItem );
502 513
 			}, 20 );
@@ -541,23 +552,24 @@ $.widget( "ui.menu", {
541 552
 	},
542 553
 
543 554
 	nextPage: function( event ) {
  555
+		var item, base, height;
  556
+
544 557
 		if ( !this.active ) {
545  
-			this._move( "next", "first", event );
  558
+			this.next( event );
546 559
 			return;
547 560
 		}
548 561
 		if ( this.isLastItem() ) {
549 562
 			return;
550 563
 		}
551 564
 		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();
555 567
 			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;
558 570
 			});
559 571
 
560  
-			this.focus( event, result );
  572
+			this.focus( event, item );
561 573
 		} else {
562 574
 			this.focus( event, this.activeMenu.children( ".ui-menu-item" )
563 575
 				[ !this.active ? "first" : "last" ]() );
@@ -565,23 +577,23 @@ $.widget( "ui.menu", {
565 577
 	},
566 578
 
567 579
 	previousPage: function( event ) {
  580
+		var item, base, height;
568 581
 		if ( !this.active ) {
569  
-			this._move( "next", "first", event );
  582
+			this.next( event );
570 583
 			return;
571 584
 		}
572 585
 		if ( this.isFirstItem() ) {
573 586
 			return;
574 587
 		}
575 588
 		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();
579 591
 			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;
582 594
 			});
583 595
 
584  
-			this.focus( event, result );
  596
+			this.focus( event, item );
585 597
 		} else {
586 598
 			this.focus( event, this.activeMenu.children( ".ui-menu-item" ).first() );
587 599
 		}
@@ -592,7 +604,7 @@ $.widget( "ui.menu", {
592 604
 	},
593 605
 
594 606
 	select: function( event ) {
595  
-		// save active reference before collapseAll triggers blur
  607
+		// Save active reference before collapseAll triggers blur
596 608
 		var ui = {
597 609
 			item: this.active
598 610
 		};

0 notes on commit 378dacf

Please sign in to comment.
Something went wrong with that request. Please try again.