Skip to content

Commit

Permalink
Position: Added a check for fraction support in element positions. Fi…
Browse files Browse the repository at this point in the history
…xes #7255 - Position: Revisit solution for off-by-1 errors.
  • Loading branch information
kborchers authored and scottgonzalez committed Oct 26, 2011
1 parent 757384b commit bfbc0b1
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 20 deletions.
34 changes: 17 additions & 17 deletions tests/unit/position/position_core.js
Expand Up @@ -406,22 +406,22 @@ test("collision: flip, with margin", function() {
}, { top: 0, left: 0 }, "right bottom");
});

//test('bug #5280: consistent results (avoid fractional values)', function() {
// var wrapper = $('#bug-5280'),
// elem = wrapper.children(),
// offset1 = elem.position({
// my: 'center',
// at: 'center',
// of: wrapper,
// collision: 'none'
// }).offset(),
// offset2 = elem.position({
// my: 'center',
// at: 'center',
// of: wrapper,
// collision: 'none'
// }).offset();
// same(offset1, offset2);
//});
test('bug #5280: consistent results (avoid fractional values)', function() {
var wrapper = $('#bug-5280'),
elem = wrapper.children(),
offset1 = elem.position({
my: 'center',
at: 'center',
of: wrapper,
collision: 'none'
}).offset(),
offset2 = elem.position({
my: 'center',
at: 'center',
of: wrapper,
collision: 'none'
}).offset();
same(offset1, offset2);
});

})(jQuery);
52 changes: 49 additions & 3 deletions ui/jquery.ui.position.js
Expand Up @@ -14,6 +14,7 @@ $.ui = $.ui || {};
var horizontalPositions = /left|center|right/,
verticalPositions = /top|center|bottom/,
center = "center",
support = {},
_position = $.fn.position,
_offset = $.fn.offset;

Expand Down Expand Up @@ -121,9 +122,11 @@ $.fn.position = function( options ) {
position.top -= elemHeight / 2;
}

// prevent fractions (see #5280)
position.left = Math.round( position.left );
position.top = Math.round( position.top );
// prevent fractions if jQuery version doesn't support them (see #5280)
if ( !support.fractions ) {
position.left = Math.round( position.left );
position.top = Math.round( position.top );
}

collisionPosition = {
left: position.left - marginLeft,
Expand Down Expand Up @@ -249,4 +252,47 @@ if ( !$.offset.setOffset ) {
};
}

// fraction support test (older versions of jQuery don't support fractions)
(function () {
var body = document.getElementsByTagName( "body" )[ 0 ],
div = document.createElement( "div" ),
testElement, testElementParent, testElementStyle, offset, offsetTotal;

//Create a "fake body" for testing based on method used in jQuery.support
testElement = document.createElement( body ? "div" : "body" );
testElementStyle = {
visibility: "hidden",
width: 0,
height: 0,
border: 0,
margin: 0,
background: "none"
};
if ( body ) {
jQuery.extend( testElementStyle, {
position: "absolute",
left: "-1000px",
top: "-1000px"
});
}
for ( var i in testElementStyle ) {
testElement.style[ i ] = testElementStyle[ i ];
}
testElement.appendChild( div );
testElementParent = body || document.documentElement;
testElementParent.insertBefore( testElement, testElementParent.firstChild );

div.style.cssText = "position: absolute; left: 10.7432222px; top: 10.432325px; height: 30px; width: 201px;";

offset = $( div ).offset( function( _, offset ) {
return offset;
}).offset();

This comment has been minimized.

Copy link
@SeanMcMillan

SeanMcMillan Apr 23, 2012

Can anyone explain why it's calling .offset(function(_ offset) {return offset}).offset()? Master is calling .offset().left, without this hoop-jumping.

(I ask because I'd love to see #8254 fixed in this branch. Yes, I have an app that is still stuck on 1.3.2...)

This comment has been minimized.

Copy link
@scottgonzalez

This comment has been minimized.

Copy link
@SeanMcMillan

SeanMcMillan Apr 23, 2012

Thank you. my github-fu was not strong enough to find that. :-(

testElement.innerHTML = "";
testElementParent.removeChild( testElement );

offsetTotal = offset.top + offset.left + ( body ? 2000 : 0 );
support.fractions = offsetTotal > 21 && offsetTotal < 22;
})();

}( jQuery ));

1 comment on commit bfbc0b1

@scottgonzalez
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes the dialog to jump when resizing. Open the default dialog demo, resize from the bottom right and notice the top left of the dialog will move.

Please sign in to comment.