Hey Alex,
LGTM, with the following suggestions and questions:

PopupPanel.java

590: Might want to elaborate a bit here ("Not always zero."). Maybe say
something like: "We check to see if left is less than clientLeft instead of
zero.."


General:

-Might want to add some JavaDoc to PopupPanel.setPopupPosition and the
DialogBox class indicating that the popup cannot be positioned (or dragged,
in the DialogBox case) horizontally outside of the visible area.

Testing:

-it seems that even if the body is horizontally scrollable, one will not be
able to drag the DialogBox into the "scrolled" area offscreen.  Is that the
case?



Rajeev

On Tue, Sep 30, 2008 at 1:54 PM, Alex Rudnick <[EMAIL PROTECTED]> wrote:

> Hey Rajeev,
>
> Would you review this patch for PopupPanel? It's a cleaned up version
> of what we did yesterday. This fixes the odd behavior we noticed where
> Popup Panels could not be placed near the right edge of the screen on
> IE, and normalizes popups in RTL and LTR modes, so they can't be
> dragged off the horizontal edges in either case.
>
> Testing: we made sure popup panels worked as expected in Showcase on
> all major browsers, both in Quirks and Standards mode. GWT tests all
> still work.
>
> Thanks!
>
> --
> Alex Rudnick
> swe, gwt, atl
>

--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---

Reply via email to