Skip to content

Commit fd1b4f8

Browse files
jzaeffererjzaefferer
committed
Menubar: Code formatting. A bit of cleanup: Replace self with that;
replace an unnecessary each.
1 parent c7459df commit fd1b4f8

File tree

1 file changed

+153
-135
lines changed

1 file changed

+153
-135
lines changed

‎tests/visual/menu/menubar.js

Lines changed: 153 additions & 135 deletions
Original file line numberDiff line numberDiff line change
@@ -3,242 +3,260 @@
33
*
44
* TODO move to jquery.ui.menubar.js
55
*/
6-
(function($) {
6+
(function( $ ) {
77

8-
// TODO code formatting
9-
$.widget("ui.menubar", {
8+
// TODO when mixing clicking menus and keyboard navigation, focus handling is broken
9+
// there has to be just one item that has tabindex
10+
$.widget( "ui.menubar", {
1011
options: {
1112
buttons: false,
1213
menuIcon: false
1314
},
1415
_create: function() {
15-
var self = this;
16-
var items = this.items = this.element.children("li")
17-
.addClass("ui-menubar-item")
18-
.attr("role", "presentation")
19-
.children("button, a");
20-
items.slice(1).attr("tabIndex", -1);
21-
var o = this.options;
22-
23-
this.element.addClass('ui-menubar ui-widget-header ui-helper-clearfix').attr("role", "menubar");
24-
this._focusable(items);
25-
this._hoverable(items);
26-
// TODO elm is used just once, so the each probably isn't nedded anymore
27-
items.next("ul").each(function(i, elm) {
28-
$(elm).menu({
29-
select: function(event, ui) {
30-
ui.item.parents("ul.ui-menu:last").hide();
31-
self._trigger( "select", event, ui );
32-
self._close();
16+
var that = this;
17+
var items = this.items = this.element.children( "li" )
18+
.addClass( "ui-menubar-item" )
19+
.attr( "role", "presentation" )
20+
.children( "button, a" );
21+
// let only the first item receive focus
22+
items.slice(1).attr( "tabIndex", -1 );
23+
24+
this.element
25+
.addClass( "ui-menubar ui-widget-header ui-helper-clearfix" )
26+
.attr( "role", "menubar" );
27+
this._focusable( items );
28+
this._hoverable( items );
29+
items.next( "ul" )
30+
.menu({
31+
select: function( event, ui ) {
32+
ui.item.parents( "ul.ui-menu:last" ).hide();
33+
that._trigger( "select", event, ui );
34+
that._close();
3335
// TODO what is this targetting? there's probably a better way to access it
3436
$(event.target).prev().focus();
3537
}
36-
}).hide()
37-
.attr("aria-hidden", "true")
38-
.attr("aria-expanded", "false")
39-
.keydown(function(event) {
40-
var menu = $(this);
41-
if (menu.is(":hidden"))
38+
})
39+
.hide()
40+
.attr( "aria-hidden", "true" )
41+
.attr( "aria-expanded", "false" )
42+
.bind( "keydown.menubar", function( event ) {
43+
var menu = $( this );
44+
if ( menu.is( ":hidden" ) )
4245
return;
43-
switch (event.keyCode) {
46+
switch ( event.keyCode ) {
4447
case $.ui.keyCode.LEFT:
45-
self._left(event);
48+
that._left( event );
4649
event.preventDefault();
4750
break;
4851
case $.ui.keyCode.RIGHT:
49-
self._right(event);
52+
that._right( event );
5053
event.preventDefault();
5154
break;
5255
};
53-
})
54-
});
56+
});
5557
items.each(function() {
5658
var input = $(this),
57-
menu = input.next("ul");
59+
// TODO menu var is only used on two places, doesn't quite justify the .each
60+
menu = input.next( "ul" );
5861

59-
input.bind("click focus mouseenter", function(event) {
62+
input.bind( "click.menubar focus.menubar mouseenter.menubar", function( event ) {
6063
// ignore triggered focus event
61-
if (event.type == "focus" && !event.originalEvent) {
64+
if ( event.type == "focus" && !event.originalEvent ) {
6265
return;
6366
}
6467
event.preventDefault();
6568
// TODO can we simplify or extractthis check? especially the last two expressions
6669
// there's a similar active[0] == menu[0] check in _open
67-
if (event.type == "click" && menu.is(":visible") && self.active && self.active[0] == menu[0]) {
68-
self._close();
70+
if ( event.type == "click" && menu.is( ":visible" ) && that.active && that.active[0] == menu[0] ) {
71+
that._close();
6972
return;
7073
}
71-
if ((self.open && event.type == "mouseenter") || event.type == "click") {
72-
self._open(event, menu);
73-
}
74+
if ( ( that.open && event.type == "mouseenter" ) || event.type == "click" ) {
75+
that._open( event, menu );
76+
}
7477
})
7578
.bind( "keydown", function( event ) {
7679
switch ( event.keyCode ) {
7780
case $.ui.keyCode.SPACE:
7881
case $.ui.keyCode.UP:
7982
case $.ui.keyCode.DOWN:
80-
self._open( event, $( this ).next() );
83+
that._open( event, $( this ).next() );
8184
event.preventDefault();
8285
break;
8386
case $.ui.keyCode.LEFT:
84-
self._prev( event, $( this ) );
87+
that._prev( event, $( this ) );
8588
event.preventDefault();
8689
break;
8790
case $.ui.keyCode.RIGHT:
88-
self._next( event, $( this ) );
91+
that._next( event, $( this ) );
8992
event.preventDefault();
9093
break;
9194
}
9295
})
93-
.addClass("ui-button ui-widget ui-button-text-only ui-menubar-link")
94-
.attr("role", "menuitem")
95-
.attr("aria-haspopup", "true")
96-
.wrapInner("<span class='ui-button-text'></span>");
96+
.addClass( "ui-button ui-widget ui-button-text-only ui-menubar-link" )
97+
.attr( "role", "menuitem" )
98+
.attr( "aria-haspopup", "true" )
99+
.wrapInner( "<span class='ui-button-text'></span>" );
97100

98101
// TODO review if these options are a good choice, maybe they can be merged
99-
if (o.menuIcon) {
100-
input.addClass("ui-state-default").append("<span class='ui-button-icon-secondary ui-icon ui-icon-triangle-1-s'></span>");
101-
input.removeClass("ui-button-text-only").addClass("ui-button-text-icon-secondary");
102+
if ( that.options.menuIcon ) {
103+
input.addClass( "ui-state-default" ).append( "<span class='ui-button-icon-secondary ui-icon ui-icon-triangle-1-s'></span>" );
104+
input.removeClass( "ui-button-text-only" ).addClass( "ui-button-text-icon-secondary" );
102105
}
103106

104-
if (!o.buttons) {
107+
if ( !that.options.buttons ) {
105108
// TODO ui-menubar-link is added above, not needed here?
106-
input.addClass('ui-menubar-link').removeClass('ui-state-default');
109+
input.addClass( "ui-menubar-link" ).removeClass( "ui-state-default" );
107110
};
108111

109112
});
110-
self._bind({
111-
keydown: function(event) {
112-
// TODO merge the two ifs
113-
if (event.keyCode == $.ui.keyCode.ESCAPE) {
114-
if (self.active && self.active.menu("left", event) !== true) {
115-
var active = self.active;
116-
self.active.blur();
117-
self._close( event );
118-
active.prev().focus();
119-
}
113+
that._bind( {
114+
keydown: function( event ) {
115+
if ( event.keyCode == $.ui.keyCode.ESCAPE && that.active && that.active.menu( "left", event ) !== true ) {
116+
var active = that.active;
117+
that.active.blur();
118+
that._close( event );
119+
active.prev().focus();
120120
}
121121
},
122-
focusout : function( event ) {
123-
self.closeTimer = setTimeout(function() {
124-
self._close( event );
125-
}, 100);
122+
focusin: function( event ) {
123+
clearTimeout( that.closeTimer );
126124
},
127-
// TODO change order, focusin first
128-
focusin :function( event ) {
129-
clearTimeout(self.closeTimer);
125+
focusout: function( event ) {
126+
that.closeTimer = setTimeout( function() {
127+
that._close( event );
128+
}, 100);
130129
}
131130
});
132131
},
133132

134133
_destroy : function() {
135-
var items = this.element.children("li")
136-
.removeClass("ui-menubar-item")
137-
.removeAttr("role", "presentation")
138-
.children("button, a");
134+
var items = this.element.children( "li" )
135+
.removeClass( "ui-menubar-item" )
136+
.removeAttr( "role", "presentation" )
137+
.children( "button, a" );
139138

140-
this.element.removeClass('ui-menubar ui-widget-header ui-helper-clearfix').removeAttr("role", "menubar").unbind(".menubar");;
141-
items.unbind("focusin focusout click focus mouseenter keydown");
139+
this.element
140+
.removeClass( "ui-menubar ui-widget-header ui-helper-clearfix" )
141+
.removeAttr( "role", "menubar" )
142+
.unbind( ".menubar" );
142143

143144
items
144-
.removeClass("ui-button ui-widget ui-button-text-only ui-menubar-link ui-state-default")
145-
.removeAttr("role", "menuitem")
146-
.removeAttr("aria-haspopup", "true")
147-
.children("span.ui-button-text").each(function(i, e) {
148-
var item = $(this);
149-
item.parent().html(item.html());
145+
.unbind( ".menubar" )
146+
.removeClass( "ui-button ui-widget ui-button-text-only ui-menubar-link ui-state-default" )
147+
.removeAttr( "role", "menuitem" )
148+
.removeAttr( "aria-haspopup", "true" )
149+
// TODO unwrap?
150+
.children( "span.ui-button-text" ).each(function( i, e ) {
151+
var item = $( this );
152+
item.parent().html( item.html() );
150153
})
151154
.end()
152-
.children(".ui-icon").remove();
155+
.children( ".ui-icon" ).remove();
153156

154-
$(document).unbind(".menubar");
155-
156-
this.element.find(":ui-menu").menu("destroy")
157-
.show()
158-
.removeAttr("aria-hidden", "true")
159-
.removeAttr("aria-expanded", "false")
160-
.removeAttr("tabindex")
161-
.unbind("keydown", "blur");
157+
this.element.find( ":ui-menu" )
158+
.menu( "destroy" )
159+
.show()
160+
.removeAttr( "aria-hidden", "true" )
161+
.removeAttr( "aria-expanded", "false" )
162+
.removeAttr( "tabindex" )
163+
.unbind( ".menubar" );
162164
},
163165

164166
_close: function() {
165-
if (!this.active || !this.active.length)
167+
if ( !this.active || !this.active.length )
166168
return;
167-
this.active.menu("closeAll").hide().attr("aria-hidden", "true").attr("aria-expanded", "false");
168-
this.active.prev().removeClass("ui-state-active").removeAttr("tabIndex");
169+
this.active
170+
.menu( "closeAll" )
171+
.hide()
172+
.attr( "aria-hidden", "true" )
173+
.attr( "aria-expanded", "false" );
174+
this.active
175+
.prev()
176+
.removeClass( "ui-state-active" )
177+
.removeAttr( "tabIndex" );
169178
this.active = null;
170179
this.open = false;
171180
},
172181

173-
_open: function(event, menu) {
182+
_open: function( event, menu ) {
174183
// on a single-button menubar, ignore reopening the same menu
175-
if (this.active && this.active[0] == menu[0]) {
184+
if ( this.active && this.active[0] == menu[0] ) {
176185
return;
177186
}
178-
// almost the same as _close above, but don't remove tabIndex
179-
if (this.active) {
180-
this.active.menu("closeAll").hide().attr("aria-hidden", "true").attr("aria-expanded", "false");
181-
this.active.prev().removeClass("ui-state-active");
187+
// TODO refactor, almost the same as _close above, but don't remove tabIndex
188+
if ( this.active ) {
189+
this.active
190+
.menu( "closeAll" )
191+
.hide()
192+
.attr( "aria-hidden", "true" )
193+
.attr( "aria-expanded", "false" );
194+
this.active
195+
.prev()
196+
.removeClass( "ui-state-active" );
182197
}
183198
// set tabIndex -1 to have the button skipped on shift-tab when menu is open (it gets focus)
184-
var button = menu.prev().addClass("ui-state-active").attr("tabIndex", -1);
185-
this.active = menu.show().position({
186-
my: "left top",
187-
at: "left bottom",
188-
of: button
189-
})
190-
.removeAttr("aria-hidden").attr("aria-expanded", "true")
191-
.menu("focus", event, menu.children("li").first())
192-
// TODO need a comment here why both events are triggered
193-
.focus()
194-
.focusin();
199+
var button = menu.prev().addClass( "ui-state-active" ).attr( "tabIndex", -1 );
200+
this.active = menu
201+
.show()
202+
.position( {
203+
my: "left top",
204+
at: "left bottom",
205+
of: button
206+
})
207+
.removeAttr( "aria-hidden" )
208+
.attr( "aria-expanded", "true" )
209+
.menu("focus", event, menu.children( "li" ).first() )
210+
// TODO need a comment here why both events are triggered
211+
.focus()
212+
.focusin();
195213
this.open = true;
196214
},
197215

198216
// TODO refactor this and the next three methods
199217
_prev: function( event, button ) {
200-
button.attr("tabIndex", -1);
201-
var prev = button.parent().prevAll("li").children( ".ui-button" ).eq( 0 );
202-
if (prev.length) {
203-
prev.removeAttr("tabIndex")[0].focus();
218+
button.attr( "tabIndex", -1 );
219+
var prev = button.parent().prevAll( "li" ).children( ".ui-button" ).eq( 0 );
220+
if ( prev.length ) {
221+
prev.removeAttr( "tabIndex" )[0].focus();
204222
} else {
205-
var lastItem = this.element.children("li:last").children(".ui-button:last");
206-
lastItem.removeAttr("tabIndex")[0].focus();
223+
var lastItem = this.element.children( "li:last" ).children( ".ui-button:last" );
224+
lastItem.removeAttr( "tabIndex" )[0].focus();
207225
}
208226
},
209227

210228
_next: function( event, button ) {
211-
button.attr("tabIndex", -1);
212-
var next = button.parent().nextAll("li").children( ".ui-button" ).eq( 0 );
213-
if (next.length) {
214-
next.removeAttr("tabIndex")[0].focus();
229+
button.attr( "tabIndex", -1 );
230+
var next = button.parent().nextAll( "li" ).children( ".ui-button" ).eq( 0 );
231+
if ( next.length ) {
232+
next.removeAttr( "tabIndex")[0].focus();
215233
} else {
216-
var firstItem = this.element.children("li:first").children(".ui-button:first");
217-
firstItem.removeAttr("tabIndex")[0].focus();
234+
var firstItem = this.element.children( "li:first" ).children( ".ui-button:first" );
235+
firstItem.removeAttr( "tabIndex" )[0].focus();
218236
}
219237
},
220238

221239
// TODO rename to parent
222-
_left: function(event) {
223-
var prev = this.active.parent().prevAll("li:eq(0)").children( ".ui-menu" ).eq( 0 );
224-
if (prev.length) {
225-
this._open(event, prev);
240+
_left: function( event ) {
241+
var prev = this.active.parent().prevAll( "li:eq(0)" ).children( ".ui-menu" ).eq( 0 );
242+
if ( prev.length ) {
243+
this._open( event, prev );
226244
} else {
227-
var lastItem = this.element.children("li:last").children(".ui-menu:first");
228-
this._open(event, lastItem);
245+
var lastItem = this.element.children( "li:last" ).children( ".ui-menu:first" );
246+
this._open( event, lastItem );
229247
}
230248
},
231249

232250
// TODO rename to child (or something like that)
233-
_right: function(event) {
234-
var next = this.active.parent().nextAll("li:eq(0)").children( ".ui-menu" ).eq( 0 );
235-
if (next.length) {
236-
this._open(event, next);
251+
_right: function( event ) {
252+
var next = this.active.parent().nextAll( "li:eq(0)" ).children( ".ui-menu" ).eq( 0 );
253+
if ( next.length ) {
254+
this._open( event, next );
237255
} else {
238-
var firstItem = this.element.children("li:first").children(".ui-menu:first");
239-
this._open(event, firstItem);
256+
var firstItem = this.element.children( "li:first" ).children( ".ui-menu:first" );
257+
this._open( event, firstItem );
240258
}
241259
}
242260
});
243261

244-
}(jQuery));
262+
}( jQuery ));

0 commit comments

Comments
 (0)