Add smarter popup position code to DOM UI Shared Resources....

Add smarter popup position code to DOM UI Shared Resources.

BUG=None
TEST=js unit tests as well as manually ensuring the menu button in the bookmarks manager flips side when the window gets small.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=47198

[arv (Not doing code reviews), 2010-05-13 00:08:54]

I thought of making this even more flexible by masks or corners that should be
aligned. For example, it is not uncommon for people to want to a popup to be
positioned below, but make the right edges align instead of the left edges. Do
you have any ideas of how to make this sane?

[feldstein, 2010-05-13 08:46:51]

Code LGTM, and I don't think I would make this too much more complicated if it's
only going to be used for menus.  If we're going to use it for popups like
dialog boxes we may want to use centering in addition to edges.

I can't think of a way to make this any more flexible or powerful without
getting into a weird api.  Perhaps something like alignPoints(TOP_LEFT,
BOTTOM_LEFT) to align a point on the popup to a point on the anchor.  It would
even allow you to do something like alignPoints(CENTER, CENTER) for things like
dialog boxes. or alignPoints(TOP_CENTER, TOP_CENTER) for mac-sheet-like popups. 
This seems like a pretty complicated implementation for not too much gain
though.

http://codereview.chromium.org/2042017/diff/2001/3003
File chrome/browser/resources/shared/js/cr/ui/position_util.js (right):

http://codereview.chromium.org/2042017/diff/2001/3003#newcode16
chrome/browser/resources/shared/js/cr/ui/position_util.js:16: /**
Do we want to be able to anchor inside of an element?  IE so the top left
corners match, or even centered, so the top edges line up, but the popup is
centered inside the anchor, sort of like mac sheets.

http://codereview.chromium.org/2042017/diff/2001/3003#newcode137
chrome/browser/resources/shared/js/cr/ui/position_util.js:137: // not enough
room on either side
This comment and the one above it should line up (not sure which is right)

[arv (Not doing code reviews), 2010-05-13 18:27:48]

I realized that I could replace the logic in context menu handler to use this
code as well. I renamed the function and added another one as well.

http://codereview.chromium.org/2042017/diff/2001/3003
File chrome/browser/resources/shared/js/cr/ui/position_util.js (right):

http://codereview.chromium.org/2042017/diff/2001/3003#newcode16
chrome/browser/resources/shared/js/cr/ui/position_util.js:16: /**
On 2010/05/13 08:46:52, feldstein wrote:
> Do we want to be able to anchor inside of an element?  IE so the top left
> corners match, or even centered, so the top edges line up, but the popup is
> centered inside the anchor, sort of like mac sheets.

I can see that being useful but I'd do it when we need it. It is very likely
that we do not need this because position dialogs and sheets can be done with
css.

http://codereview.chromium.org/2042017/diff/2001/3003#newcode137
chrome/browser/resources/shared/js/cr/ui/position_util.js:137: // not enough
room on either side
On 2010/05/13 08:46:52, feldstein wrote:
> This comment and the one above it should line up (not sure which is right)

Done.

[feldstein, 2010-05-13 19:28:54]

On 2010/05/13 18:27:48, arv wrote:
> I realized that I could replace the logic in context menu handler to use this
> code as well. I renamed the function and added another one as well.
> 
> http://codereview.chromium.org/2042017/diff/2001/3003
> File chrome/browser/resources/shared/js/cr/ui/position_util.js (right):
> 
> http://codereview.chromium.org/2042017/diff/2001/3003#newcode16
> chrome/browser/resources/shared/js/cr/ui/position_util.js:16: /**
> On 2010/05/13 08:46:52, feldstein wrote:
> > Do we want to be able to anchor inside of an element?  IE so the top left
> > corners match, or even centered, so the top edges line up, but the popup is
> > centered inside the anchor, sort of like mac sheets.
> 
> I can see that being useful but I'd do it when we need it. It is very likely
> that we do not need this because position dialogs and sheets can be done with
> css.
> 
> http://codereview.chromium.org/2042017/diff/2001/3003#newcode137
> chrome/browser/resources/shared/js/cr/ui/position_util.js:137: // not enough
> room on either side
> On 2010/05/13 08:46:52, feldstein wrote:
> > This comment and the one above it should line up (not sure which is right)
> 
> Done.

did you have a new patch?

[arv (Not doing code reviews), 2010-05-13 21:18:17]

Sorry about that.

Uploaded.

http://codereview.chromium.org/2042017/diff/10001/11001
File chrome/browser/resources/bookmark_manager/main.html (right):

http://codereview.chromium.org/2042017/diff/10001/11001#newcode51
chrome/browser/resources/bookmark_manager/main.html:51: console.error('Bookmarks
extension API is not avaiable');
stupid windows and lack of usable git.

I'll make sure this one gets included in the right patch.

[feldstein, 2010-05-13 21:54:44]

lgtm