Skip to content

Commit

Permalink
Position: Add flip-classes. Fixes #5937 - Position: Add ability to de…
Browse files Browse the repository at this point in the history 
…termine if the element is flipped via css
  • Loading branch information
@bmsterling
bmsterling committed Jul 11, 2011
1 parent 0245b72 commit d5452c0
Show file tree
Hide file tree
Showing 4 changed files with 205 additions and 1 deletion.
12 changes: 12 additions & 0 deletions demos/position/default.html
View file
Expand Up @@ -30,6 +30,18 @@
background-color: #bcd5e6;
text-align: center;
}
div.ui-flipped-top {
border-top: 3px solid #000000;
}
div.ui-flipped-bottom {
border-bottom: 3px solid #000000;
}
div.ui-flipped-left {
border-left: 3px solid #000000;
}
div.ui-flipped-right {
border-right: 3px solid #000000;
}
select, input {
margin-left: 15px;
}
Expand Down
81 changes: 81 additions & 0 deletions tests/unit/position/position_core.js
View file
Expand Up @@ -435,6 +435,87 @@ test( "collision: flip, with margin", function() {
}, { top: 0, left: 0 }, "right bottom" );
});

test( "addClass: flipped left", function() {
var elem = $( "#elx" ).position( {
my: "left center",
of: window,
collision: "flip",
at: "right center"
});

same( elem.hasClass( 'ui-flipped-left' ), true, 'Has ui-flipped-left class' );

elem.position( {
my: "right center",
of: window,
collision: "flip",
at: "left center"
})

same( elem.hasClass( 'ui-flipped-left' ), false, 'Removed ui-flipped-left class' );
});

test( "addClass: flipped top", function() {
var elem = $( "#elx" ).position( {
my: "left top",
of: window,
collision: "flip",
at: "right bottom"
});

same( elem.hasClass( 'ui-flipped-top' ), true, 'Has ui-flipped-top class' );

elem.position( {
my: "left bottom",
of: window,
collision: "flip",
at: "right top"
});

same( elem.hasClass( 'ui-flipped-top' ), false, 'Removed ui-flipped-top class' );
});

test( "addClass: flipped right", function() {
var elem = $( "#elx" ).position( {
my: "right center",
of: window,
collision: "flip",
at: "left center"
});

same( elem.hasClass( 'ui-flipped-right' ), true, 'Has ui-flipped-right class' );

elem.position( {
my: "left center",
of: window,
collision: "flip",
at: "right center"
});

same( elem.hasClass( 'ui-flipped-right' ), false, 'Removed ui-flipped-right class' );

});

test( "addClass: flipped bottom", function() {
var elem = $( "#elx" ).position( {
my: "left bottom",
of: window,
collision: "flip",
at: "right top"
});

same( elem.hasClass( 'ui-flipped-bottom' ), true, 'Has ui-flipped-bottom class' );

elem.position( {
my: "left top",
of: window,
collision: "flip",
at: "right bottom"
});

same( elem.hasClass( 'ui-flipped-bottom' ), false, 'Removed ui-flipped-bottom class' );
});

//test( "bug #5280: consistent results (avoid fractional values)", function() {
// var wrapper = $( "#bug-5280" ),
// elem = wrapper.children(),
Expand Down
95 changes: 95 additions & 0 deletions tests/unit/position/position_core_within.js
View file
Expand Up @@ -438,4 +438,99 @@ test( "collision: flip, with margin", function() {
}, { top: addTop + 0, left: addLeft + 0 }, "right bottom" );
});

test( "addClass: flipped left", function() {
var within = $("#within-container");

var elem = $( "#elx" ).position( {
my: "left center",
of: within[0],
within: within,
collision: "flip",
at: "right center"
});

same( elem.hasClass( 'ui-flipped-left' ), true, 'Has ui-flipped-left class' );

elem.position( {
my: "right center",
of: within[0],
within: within,
collision: "flip",
at: "left center"
})

same( elem.hasClass( 'ui-flipped-left' ), false, 'Removed ui-flipped-left class' );
});

test( "addClass: flipped top", function() {
var within = $("#within-container");

var elem = $( "#elx" ).position( {
my: "left top",
of: within[0],
within: within,
collision: "flip",
at: "right bottom"
});

same( elem.hasClass( 'ui-flipped-top' ), true, 'Has ui-flipped-top class' );

elem.position( {
my: "left bottom",
of: within[0],
within: within,
collision: "flip",
at: "right top"
});

same( elem.hasClass( 'ui-flipped-top' ), false, 'Removed ui-flipped-top class' );
});

test( "addClass: flipped right", function() {
var within = $("#within-container");

var elem = $( "#elx" ).position( {
my: "right center",
of: within[0],
within: within,
collision: "flip",
at: "left center"
});

same( elem.hasClass( 'ui-flipped-right' ), true, 'Has ui-flipped-right class' );

elem.position( {
my: "left center",
of: within[0],
within: within,
collision: "flip",
at: "right center"
});

same( elem.hasClass( 'ui-flipped-right' ), false, 'Removed ui-flipped-right class' );

});

test( "addClass: flipped bottom", function() {
var within = $("#within-container");

var elem = $( "#elx" ).position( {
my: "left bottom",
of: window,
collision: "flip",
at: "right top"
});

same( elem.hasClass( 'ui-flipped-bottom' ), true, 'Has ui-flipped-bottom class' );

elem.position( {
my: "left top",
of: window,
collision: "flip",
at: "right bottom"
});

same( elem.hasClass( 'ui-flipped-bottom' ), false, 'Removed ui-flipped-bottom class' );
});

}( jQuery ) );
18 changes: 17 additions & 1 deletion ui/jquery.ui.position.js
View file
Expand Up @@ -204,7 +204,8 @@ $.fn.position = function( options ) {
offset: [ atOffset[ 0 ] + myOffset[ 0 ], atOffset [ 1 ] + myOffset[ 1 ] ],
my: options.my,
at: options.at,
within: within
within: within,
elem : elem
});
}
});
Expand Down Expand Up @@ -265,6 +266,9 @@ $.ui.position = {
return;
}

data.elem
.removeClass( "ui-flipped-left ui-flipped-right" );

var within = data.within,
win = $( window ),
isWindow = $.isWindow( data.within[0] ),
Expand All @@ -283,13 +287,21 @@ $.ui.position = {
-data.targetWidth,
offset = -2 * data.offset[ 0 ];
if ( overLeft < 0 || overRight > 0 ) {

data.elem
.addClass( "ui-flipped-" + ( overLeft < 0 ? "right" : "left" ) );

position.left += myOffset + atOffset + offset;
}
},
top: function( position, data ) {
if ( data.at[ 1 ] === center ) {
return;
}

data.elem
.removeClass( "ui-flipped-top ui-flipped-bottom" );

var within = data.within,
win = $( window ),
isWindow = $.isWindow( data.within[0] ),
Expand All @@ -308,6 +320,10 @@ $.ui.position = {
-data.targetHeight,
offset = -2 * data.offset[ 1 ];
if ( overTop < 0 || overBottom > 0 ) {

data.elem
.addClass( "ui-flipped-" + ( overTop < 0 ? "bottom" : "top" ) );

position.top += myOffset + atOffset + offset;
}
}
Expand Down

10 comments on commit d5452c0

@scottgonzalez
Copy link
Member

Choose a reason for hiding this comment

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

We should really back out this commit. We shouldn't be using classes to designate that the element was flipped. Do we have any practical cases where classes are useful?

@bmsterling
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Especially in the case of the tooltips, yes. I'm am also using it for specific styles for the selectmenu where, if the menu flips to the top, it shows another style.

@scottgonzalez
Copy link
Member

Choose a reason for hiding this comment

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

Can you please provide a demo of this being used? It seems like knowing that it flipped isn't nearly as useful as knowing where it is, e.g., knowing that you're at the top is way more useful than knowing that you flipped vertically.

@bmsterling
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here are the tooltips, the site is behind a firewall so only screen grabs: http://dl.dropbox.com/u/39086/tooltips.png

@scottgonzalez
Copy link
Member

Choose a reason for hiding this comment

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

Screen grabs don't help here. I'm not saying that it's not possible to use these classes, I'm saying I don't think they're a good implementation.

@bmsterling
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough, the screen grabs should at least give an idea of the situation the tooltips are used.

Not clear on the reasoning for it not being a good implementation, how else do you propose that a developer knows that an element was been flipped? All of the alternatives I used within the tooltip and the select menu code added more overhead to each one of the widgets. Basically, every time one of them opened I'd have to figure out where the element that is being positioned is in relation to the trigger element and then add a class accordingly. So, instead of have roughly six lines of code within the position plugin I had 10+ lines within each one of those widget either by extending or hacking the core codes.

Obviously I am open for discussion, there definitely needs to be a simple way for a developer to know an element has been flipped IMO.

@scottgonzalez
Copy link
Member

Choose a reason for hiding this comment

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

I assume the 10 lines of code you wrote outside of position didn't check if the element flipped, but whether it was on the right or left. That seems much more useful, and is what we should expose. But it shouldn't be exposed as a class, it should be exposed some other yet to be defined way, possibly as additional information passed to the using option.

@bmsterling
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we should expose it but just seems like an extra step for the sake of an extra step. If we add a CSS class all the end developer needs to worry about is creating the correct CSS styles based on the flipped position.

If we simply expose the flipped variable then, not only do we expect the end developer to write the CSS styles but they also have to incorporate some logic in an "onBeforeOpen" callback to apply the correct class based on the flip.

So, for the sake of argument, I am building an app that uses tooltip, selectmenu, menubar, and autocomplete (just using those as examples since they all use position at some point in their code) and the designs require a specific look for all the possible views of each one of those widgets, I would need have code for each one of those widgets that does a check before displaying the widget so that they have the correct styles. Let's assume each widget has roughly 12 lines of code to check for flipped top, left, right, bottom, that's 48 lines of extra code at a minimum.

It just seems we can keep everything clean and simple by applying the class right in the position utility IMO.

@scottgonzalez
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately at this point you haven't proven anything because we still haven't seen any code. We can't use one person's private code as the defining way that we implement a feature.

@bmsterling
Copy link
Contributor Author

@bmsterling bmsterling commented on d5452c0 Aug 17, 2011 via email

Choose a reason for hiding this comment

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

Please sign in to comment.