Various PopupPanel fixes

42 views
Skip to first unread message

t.br...@gmail.com

unread,
Nov 26, 2009, 12:34:57 PM11/26/09
to j...@google.com, rj...@google.com, google-web-tool...@googlegroups.com
Reviewers: jgw, Ray Ryan,

Description:
Fixes issues 2907, 4277 and 4278.

Issue 2907: use currentStyle instead of style to get the style of the
popup (as suggested in the issue tracker)

Issue 4277: that's a bit "quick and dirty": I set the glass.className to
the same as the PopupPanel::getStyleName except the StylePrimaryName is
appended a "Glass" suffix (i.e. defaults to gwt-PopupPanelGlass). The
idea was that the glass panel would "inherit" the dependent style names
of the PopupPanel (i.e. gwt-PopupPanelGlass-aboveEverything). Another,
simpler, solution that'd work for me would be to just add a
get/setGlassStyleName pair of methods (and/or add/removeGlassStyleName)
or an accessor for the glass Element. I'd be happy to provide another
patch implementing this alternative if you prefer it.

Issue 4278: the "trick" here is to pass the glass panel to
PopupPanelImpl::onShow when isGlassEnabled()==true. On hide however,
PopupPanelImpl::onHide is called on both the PopupPanel and the glass
panel to make sure the iframe shim is removed (even if isGlassEnabled()
has been modified between show() and hide()).

To make things easier, I converted the PopupPanelImpl to use the
dom.client.Element instead of user.client.Element.

Unfortunately, I have neither selenium nor RMI configured, and
PopupPanel tests are disabled in HtmlUnit; so I haven't been able to run
the unit tests against my changes; but I used them in a project (by
overriding the files, just put them earlier in the classpath) and had
the expected result (though obviously not everything changes I made had
been "tested" this way).

Please review this at http://gwt-code-reviews.appspot.com/113801

Affected files:
user/src/com/google/gwt/user/client/ui/PopupPanel.java
user/src/com/google/gwt/user/client/ui/impl/PopupImpl.java
user/src/com/google/gwt/user/client/ui/impl/PopupImplIE6.java
user/src/com/google/gwt/user/client/ui/impl/PopupImplMozilla.java


t.br...@gmail.com

unread,
Nov 26, 2009, 8:03:50 PM11/26/09
to j...@google.com, rj...@google.com, google-web-tool...@googlegroups.com
I finally managed to run RemoteWeb tests (PopupTest, DecoratedPopupTest
and DialogBoxTest in -prod mode; on a Vista box with IE8 –which
fortunately runs in compat mode and tests the user.agent=ie6
permutation–).
It turned out there were a few bugs, and tests had to be updated
(because I create the glass panel lazily when the PopupPanel is shown,
instead of eagerly as soon as setGlassEnabled(true) is called)

http://gwt-code-reviews.appspot.com/113801

John Tamplin

unread,
Nov 26, 2009, 8:08:28 PM11/26/09
to Thomas Broyer, GWTcontrib
On Thu, Nov 26, 2009 at 12:34 PM, <t.br...@gmail.com> wrote:
Unfortunately, I have neither selenium nor RMI configured, and
PopupPanel tests are disabled in HtmlUnit; so I haven't been able to run
the unit tests against my changes; but I used them in a project (by
overriding the files, just put them earlier in the classpath) and had
the expected result (though obviously not everything changes I made had
been "tested" this way).

The easiest way to test with particular browsers (if you don't need to have a test farm setup anyway) is to use -runStyle Manual and connect with whatever browser you want to test.  You can use -runStyle Manual:2 and test two browsers simultaneously, etc.

--
John A. Tamplin
Software Engineer (GWT), Google

Thomas Broyer

unread,
Nov 27, 2009, 3:55:51 AM11/27/09
to Google Web Toolkit Contributors


On Nov 27, 2:08 am, John Tamplin <j...@google.com> wrote:
> On Thu, Nov 26, 2009 at 12:34 PM, <t.bro...@gmail.com> wrote:
> > Unfortunately, I have neither selenium nor RMI configured, and
> > PopupPanel tests are disabled in HtmlUnit; so I haven't been able to run
> > the unit tests against my changes; but I used them in a project (by
> > overriding the files, just put them earlier in the classpath) and had
> > the expected result (though obviously not everything changes I made had
> > been "tested" this way).
>
> The easiest way to test with particular browsers (if you don't need to have
> a test farm setup anyway) is to use -runStyle Manual and connect with
> whatever browser you want to test.  You can use -runStyle Manual:2 and test
> two browsers simultaneously, etc.

Yes, that's what I usually do ...except for GWT itself, for which I
try to only use the build.xml (to use the same classpath, JUnit
arguments, etc. as your build system), and there's no such targets as
test.manual, test.web.manual, test.hosted.manual, etc. in user/
build.xml ;-)

(I also tried adding such test.manual targets, but it took an eternity
for the "open this URL in a browser" line to appear so I gave up –
t'was time to go home ;-) )
Reply all
Reply to author
Forward
0 new messages