Skip to content

Commit 9fe3a62

Browse files
committed
Dialog: Move array notation support for position option to backCompat check. Fixes #8824 - Deprecate array notation for position option.
1 parent 6dccd91 commit 9fe3a62

File tree

7 files changed

+147
-43
lines changed

7 files changed

+147
-43
lines changed

build/tasks/testswarm.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ var versions = {
1515
"Core": "core/core.html",
1616
"Datepicker": "datepicker/datepicker.html",
1717
"Dialog": "dialog/dialog.html",
18+
"Dialog_deprecated": "dialog/dialog_deprecated.html",
1819
"Draggable": "draggable/draggable.html",
1920
"Droppable": "droppable/droppable.html",
2021
"Effects": "effects/effects.html",

tests/unit/all.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
"core/core.html",
2323
"datepicker/datepicker.html",
2424
"dialog/dialog.html",
25+
"dialog/dialog_deprecated.html",
2526
"draggable/draggable.html",
2627
"droppable/droppable.html",
2728
"effects/effects.html",

tests/unit/dialog/dialog.html

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
<title>jQuery UI Dialog Test Suite</title>
66

77
<script src="../../jquery.js"></script>
8+
<script>
9+
$.uiBackCompat = false;
10+
</script>
811
<link rel="stylesheet" href="../../../external/qunit.css">
912
<script src="../../../external/qunit.js"></script>
1013
<script src="../../jquery.simulate.js"></script>
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
<!doctype html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="utf-8">
5+
<title>jQuery UI Dialog Test Suite</title>
6+
7+
<script src="../../jquery.js"></script>
8+
<link rel="stylesheet" href="../../../external/qunit.css">
9+
<script src="../../../external/qunit.js"></script>
10+
<script src="../../jquery.simulate.js"></script>
11+
<script src="../testsuite.js"></script>
12+
<script>
13+
TestHelpers.loadResources({
14+
css: [ "ui.core", "ui.dialog" ],
15+
js: [
16+
"ui/jquery.ui.core.js",
17+
"ui/jquery.ui.widget.js",
18+
"ui/jquery.ui.position.js",
19+
"ui/jquery.ui.mouse.js",
20+
"ui/jquery.ui.draggable.js",
21+
"ui/jquery.ui.resizable.js",
22+
"ui/jquery.ui.button.js",
23+
"ui/jquery.ui.dialog.js"
24+
]
25+
});
26+
</script>
27+
28+
<script src="dialog_common.js"></script>
29+
<script src="dialog_core.js"></script>
30+
<script src="dialog_events.js"></script>
31+
<script src="dialog_methods.js"></script>
32+
<script src="dialog_options.js"></script>
33+
<script src="dialog_test_helpers.js"></script>
34+
<script src="dialog_tickets.js"></script>
35+
<script src="dialog_deprecated.js"></script>
36+
37+
<script src="../swarminject.js"></script>
38+
</head>
39+
<body>
40+
41+
<h1 id="qunit-header">jQuery UI Dialog Test Suite</h1>
42+
<h2 id="qunit-banner"></h2>
43+
<div id="qunit-testrunner-toolbar"></div>
44+
<h2 id="qunit-userAgent"></h2>
45+
<ol id="qunit-tests"></ol>
46+
<div id="qunit-fixture">
47+
<div id="dialog1"></div>
48+
<div id="dialog2"></div>
49+
<div id="form-dialog" title="Profile Information">
50+
<fieldset>
51+
<legend>Please share some personal information</legend>
52+
<label for="favorite-animal">Your favorite animal</label><input id="favorite-animal">
53+
<label for="favorite-color">Your favorite color</label><input id="favorite-color">
54+
</fieldset>
55+
<div role="group" aria-describedby="section2">
56+
<p id="section2">Some more (optional) information</p>
57+
<label for="favorite-food">Favorite food</label><input id="favorite-food">
58+
</div>
59+
</div>
60+
</div>
61+
</body>
62+
</html>
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
module("dialog (deprecated): position opton with array");
2+
3+
if ( !$.ui.ie ) {
4+
test("position, right bottom on window w/array", function() {
5+
expect( 2 );
6+
var el = $('<div></div>').dialog({ position: ["right", "bottom"] }),
7+
dialog = el.dialog('widget'),
8+
offset = dialog.offset();
9+
closeEnough(offset.left, $(window).width() - dialog.outerWidth() + $(window).scrollLeft(), 1);
10+
closeEnough(offset.top, $(window).height() - dialog.outerHeight() + $(window).scrollTop(), 1);
11+
el.remove();
12+
});
13+
}
14+
15+
test("position, offset from top left w/array", function() {
16+
expect( 2 );
17+
var el = $('<div></div>').dialog({ position: [10, 10] }),
18+
dialog = el.dialog('widget'),
19+
offset = dialog.offset();
20+
closeEnough(offset.left, 10 + $(window).scrollLeft(), 1);
21+
closeEnough(offset.top, 10 + $(window).scrollTop(), 1);
22+
el.remove();
23+
});

tests/unit/dialog/dialog_options.js

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -331,16 +331,6 @@ if ( !$.ui.ie ) {
331331
el.remove();
332332
});
333333

334-
test("position, right bottom on window w/array", function() {
335-
expect( 2 );
336-
var el = $('<div></div>').dialog({ position: ["right", "bottom"] }),
337-
dialog = el.dialog('widget'),
338-
offset = dialog.offset();
339-
closeEnough(offset.left, $(window).width() - dialog.outerWidth() + $(window).scrollLeft(), 1);
340-
closeEnough(offset.top, $(window).height() - dialog.outerHeight() + $(window).scrollTop(), 1);
341-
el.remove();
342-
});
343-
344334
test("position, right bottom at right bottom via ui.position args", function() {
345335
expect( 2 );
346336
var el = $('<div></div>').dialog({
@@ -359,16 +349,6 @@ if ( !$.ui.ie ) {
359349

360350
}
361351

362-
test("position, offset from top left w/array", function() {
363-
expect( 2 );
364-
var el = $('<div></div>').dialog({ position: [10, 10] }),
365-
dialog = el.dialog('widget'),
366-
offset = dialog.offset();
367-
closeEnough(offset.left, 10 + $(window).scrollLeft(), 1);
368-
closeEnough(offset.top, 10 + $(window).scrollTop(), 1);
369-
el.remove();
370-
});
371-
372352
test("position, at another element", function() {
373353
expect( 4 );
374354
var parent = $('<div></div>').css({

ui/jquery.ui.dialog.js

Lines changed: 57 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ $.widget("ui.dialog", {
211211
this.opener = $( this.document[ 0 ].activeElement );
212212

213213
this._size();
214-
this._position( this.options.position );
214+
this._position();
215215
if ( this.options.modal ) {
216216
this.overlay = new $.ui.dialog.overlay( this );
217217
}
@@ -500,38 +500,24 @@ $.widget("ui.dialog", {
500500
}
501501
},
502502

503-
_position: function( position ) {
504-
var myAt = [],
505-
offset = [ 0, 0 ],
503+
_position: function() {
504+
var position = this.options.position,
505+
myAt = [],
506506
isVisible;
507507

508508
if ( position ) {
509-
// TODO we don't support 1.3.2 anymore, clean this mess up
510-
// deep extending converts arrays to objects in jQuery <= 1.3.2 :-(
511-
// if (typeof position == 'string' || $.isArray(position)) {
512-
// myAt = $.isArray(position) ? position : position.split(' ');
513-
514-
if ( typeof position === "string" || (typeof position === "object" && "0" in position ) ) {
515-
myAt = position.split ? position.split( " " ) : [ position[ 0 ], position[ 1 ] ];
509+
if ( typeof position === "string" ) {
510+
myAt = position.split( " " );
516511
if ( myAt.length === 1 ) {
517512
myAt[ 1 ] = myAt[ 0 ];
518513
}
519-
520-
$.each( [ "left", "top" ], function( i, offsetPosition ) {
521-
if ( +myAt[ i ] === myAt[ i ] ) {
522-
offset[ i ] = myAt[ i ];
523-
myAt[ i ] = offsetPosition;
524-
}
525-
});
526-
527514
position = {
528-
my: myAt[0] + (offset[0] < 0 ? offset[0] : "+" + offset[0]) + " " +
529-
myAt[1] + (offset[1] < 0 ? offset[1] : "+" + offset[1]),
515+
my: myAt[0] + " " + myAt[1],
530516
at: myAt.join( " " )
531517
};
518+
position = $.extend( {}, $.ui.dialog.prototype.options.position, position );
532519
}
533520

534-
position = $.extend( {}, $.ui.dialog.prototype.options.position, position );
535521
} else {
536522
position = $.ui.dialog.prototype.options.position;
537523
}
@@ -610,7 +596,7 @@ $.widget("ui.dialog", {
610596
}
611597

612598
if ( key === "position" ) {
613-
this._position( value );
599+
this._position();
614600
}
615601

616602
if ( key === "resizable" ) {
@@ -744,4 +730,52 @@ $.extend( $.ui.dialog.overlay.prototype, {
744730
}
745731
});
746732

733+
// DEPRECATED
734+
if ( $.uiBackCompat !== false ) {
735+
// position option with array notation
736+
// just override with old implementation
737+
$.ui.dialog.prototype._position = function() {
738+
var position = this.options.position,
739+
myAt = [],
740+
offset = [ 0, 0 ],
741+
isVisible;
742+
743+
if ( position ) {
744+
if ( typeof position === "string" || (typeof position === "object" && "0" in position ) ) {
745+
myAt = position.split ? position.split( " " ) : [ position[ 0 ], position[ 1 ] ];
746+
if ( myAt.length === 1 ) {
747+
myAt[ 1 ] = myAt[ 0 ];
748+
}
749+
750+
$.each( [ "left", "top" ], function( i, offsetPosition ) {
751+
if ( +myAt[ i ] === myAt[ i ] ) {
752+
offset[ i ] = myAt[ i ];
753+
myAt[ i ] = offsetPosition;
754+
}
755+
});
756+
757+
position = {
758+
my: myAt[0] + (offset[0] < 0 ? offset[0] : "+" + offset[0]) + " " +
759+
myAt[1] + (offset[1] < 0 ? offset[1] : "+" + offset[1]),
760+
at: myAt.join( " " )
761+
};
762+
}
763+
764+
position = $.extend( {}, $.ui.dialog.prototype.options.position, position );
765+
} else {
766+
position = $.ui.dialog.prototype.options.position;
767+
}
768+
769+
// need to show the dialog to get the actual offset in the position plugin
770+
isVisible = this.uiDialog.is( ":visible" );
771+
if ( !isVisible ) {
772+
this.uiDialog.show();
773+
}
774+
this.uiDialog.position( position );
775+
if ( !isVisible ) {
776+
this.uiDialog.hide();
777+
}
778+
};
779+
}
780+
747781
}( jQuery ) );

0 commit comments

Comments
 (0)