Position: Take margin into account when performing collisions. Fixes …

…#5766 - position: collision should take margin into account.
jquery · Sep 3, 2010 · 4b9d5d1 · 4b9d5d1
1 parent 0a0a39f
commit 4b9d5d1
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 9 deletions.
diff --git a/tests/unit/position/position_core.js b/tests/unit/position/position_core.js
@@ -354,6 +354,58 @@ test("collision: none, with offset", function() {
 	}, { top: -13, left: -12 }, "left top, negative offset");
 });
 
+test("collision: fit, with margin", function() {
+	$("#elx").css("margin", 10);
+
+	collisionTest({
+		collision: "fit"
+	}, { top: $(window).height() - 20, left: $(window).width() - 20 }, "right bottom");
+
+	collisionTest2({
+		collision: "fit"
+	}, { top: 10, left: 10 }, "left top");
+
+	$("#elx").css({
+		"margin-left": 5,
+		"margin-top": 5
+	});
+
+	collisionTest({
+		collision: "fit"
+	}, { top: $(window).height() - 20, left: $(window).width() - 20 }, "right bottom");
+
+	collisionTest2({
+		collision: "fit"
+	}, { top: 5, left: 5 }, "left top");
+
+	$("#elx").css({
+		"margin-right": 15,
+		"margin-bottom": 15
+	});
+
+	collisionTest({
+		collision: "fit"
+	}, { top: $(window).height() - 25, left: $(window).width() - 25 }, "right bottom");
+
+	collisionTest2({
+		collision: "fit"
+	}, { top: 5, left: 5 }, "left top");
+});
+
+test("collision: flip, with margin", function() {
+	$("#elx").css("margin", 10);
+
+	collisionTest({
+		collision: "flip",
+		at: "left top"
+	}, { top: $(window).height() - 10, left: $(window).width() - 10 }, "left top");
+
+	collisionTest2({
+		collision: "flip",
+		at: "right bottom"
+	}, { top: 0, left: 0 }, "right bottom");
+});
+
 //test('bug #5280: consistent results (avoid fractional values)', function() {
 //	var wrapper = $('#bug-5280'),
 //		elem = wrapper.children(),

diff --git a/ui/jquery.ui.position.js b/ui/jquery.ui.position.js
@@ -99,7 +99,14 @@ $.fn.position = function( options ) {
 		var elem = $( this ),
 			elemWidth = elem.outerWidth(),
 			elemHeight = elem.outerHeight(),
-			position = $.extend( {}, basePosition );
+			marginLeft = parseInt( $.curCSS( this, "marginLeft", true ) ) || 0,
+			marginTop = parseInt( $.curCSS( this, "marginTop", true ) ) || 0,
+			collisionWidth = elemWidth + marginLeft +
+				parseInt( $.curCSS( this, "marginRight", true ) ) || 0,
+			collisionHeight = elemHeight + marginTop +
+				parseInt( $.curCSS( this, "marginBottom", true ) ) || 0,
+			position = $.extend( {}, basePosition ),
+			collisionPosition;
 
 		if ( options.my[0] === "right" ) {
 			position.left -= elemWidth;
@@ -117,13 +124,21 @@ $.fn.position = function( options ) {
 		position.left = parseInt( position.left );
 		position.top = parseInt( position.top );
 
+		collisionPosition = {
+			left: position.left - marginLeft,
+			top: position.top - marginTop
+		};
+
 		$.each( [ "left", "top" ], function( i, dir ) {
 			if ( $.ui.position[ collision[i] ] ) {
 				$.ui.position[ collision[i] ][ dir ]( position, {
 					targetWidth: targetWidth,
 					targetHeight: targetHeight,
 					elemWidth: elemWidth,
 					elemHeight: elemHeight,
+					collisionPosition: collisionPosition,
+					collisionWidth: collisionWidth,
+					collisionHeight: collisionHeight,
 					offset: offset,
 					my: options.my,
 					at: options.at
@@ -142,13 +157,13 @@ $.ui.position = {
 	fit: {
 		left: function( position, data ) {
 			var win = $( window ),
-				over = position.left + data.elemWidth - win.width() - win.scrollLeft();
-			position.left = over > 0 ? position.left - over : Math.max( 0, position.left );
+				over = data.collisionPosition.left + data.collisionWidth - win.width() - win.scrollLeft();
+			position.left = over > 0 ? position.left - over : Math.max( position.left - data.collisionPosition.left, position.left );
 		},
 		top: function( position, data ) {
 			var win = $( window ),
-				over = position.top + data.elemHeight - win.height() - win.scrollTop();
-			position.top = over > 0 ? position.top - over : Math.max( 0, position.top );
+				over = data.collisionPosition.top + data.collisionHeight - win.height() - win.scrollTop();
+			position.top = over > 0 ? position.top - over : Math.max( position.top - data.collisionPosition.top, position.top );
 		}
 	},
 
@@ -158,7 +173,7 @@ $.ui.position = {
 				return;
 			}
 			var win = $( window ),
-				over = position.left + data.elemWidth - win.width() - win.scrollLeft(),
+				over = data.collisionPosition.left + data.collisionWidth - win.width() - win.scrollLeft(),
 				myOffset = data.my[ 0 ] === "left" ?
 					-data.elemWidth :
 					data.my[ 0 ] === "right" ?
@@ -168,7 +183,7 @@ $.ui.position = {
 					data.targetWidth :
 					-data.targetWidth,
 				offset = -2 * data.offset[ 0 ];
-			position.left += position.left < 0 ?
+			position.left += data.collisionPosition.left < 0 ?
 				myOffset + atOffset + offset :
 				over > 0 ?
 					myOffset + atOffset + offset :
@@ -179,7 +194,7 @@ $.ui.position = {
 				return;
 			}
 			var win = $( window ),
-				over = position.top + data.elemHeight - win.height() - win.scrollTop(),
+				over = data.collisionPosition.top + data.collisionHeight - win.height() - win.scrollTop(),
 				myOffset = data.my[ 1 ] === "top" ?
 					-data.elemHeight :
 					data.my[ 1 ] === "bottom" ?
@@ -189,7 +204,7 @@ $.ui.position = {
 					data.targetHeight :
 					-data.targetHeight,
 				offset = -2 * data.offset[ 1 ];
-			position.top += position.top < 0 ?
+			position.top += data.collisionPosition.top < 0 ?
 				myOffset + atOffset + offset :
 				over > 0 ?
 					myOffset + atOffset + offset :