Skip to content

Commit 6d01873

Browse files
hanshillenhanshillen
committed
Fix various pull request comments and coding standards issues
1 parent ab2bbd3 commit 6d01873

File tree

2 files changed

+73
-81
lines changed

2 files changed

+73
-81
lines changed

‎demos/popup/default.html

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
<style type="text/css">
3030
.ui-popup { position: absolute; z-index: 5000; }
3131
.ui-menu { width: 200px; }
32-
32+
3333
/*
3434
table {
3535
border-collapse: collapse;
@@ -54,26 +54,20 @@
5454
<body>
5555

5656
<div class="demo">
57-
<a href="#login-form">Log In</a>
57+
<a href="#login-form">Log In</a>
5858
<div class="ui-widget-content" id="login-form" aria-label="Login options">
59-
<form>
60-
<div>
61-
<label for="un">Username</label>
62-
<input type="text" id="un" />
63-
</div>
64-
<div>
65-
<label for="pw">Password</label>
66-
<input type="password" id="pw" />
67-
</div>
68-
<div>
69-
<input type="submit" value="Login" class="submit" />
70-
</div>
71-
</form>
59+
<div>
60+
<label for="un">Username</label>
61+
<input type="text" id="un" />
62+
</div>
63+
<div>
64+
<label for="pw">Password</label>
65+
<input type="password" id="pw" />
66+
</div>
67+
<div>
68+
<input type="submit" value="Login" class="submit" />
69+
</div>
7270
</div>
73-
74-
75-
76-
7771
</div>
7872

7973
<div class="demo-description">

‎ui/jquery.ui.popup.js

Lines changed: 60 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
* jquery.ui.position.js
1414
*/
1515
(function($) {
16-
16+
1717
var idIncrement = 0;
1818

1919
$.widget( "ui.popup", {
@@ -27,34 +27,34 @@ $.widget( "ui.popup", {
2727
if ( !this.options.trigger ) {
2828
this.options.trigger = this.element.prev();
2929
}
30-
30+
3131
if ( !this.element.attr( "id" ) ) {
3232
this.element.attr( "id", "ui-popup-" + idIncrement++ );
3333
this.generatedId = true;
3434
}
35-
35+
3636
if ( !this.element.attr( "role" ) ) {
3737
// TODO alternatives to tooltip are dialog and menu, all three aren't generic popups
3838
this.element.attr( "role", "dialog" );
3939
this.generatedRole = true;
4040
}
41-
41+
4242
this.options.trigger
4343
.attr( "aria-haspopup", true )
4444
.attr( "aria-owns", this.element.attr( "id" ) );
45-
45+
4646
this.element
47-
.addClass("ui-popup")
47+
.addClass( "ui-popup" )
4848
this.close();
4949

5050
this._bind(this.options.trigger, {
5151
keydown: function( event ) {
52-
// prevent space-to-open to scroll the page, only hapens for anchor ui.button
53-
if ( this.options.trigger.is( "a:ui-button" ) && event.keyCode == $.ui.keyCode.SPACE) {
54-
event.preventDefault()
52+
// prevent space-to-open to scroll the page, only happens for anchor ui.button
53+
if ( this.options.trigger.is( "a:ui-button" ) && event.keyCode == $.ui.keyCode.SPACE ) {
54+
event.preventDefault();
5555
}
5656
// TODO handle SPACE to open popup? only when not handled by ui.button
57-
if ( event.keyCode == $.ui.keyCode.SPACE && this.options.trigger.is("a:not(:ui-button)") ) {
57+
if ( event.keyCode == $.ui.keyCode.SPACE && this.options.trigger.is( "a:not(:ui-button)" ) ) {
5858
this.options.trigger.trigger( "click", event );
5959
}
6060
// translate keydown to click
@@ -78,8 +78,29 @@ $.widget( "ui.popup", {
7878
}, 1);
7979
}
8080
});
81-
82-
this._bind(this.element, {
81+
82+
if ( !this.element.is( ":ui-menu" ) ) {
83+
//default use case, wrap tab order in popup
84+
this.element.bind( "keydown.ui-popup", function( event ) {
85+
if ( event.keyCode !== $.ui.keyCode.TAB ) {
86+
return;
87+
}
88+
89+
var tabbables = $( ":tabbable", this ),
90+
first = tabbables.first(),
91+
last = tabbables.last();
92+
93+
if ( event.target === last[ 0 ] && !event.shiftKey ) {
94+
first.focus( 1 );
95+
return false;
96+
} else if ( event.target === first[ 0 ] && event.shiftKey ) {
97+
last.focus( 1 );
98+
return false;
99+
}
100+
});
101+
}
102+
103+
this._bind({
83104
focusout: function( event ) {
84105
var that = this;
85106
// use a timer to allow click to clear it and letting that
@@ -89,52 +110,52 @@ $.widget( "ui.popup", {
89110
}, 100);
90111
},
91112
focusin: function( event ) {
92-
var that = this;
93-
clearTimeout( that.closeTimer );
94-
}
113+
clearTimeout( this.closeTimer );
114+
}
95115
});
96116

97117
this._bind({
98118
// TODO only triggered on element if it can receive focus
99119
// bind to document instead?
100120
// either element itself or a child should be focusable
101121
keyup: function( event ) {
102-
if (event.keyCode == $.ui.keyCode.ESCAPE && this.element.is( ":visible" )) {
122+
if ( event.keyCode == $.ui.keyCode.ESCAPE && this.element.is( ":visible" ) ) {
103123
this.close( event );
104124
// TODO move this to close()? would allow menu.select to call popup.close, and get focus back to trigger
105125
this.options.trigger.focus();
106126
}
107127
}
108128
});
109-
129+
110130
this._bind(document, {
111131
click: function( event ) {
112-
if (this.isOpen && !$(event.target).closest(".ui-popup").length) {
132+
if ( this.isOpen && !$(event.target).closest(".ui-popup").length ) {
113133
this.close( event );
114134
}
115135
}
116136
})
117137
},
118-
138+
119139
_destroy: function() {
120140
this.element
121141
.show()
122142
.removeClass( "ui-popup" )
123143
.removeAttr( "aria-hidden" )
124-
.removeAttr( "aria-expanded" );
144+
.removeAttr( "aria-expanded" )
145+
.unbind( "keypress.ui-popup");
125146

126147
this.options.trigger
127148
.removeAttr( "aria-haspopup" )
128149
.removeAttr( "aria-owns" );
129-
150+
130151
if ( this.generatedId ) {
131152
this.element.removeAttr( "id" );
132153
}
133154
if ( this.generatedRole ) {
134155
this.element.removeAttr( "role" );
135156
}
136157
},
137-
158+
138159
open: function( event ) {
139160
var position = $.extend( {}, {
140161
of: this.options.trigger
@@ -144,46 +165,26 @@ $.widget( "ui.popup", {
144165
.show()
145166
.attr( "aria-hidden", false )
146167
.attr( "aria-expanded", true )
147-
.position( position );
148-
149-
if (this.element.is(":ui-menu")) { //popup is a menu
150-
this.element.focus();
151-
this.element.menu("focus", event, this.element.children( "li" ).first() );
168+
.position( position );
169+
170+
if (this.element.is( ":ui-menu" )) { //popup is a menu
171+
this.element.menu( "focus", event, this.element.children( "li" ).first() );
152172
this.element.focus();
153-
}
154-
// TODO add other special use cases that differ from the default dialog style keyboard mechanism
155-
else {
156-
//default use case, popup could be anything (e.g. a form)
157-
this.element
158-
.bind( "keydown.ui-popup", function( event ) {
159-
if ( event.keyCode !== $.ui.keyCode.TAB ) {
160-
return;
161-
}
162-
var tabbables = $( ":tabbable", this ),
163-
first = tabbables.filter( ":first" ),
164-
last = tabbables.filter( ":last" );
165-
if ( event.target === last[0] && !event.shiftKey ) {
166-
first.focus( 1 );
167-
return false;
168-
} else if ( event.target === first[0] && event.shiftKey ) {
169-
last.focus( 1 );
170-
return false;
171-
}
172-
});
173-
173+
} else {
174174
// set focus to the first tabbable element in the popup container
175-
// if there are no tabbable elements, set focus on the popup itself
176-
var tabbables = this.element.find( ":tabbable" );
177-
if (!tabbables.length) {
178-
this.element.attr("tabindex", "0");
179-
tabbables.add(this.element);
180-
}
181-
tabbables.eq( 0 ).focus(1);
175+
// if there are no tabbable elements, set focus on the popup itself
176+
var tabbables = this.element.find( ":tabbable" );
177+
if ( !tabbables.length ) {
178+
if ( !this.element.is(":tabbable") ) {
179+
this.element.attr("tabindex", "0");
180+
}
181+
tabbables.add(this.element);
182+
}
183+
tabbables.first().focus( 1 );
182184
}
183-
184-
// take trigger out of tab order to allow shift-tab to skip trigger
185-
this.options.trigger.attr("tabindex", -1);
186185

186+
// take trigger out of tab order to allow shift-tab to skip trigger
187+
this.options.trigger.attr( "tabindex", -1 );
187188
this.isOpen = true;
188189
this._trigger( "open", event );
189190
},
@@ -192,16 +193,13 @@ $.widget( "ui.popup", {
192193
this.element
193194
.hide()
194195
.attr( "aria-hidden", true )
195-
.attr( "aria-expanded", false )
196-
.unbind( "keypress.ui-popup");
196+
.attr( "aria-expanded", false );
197197

198198
this.options.trigger.attr("tabindex", 0);
199199

200200
this.isOpen = false;
201201
this._trigger( "close", event );
202202
}
203-
204-
205203
});
206204

207205
}(jQuery));

0 commit comments

Comments
 (0)