[gwt-contrib] Re: GWT support for Chrome Frame (issue1449808)
On 2011/05/31 16:42:35, fabiomfv wrote: LVVTGM. http://gwt-code-reviews.appspot.com/1449808/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixes a bug in TypeOracle for computing information about single JSO impls. (issue1373803)
On 2011/03/02 16:30:22, knorton wrote: lgtm me too http://gwt-code-reviews.appspot.com/1373803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Change GWT History to work when multiple applications are loaded within the same page. (issue1322801)
On 2011/01/25 00:24:52, jhollenbach wrote: LGTM http://gwt-code-reviews.appspot.com/1322801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixing the overflow-x and overflow-y properties in Style to use camelCase instead of hyphenated ... (issue1315801)
On 2011/01/21 16:09:17, jlabanca wrote: LGTM. http://gwt-code-reviews.appspot.com/1315801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Cleanup DOM after DoubleClickEventSinkTest tests complete (issue1150801)
On 2010/11/24 17:47:45, fredsa wrote: LGTM. http://gwt-code-reviews.appspot.com/1150801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add support for touch events for supported mobile webkit platforms. (issue867801)
On 2010/11/23 21:08:03, fredsa wrote: LGTM http://gwt-code-reviews.appspot.com/867801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds overflow-x and overflow-y to Style since they have at least partial (issue1100801)
On 2010/11/11 23:22:56, knorton wrote: LGTM http://gwt-code-reviews.appspot.com/1100801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix issue with FF3.5 and below not having readyState (issue1087801)
On 2010/11/08 05:34:00, unnurg wrote: LGTM. Disturbing that we have to do this, but LGTM all the same :) http://gwt-code-reviews.appspot.com/1087801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Loosens up JSO restrictions in the TypeOracle to allow multiple JSO (issue1076801)
On 2010/11/04 15:42:44, scottb wrote: http://gwt-code-reviews.appspot.com/1076801/diff/1/2 File dev/core/src/com/google/gwt/core/ext/typeinfo/TypeOracle.java (right): http://gwt-code-reviews.appspot.com/1076801/diff/1/2#newcode687 dev/core/src/com/google/gwt/core/ext/typeinfo/TypeOracle.java:687: JMethod found = cls.findMethod(meth.getName(), methodParamTypes(meth)); If it's working, just throw a comment in there, if you would that maybe it could be done more cleanly with what tbroyer suggested. Already committed at r9183. I'll come back and make a note later. FWIW, Jaime was trying to do something with an even more complex inheritance pattern than I tested, and it worked fine for him. http://gwt-code-reviews.appspot.com/1076801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Loosens up JSO restrictions in the TypeOracle to allow multiple JSO (issue1076801)
http://gwt-code-reviews.appspot.com/1076801/diff/1/2 File dev/core/src/com/google/gwt/core/ext/typeinfo/TypeOracle.java (right): http://gwt-code-reviews.appspot.com/1076801/diff/1/2#newcode676 dev/core/src/com/google/gwt/core/ext/typeinfo/TypeOracle.java:676: // TODO(jgw): Not sure if getInheritableMethods() includes super-interfaces. On 2010/11/03 23:55:54, tbroyer wrote: Yes it does. FYI, getOverridableMethods() returns the same as getInheritableMethods(), just without the methods marked 'final' (which are inherited, i.e. callable from a subclass, but not overridable). Ah, thanks for catching that. http://gwt-code-reviews.appspot.com/1076801/diff/1/2#newcode687 dev/core/src/com/google/gwt/core/ext/typeinfo/TypeOracle.java:687: JMethod found = cls.findMethod(meth.getName(), methodParamTypes(meth)); On 2010/11/03 23:55:54, tbroyer wrote: Given that (IIRC) getOverridableMethods and getInheritableMethods would return the implemented methods as attached to the implementing class and unimplemented methods attached to the interface defining them; it might be simpler to just loop over all inheritable methods of the class and check that the method's declaring class is not an interface (or maybe, is not intf or an interface that extends intf, if you want to limit the scope to a particular interface). My knowledge of the compiler is pretty limited though, so don't take my word at it and ask confirmation to an authoritative person ;-) That's a good point -- I'm not all that familiar with the TypeOracle hierarchy anymore either, so I may just leave it alone for now, given that it's working. @scottb: If you have any preference for one approach or the other, I'm glad to change it. http://gwt-code-reviews.appspot.com/1076801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add support for touch events for supported mobile webkit platforms. (issue867801)
LGTM. Comments are advisory only. http://gwt-code-reviews.appspot.com/867801/diff/12001/13003 File user/src/com/google/gwt/dom/client/NativeEvent.java (right): http://gwt-code-reviews.appspot.com/867801/diff/12001/13003#newcode72 user/src/com/google/gwt/dom/client/NativeEvent.java:72: return DOMImpl.impl.getChangedTouches(this); It's unfortunate that there's no way to put these methods elsewhere, but I suppose we have no choice until we find a way to refactor the event hierarchy... :( http://gwt-code-reviews.appspot.com/867801/diff/12001/13004 File user/src/com/google/gwt/dom/client/Touch.java (right): http://gwt-code-reviews.appspot.com/867801/diff/12001/13004#newcode25 user/src/com/google/gwt/dom/client/Touch.java:25: public class Touch extends JavaScriptObject { FWIW, it wasn't strictly necessary to add all the shims to DOMImpl for these methods, since we don't have any need to create separate browser versions of them. But it won't hurt anything either (IOW, don't refactor it on account of this). http://gwt-code-reviews.appspot.com/867801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Loosens up JSO restrictions in the TypeOracle to allow multiple JSO (issue1076801)
Reviewers: scottb, Description: Loosens up JSO restrictions in the TypeOracle to allow multiple JSO implementations of an interface if the implementations share a common base class that fully implements the interface. Review by: sco...@google.com Please review this at http://gwt-code-reviews.appspot.com/1076801/show Affected files: M dev/core/src/com/google/gwt/core/ext/typeinfo/TypeOracle.java M dev/core/test/com/google/gwt/dev/javac/JSORestrictionsTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix some issues in the xsiframe linker (issue1057801)
On 2010/10/26 22:11:25, unnurg wrote: LGTM http://gwt-code-reviews.appspot.com/1057801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add support for touch events for supported mobile webkit platforms. (issue867801)
On 2010/09/14 00:49:29, fredsa wrote: Looks good. That weird sink-all-events stuff in Image is unfortunate, but it looks trickier to deal with than is appropriate with this patch. http://gwt-code-reviews.appspot.com/867801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adding a constructor override to SplitLayoutPanel so users can specify the width of the splitter. (issue1001801)
On 2010/10/14 14:01:05, jlabanca wrote: LGTM http://gwt-code-reviews.appspot.com/1001801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add Support for server side script selection in linker (issue941802)
Wow, that's one hell of a refactoring. I added a couple of 'taste' comments that you're free to do with as you will; and there's one spot where I think you're missing a 'var' or two. No need to re-review. Once you're satisfied, feel free to submit. http://gwt-code-reviews.appspot.com/941802/diff/24001/25006 File dev/core/src/com/google/gwt/core/ext/linker/impl/computeScriptBase.js (right): http://gwt-code-reviews.appspot.com/941802/diff/24001/25006#newcode31 dev/core/src/com/google/gwt/core/ext/linker/impl/computeScriptBase.js:31: metaVal = __gwt_getMetaProperty('baseUrl'); I think you need a declaration for 'base' and 'metaVal' here. http://gwt-code-reviews.appspot.com/941802/diff/24001/25022 File dev/core/src/com/google/gwt/core/linker/CrossSiteIframeLinker.java (right): http://gwt-code-reviews.appspot.com/941802/diff/24001/25022#newcode59 dev/core/src/com/google/gwt/core/linker/CrossSiteIframeLinker.java:59: protected boolean installCode = true; When would you want to turn this off? http://gwt-code-reviews.appspot.com/941802/diff/24001/25022#newcode79 dev/core/src/com/google/gwt/core/linker/CrossSiteIframeLinker.java:79: com/google/gwt/core/ext/linker/impl/waitForBodyLoaded.js; I assume all of the above fields are meant to be overwritten by subclasses. I think they're fairly self-explanatory, but could we not make them template methods instead (e.g., boolean installCode(), String getProcessMetasJs(), etc)? Seems like it might be slightly less prone to confusion in subclasses. It's a taste thing, though, so I'll leave it to your discretion. http://gwt-code-reviews.appspot.com/941802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add Support for server side script selection in linker (issue941802)
On 2010/10/07 21:17:49, unnurg wrote: I thought I already saw this go through -- is this still awaiting review? http://gwt-code-reviews.appspot.com/941802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Let MenuItem implement HasEnabled (issue846801)
On 2010/09/20 13:06:07, jgw wrote: Sorry about that. Will look at it this afternoon (US/EST). And apparently this afternoon means several days later... Sorry about that. The patch seems fine overall, but if you have a moment to add a test of the enabled/disabled state, that would be helpful (I'll try to get to it myself if I don't hear back, but that could prolong things even further at my current rate of progress...). http://gwt-code-reviews.appspot.com/846801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: GWT implementation of json2.js parse and stringify, plus removal of json2.js dependency from RF.... (issue922801)
On 2010/09/24 20:17:34, rjrjr wrote: Joel, any chance of you getting to this today? That would be huge. On Fri, Sep 24, 2010 at 1:15 PM, mailto:cromwell...@google.com wrote: Reviewers: jgw, rjrjr, Description: GWT implementation of json2.js parse and stringify, plus removal of json2.js dependency from RF. This is a slice of a much more ambitious JSON library that unified client and server JSON APIs into a single shared library. Thus, it is expected that this API is not final, and will eventually be moved into another package. Currently, pure GWT only, does not attempt to delegate to native browser JSON methods (which are full of browser-version dependant bugs). Please review this at http://gwt-code-reviews.appspot.com/922801/show Affected files: M user/src/com/google/gwt/requestfactory/client/impl/JsonResults.java M user/src/com/google/gwt/requestfactory/client/impl/ProxyJsoImpl.java A user/src/com/google/gwt/requestfactory/client/json/ClientJsonUtil.java A user/src/com/google/gwt/requestfactory/client/json/JsonArrayContext.java A user/src/com/google/gwt/requestfactory/client/json/JsonContext.java A user/src/com/google/gwt/requestfactory/client/json/JsonException.java A user/src/com/google/gwt/requestfactory/client/json/JsonMap.java A user/src/com/google/gwt/requestfactory/client/json/JsonMapContext.java A user/src/com/google/gwt/requestfactory/client/json/JsonVisitor.java M user/test/com/google/gwt/requestfactory/RequestFactorySuite.java A user/test/com/google/gwt/requestfactory/client/json/ClientJsonUtilTest.java Looks fine to me; feel free to commit. Two quick questions, though: 1. Be sure we really want to commit to this if you're going to make it public (fine by me, just wanted to make sure it was intentional, and not accidental). 2. Do we actually need safe parsing for our use cases? http://gwt-code-reviews.appspot.com/922801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Fix for issue 4694. Adds a 'visibility' property to layout layers, along with (issue869801)
Reviewers: jlabanca, Description: Fix for issue 4694. Adds a 'visibility' property to layout layers, along with a hack to LayoutImplIE8 that ensures relative units are recalculated correctly on visibility change. Also changes LayoutPanel and TabLayoutPanel to ensure that they use this new layer property, rather than modifying the container element's style directly. Please review this at http://gwt-code-reviews.appspot.com/869801/show Affected files: M user/src/com/google/gwt/layout/client/Layout.java M user/src/com/google/gwt/layout/client/LayoutImpl.java M user/src/com/google/gwt/layout/client/LayoutImplIE6.java M user/src/com/google/gwt/layout/client/LayoutImplIE8.java M user/src/com/google/gwt/user/client/ui/LayoutPanel.java M user/src/com/google/gwt/user/client/ui/TabLayoutPanel.java M user/test/com/google/gwt/user/client/ui/TabLayoutPanelTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix for issue 4694. Adds a 'visibility' property to layout layers, along with (issue869801)
On 2010/09/14 15:17:21, jlabanca wrote: LGTM http://gwt-code-reviews.appspot.com/869801/diff/1/5 File user/src/com/google/gwt/layout/client/LayoutImplIE8.java (right): http://gwt-code-reviews.appspot.com/869801/diff/1/5#newcode177 user/src/com/google/gwt/layout/client/LayoutImplIE8.java:177: // only way to correctly ensure that layout units get translated correctly. Is performance at least acceptable? It seems perfectly fast in my tests, though they're not really comprehensive (they test the bug, but aren't all that large). But I don't think it will matter all that much in practice, because it only runs on a visibility change, which is already a fairly large user action (in practice I'd be surprised if it took more than a few milliseconds). Committed at r8775. http://gwt-code-reviews.appspot.com/869801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adding support for Direction.LINE_START/END to DockLayoutPanel and SplitLayoutPanel. (issue828801)
On 2010/09/02 16:33:00, jlabanca wrote: Patch Set 2 updates DOMRtlTest, which was never really in RTL mode until DockLayoutPanelRtlTest came along. The body isn't put into RTL mode until RootPanel.get() is called, but DOMRtlTest never called it. I also renamed DockLayoutPanelTestRtl to DockLayoutPanelRtlTest. LGTM. http://gwt-code-reviews.appspot.com/828801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add Late Loading support to xsiframe linker (issue807801)
Assuming everything works as expected, everything here looks good to me (not bad for someone who claims to not know Javascript :) I'm stuck at home right now, so I don't have the facilities here to test out IE, which is where any problems would likely crop up. In particular, I'd make sure it works on IE6 and 7, and in the presence/absence of normal script/meta tags (the stuff that might trip up computeScriptBase(). Also, I assume it's intended that this *not* support script/style injection from the .gwt.xml (fine with me -- those features have been 1000x more trouble than they're worth). But we want to make sure we document this fact somewhere. Finally, it looks like a lot of the work you've done here is equivalent to the XHTML changes in http://codereview.appspot.com/1873052. While I personally don't care much about XHTML, if it is the case that this just happens to support it (because of the elimination of document.write()), that would be nice to know. IOW, LGTM. http://gwt-code-reviews.appspot.com/807801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add Late Loading support to xsiframe linker (issue807801)
Excellent. As far as the XHTML thing is concerned, let's just ask him to see if this actually solves the problem. If it's necessary to make those DOM case changes to fix XHTML problems, I'd be ok with that (though I don't really want to sign on to testing all our code in an XHTML context -- that's just another constraint we can't accept, especially given that you can't get XHTML to even work on all browsers). But we can address that once he confirms that your linker fixes that part of the problem. http://gwt-code-reviews.appspot.com/807801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Avoids a hidden NPE in eventGetRelatedTarget() when Mozilla decides to return (issue819801)
Reviewers: bobbrose_google.com, Description: Avoids a hidden NPE in eventGetRelatedTarget() when Mozilla decides to return a null value for Event.relatedTarget. This NPE was getting trapped by an exception handler, but was wreaking havoc for people debugging compiled code under Firebug (with stop on all errors enabled). Please review this at http://gwt-code-reviews.appspot.com/819801/show Affected files: M user/src/com/google/gwt/dom/client/DOMImplMozilla.java Index: user/src/com/google/gwt/dom/client/DOMImplMozilla.java === --- user/src/com/google/gwt/dom/client/DOMImplMozilla.java (revision 8671) +++ user/src/com/google/gwt/dom/client/DOMImplMozilla.java (working copy) @@ -66,6 +66,9 @@ // elements). Trying to access relatedTarget.nodeName will throw an // exception if it's a XUL element. var relatedTarget = evt.relatedTarget; +if (!relatedTarget) { + return null; +} try { var nodeName = relatedTarget.nodeName; return relatedTarget; -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Makes Widget.addDomHandler public, so that helpers can add keyboard handlers to panels without c... (issue779802)
On 2010/08/19 12:00:39, gryder wrote: Well, I had thought there was a good reason to keep these protected, but I can't seem to find any problem with making them public. LGTM. http://gwt-code-reviews.appspot.com/779802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: HasEnabled (issue757801)
On 2010/08/13 15:01:58, johan.rydberg wrote: Done! On Fri, Aug 13, 2010 at 4:00 PM, mailto:jlaba...@google.com wrote: @johan - Can you sign a Contributor License Agreement so we can include your code: http://code.google.com/legal/individual-cla-v1.0.html If you scroll to the bottom, you can sign it electronically. - John http://gwt-code-reviews.appspot.com/757801/show (Finally) committed at r8594. http://gwt-code-reviews.appspot.com/757801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add mouse position information to ClickEvent and DoubleClickEvent. Manually verified that this (issue763801)
On 2010/08/12 22:32:53, fredsa wrote: LGTM. http://gwt-code-reviews.appspot.com/763801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add the ability to change the size of a widget in the (issue719802)
On 2010/08/02 18:38:29, toms wrote: On Mon, Aug 2, 2010 at 2:14 PM, mailto:j...@google.com wrote: http://gwt-code-reviews.appspot.com/719802/diff/1/2 File user/src/com/google/gwt/user/client/ui/DockLayoutPanel.java (right): http://gwt-code-reviews.appspot.com/719802/diff/1/2#newcode315 user/src/com/google/gwt/user/client/ui/DockLayoutPanel.java:315: public void setWidgetSize(Widget widget, int size) { This needs to be a double (all units except PX can be non-integral). True, just eclipse auto create method getting in the way, done. http://gwt-code-reviews.appspot.com/719802/show I'll assume that you just forgot to upload the other patch set, and that 'int' now reads 'double' :) If so, LGTM. http://gwt-code-reviews.appspot.com/719802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add the ability to change the size of a widget in the (issue719802)
http://gwt-code-reviews.appspot.com/719802/diff/1/2 File user/src/com/google/gwt/user/client/ui/DockLayoutPanel.java (right): http://gwt-code-reviews.appspot.com/719802/diff/1/2#newcode315 user/src/com/google/gwt/user/client/ui/DockLayoutPanel.java:315: public void setWidgetSize(Widget widget, int size) { This needs to be a double (all units except PX can be non-integral). http://gwt-code-reviews.appspot.com/719802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds a new CrossSiteIframeLinker. This linker works cross-site, (issue726802)
Looks great. In response to Matt's question about __COMPUTE_SCRIPT_BASE__, that replacement is done in SelectionScriptLinker.java. It's a fairly recent change to share more stuff among the various linkers -- though hopefully we'll be able to reduce the various linkers to one at some point :) http://gwt-code-reviews.appspot.com/726802/diff/1/3 File dev/core/src/com/google/gwt/core/linker/CrossSiteIframeTemplate.js (right): http://gwt-code-reviews.appspot.com/726802/diff/1/3#newcode88 dev/core/src/com/google/gwt/core/linker/CrossSiteIframeTemplate.js:88: (query.indexOf('gwt.hybrid') == -1); On 2010/07/29 19:55:33, Matt Mastracci wrote: Is it worth trimming the old gwt.hosted baggage from here? It's kind of silly, but in the interest of keeping some old scripts around here (and doubtless outside of Google) working, I'd be ok with keeping it. http://gwt-code-reviews.appspot.com/726802/diff/1/3#newcode110 dev/core/src/com/google/gwt/core/linker/CrossSiteIframeTemplate.js:110: scriptFrame.style.cssText = 'position:absolute; width:0; height:0; border:none'; On 2010/07/29 19:55:33, Matt Mastracci wrote: These should probably be !important to avoid user CSS accidentally styling them. Also, left: -1000px and top: -1000px, since absolute by default will place them at the end of the document potentially causing sizing issues. I think Matt's right about this (and I should have done this ages ago). It's pretty defensive, but that's what you have to do in CSS-land :-/ http://gwt-code-reviews.appspot.com/726802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Adds EventTarget, add/removeEventListener() et al to gwt dom. (issue623803)
Reviewers: tbroyer, Description: Adds EventTarget, add/removeEventListener() et al to gwt dom. Note: This is not remotely finished, nor is the API certain. Please review this at http://gwt-code-reviews.appspot.com/623803/show Affected files: M user/src/com/google/gwt/dom/client/DOMImpl.java M user/src/com/google/gwt/dom/client/DOMImplStandard.java M user/src/com/google/gwt/dom/client/DOMImplTrident.java M user/src/com/google/gwt/dom/client/Element.java M user/src/com/google/gwt/dom/client/EventTarget.java M user/src/com/google/gwt/dom/client/Node.java M user/test/com/google/gwt/dom/DOMSuite.java A user/test/com/google/gwt/dom/client/EventTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Adds Event.sinkEvent() to make it possible to sink custom events in gwt-user. Reimplements (issue635802)
Reviewers: tbroyer, Description: Adds Event.sinkEvent() to make it possible to sink custom events in gwt-user. Reimplements gwt-user event handling in terms of standard dom EventTarget methods. Note: This is not remotely finished, nor the API certain. Please review this at http://gwt-code-reviews.appspot.com/635802/show Affected files: M user/src/com/google/gwt/user/client/DOM.java M user/src/com/google/gwt/user/client/Event.java M user/src/com/google/gwt/user/client/impl/DOMImpl.java M user/src/com/google/gwt/user/client/impl/DOMImplMozilla.java M user/src/com/google/gwt/user/client/impl/DOMImplOpera.java M user/src/com/google/gwt/user/client/impl/DOMImplStandard.java M user/src/com/google/gwt/user/client/impl/DOMImplTrident.java M user/test/com/google/gwt/user/client/EventTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Adding microbenchmark for event sinking (using gwt-user Event.sinkEvents()). (issue668802)
Reviewers: tbroyer, Description: Adding microbenchmark for event sinking (using gwt-user Event.sinkEvents()). Please review this at http://gwt-code-reviews.appspot.com/668802/show Affected files: M reference/Microbenchmarks/src/com/google/gwt/reference/microbenchmark/client/Microbenchmarks.java A reference/Microbenchmarks/src/com/google/gwt/reference/microbenchmark/client/UserEventSinks.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds Event.sinkEvent() to make it possible to sink custom events in gwt-user. Reimplements (issue635802)
http://gwt-code-reviews.appspot.com/635802/diff/1/3 File user/src/com/google/gwt/user/client/Event.java (right): http://gwt-code-reviews.appspot.com/635802/diff/1/3#newcode556 user/src/com/google/gwt/user/client/Event.java:556: DOM.sinkEvent((com.google.gwt.user.client.Element) elem, type, sink); I'm not 100% sold on the idea of this method. The alternative would be to simply continue defining event bits for all the new events we can find. But I was also kind of hoping to get an escape hatch that would allow people to use new events as they become available, rather than waiting on us to add new bits and push a new GWT version. http://gwt-code-reviews.appspot.com/635802/diff/1/4 File user/src/com/google/gwt/user/client/impl/DOMImpl.java (right): http://gwt-code-reviews.appspot.com/635802/diff/1/4#newcode130 user/src/com/google/gwt/user/client/impl/DOMImpl.java:130: sinkEvent(elem, eventGetType(bit), (bits bit) != 0); We have to route sinkEvents(bits) through the new sinkEvent(string) because it's pretty much the only way to ensure that the two sink methods play well together. I think it would be a Bad Thing to have the same event sunk twice, because it would create potentially hard-to-debug situations where you're receiving events twice in onBrowserEvent(). The downside is that this is pretty inefficient because of all the non-inlineable layers of indirection, the added __events map, and the extra work going on in Element.addEventListener(). A simpler, more efficient, but slightly confusing alternative would be to keep sinkEvent() and sinkEvents() completely separate, and let people know that if they use both for the same event, they'll just get duplicate events. Then create event bits for the current well-known event types and be done with it. http://gwt-code-reviews.appspot.com/635802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds EventTarget, add/removeEventListener() et al to gwt dom. (issue623803)
http://gwt-code-reviews.appspot.com/623803/diff/1/3 File user/src/com/google/gwt/dom/client/DOMImplStandard.java (right): http://gwt-code-reviews.appspot.com/623803/diff/1/3#newcode115 user/src/com/google/gwt/dom/client/DOMImplStandard.java:115: public native void addEventListener(EventTarget target, String type, On 2010/07/02 15:39:30, jat wrote: How about returning Object here as the opaque handle for remove? Then you could just return the callback fuction and avoid the map. I was trying to avoid creating a non-standard DOM interface (EventTarget already has addEventListener() defined). While your proposal is a simpler solution, it just pushes the burden on the user, who's going to be forced to do the same thing we're doing here. And while the implementation is kind of ugly, it's still efficient at runtime, which is less likely to be true if we force the user to hang on to an opaque handle. http://gwt-code-reviews.appspot.com/623803/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add WebSocket API (issue646803)
I don't have a particularly strong opinion on the RawWebSocket vs. RawWebSocketImpl dichotomy, but it might be a bit more abstracted that necessary. I'm perfectly fine with the WebSocket vs. RawWebSocket difference, though, because one's a native browser class, while the other provides a more Java-centric interface. Where I think there's a breakdown, though, is that the WebSocket*Handler interface, which seem to be meant to mimic the widget-level event handlers, actually pass event objects that are JSO subtypes. This is mixing up the DOM/Java levels of abstraction in a confusing way. FWIW, I think it might be worth trying to land this in two phases: 1. Native/DOM-only stuff. The rawest possible classes necessary to interface with WebSocket. 2. Java-friendly wrappers, possibly in a separate package. This distinction is much like that between XMLHttpRequest (raw native interface) and RequestBuilder (Java-friendly, built on top of it). http://gwt-code-reviews.appspot.com/646803/diff/16001/17013 File user/src/com/google/gwt/websocket/client/RawWebSocket.java (right): http://gwt-code-reviews.appspot.com/646803/diff/16001/17013#newcode35 user/src/com/google/gwt/websocket/client/RawWebSocket.java:35: boolean useCapture); An alternative to this approach would be to use the standard EventTarget.addEventListener() signature as described in http://gwt-code-reviews.appspot.com/623803 It's a little hairier to implement, because of the need to map EventListener instances back to the lambda that wraps them, but it's easier on the user. http://gwt-code-reviews.appspot.com/646803/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds EventTarget, add/removeEventListener() et al to gwt dom. (issue623803)
http://gwt-code-reviews.appspot.com/623803/diff/1/3 File user/src/com/google/gwt/dom/client/DOMImplStandard.java (right): http://gwt-code-reviews.appspot.com/623803/diff/1/3#newcode115 user/src/com/google/gwt/dom/client/DOMImplStandard.java:115: public native void addEventListener(EventTarget target, String type, On 2010/07/02 16:12:33, jat wrote: On 2010/07/02 16:07:27, jgw wrote: I was trying to avoid creating a non-standard DOM interface (EventTarget already has addEventListener() defined). While your proposal is a simpler solution, it just pushes the burden on the user, who's going to be forced to do the same thing we're doing here. And while the implementation is kind of ugly, it's still efficient at runtime, which is less likely to be true if we force the user to hang on to an opaque handle. In most cases, the user code is never going to unregister its handlers, so it won't save the handle anyway. If it does, it would just store it in a field rather than a map, which would be more efficient than what the general implementation has to do here. Look more carefully. The production implementation just drops an expando on the listener instance. That's probably the most efficient thing you could do. Again, I think it's worth sticking to the standards here to avoid confusion at the DOM level. http://gwt-code-reviews.appspot.com/623803/diff/1/3#newcode119 user/src/com/google/gwt/dom/client/DOMImplStandard.java:119: listen...@com.google.gwt.dom.client.eventlistener::handleEvent(Lcom/google/gwt/dom/client/NativeEvent;)(e); On 2010/07/02 16:12:33, jat wrote: Why not just go ahead and add it? Did you see the description of this issue, where it says not done yet? If you want to finish it while I'm gone, be my guest :) http://gwt-code-reviews.appspot.com/623803/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds EventTarget, add/removeEventListener() et al to gwt dom. (issue623803)
http://gwt-code-reviews.appspot.com/623803/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds EventTarget, add/removeEventListener() et al to gwt dom. (issue623803)
http://gwt-code-reviews.appspot.com/623803/diff/1/4 File user/src/com/google/gwt/dom/client/DOMImplTrident.java (right): http://gwt-code-reviews.appspot.com/623803/diff/1/4#newcode161 user/src/com/google/gwt/dom/client/DOMImplTrident.java:161: // TODO: Hang on to enough information to implement dispose() properly. On 2010/07/02 16:26:08, tbroyer wrote: I guess the implementation would be pretty similar to the DOMImplStandard one re. setFn/getFn, just that setFn would probably also push on a MapEventTarget,SetEventListener (or use an expando on the EventTarget) which would be used by dispose() In other words, maybe setFn/getFn should be instance methods (non-static) at the DOMImpl level, with DOMImplTrident overriding setFn as explained above. That makes perfect sense to me. I just wanted to get the standard implementation working first. And Trident is going to require extra bookkeeping to allow dispose() to work properly (if only you could enumerate extant event handlers... sigh.). http://gwt-code-reviews.appspot.com/623803/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds EventTarget, add/removeEventListener() et al to gwt dom. (issue623803)
On 2010/07/02 16:26:08, tbroyer wrote: LGTM overall. As a side note, where is the c.g.g.dom.client.EventListener defined? Could it be made generic? I like how the WebSocket proposal added a base Event class mimicking the DOM Event interface, with NativeEvent (and other WebSocket-specific events) extending it. EventListener could then be defined as interface EventListenerT extends Event. It wouldn't be as type-safe as EventHandler (which guards you against adding a handler not matching the event's type) but would save us all doing casts to the specific Event subclass as the first line of all our EventListeners. Whoops, just added the missing EventListener interface. I hadn't thought about making it generic, but it's worth considering. I also agree that it makes sense to hoist out the Event base class as in the WebSocket patch. If you all would like to try and get a patch landed while I'm gone, that includes the hoisted Event class, along with the EventListener/EventTarget changes, that would be awesome, and you'd have my blessing :) http://gwt-code-reviews.appspot.com/623803/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Replacing PagingListView.setPageStart/Size with PagingListView.setRange. Replacing CellListImpl... (issue614803)
LGTM overall. Some minor comments that shouldn't stop you from committing. And let me just add w00t, tests! http://gwt-code-reviews.appspot.com/614803/diff/3001/4013 File user/src/com/google/gwt/user/cellview/client/CellTable.java (right): http://gwt-code-reviews.appspot.com/614803/diff/3001/4013#newcode790 user/src/com/google/gwt/user/cellview/client/CellTable.java:790: presenter.setRange(start, length); I don't see any real problems with this, but it seems slightly odd to have both setPageStart/Size() *and* setRange(). http://gwt-code-reviews.appspot.com/614803/diff/3001/4016 File user/src/com/google/gwt/user/cellview/client/PagingListViewPresenter.java (right): http://gwt-code-reviews.appspot.com/614803/diff/3001/4016#newcode45 user/src/com/google/gwt/user/cellview/client/PagingListViewPresenter.java:45: * user facing API simpler. I actually don't see this structure as being problematic. The fact that each widget happens to create its own view and uses this presenter to interact with it just indicates to me that it's behaving like its own little application. All the logical interaction between the widget and its view is then mediated by the presenter. Sounds normal to me. http://gwt-code-reviews.appspot.com/614803/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adding NoSelectionModel, which allows selection without saving the selection state. (issue667801)
On 2010/06/28 17:44:06, jlabanca wrote: LGTM. http://gwt-code-reviews.appspot.com/667801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Second attempted fix for issue 4532. This time, we revert the IE7 layout (issue628802)
On 2010/06/30 15:31:29, jlabanca wrote: LGTM Thanks, committed at r8339. http://gwt-code-reviews.appspot.com/628802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Second attempted fix for issue 4532. This time, we revert the IE7 layout (issue628802)
Reviewers: jlabanca, Description: Second attempted fix for issue 4532. This time, we revert the IE7 layout implementation back all the way to IE6. IE7's just too quirky to rely directly on its implementation of top/left/right/bottom/width/height CSS layout. Please review this at http://gwt-code-reviews.appspot.com/628802/show Affected files: M user/src/com/google/gwt/layout/client/LayoutImplIE6.java M user/src/com/google/gwt/layout/client/LayoutImplIE8.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Making StackLayoutPanel#showWidget(Widget) call showWidget(int) for legacy support. Ditto to Ta... (issue641802)
On 2010/06/28 16:54:55, jlabanca wrote: LGTM http://gwt-code-reviews.appspot.com/641802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Finishing implementation of ListViewAdapter. An extensive test class will be submitted in a lat... (issue636802)
On 2010/06/23 18:29:32, jlabanca wrote: LGTM. http://gwt-code-reviews.appspot.com/636802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: DefaultSelectionModel#setSelected currently adds an exception even if the default selection stat... (issue658801)
On 2010/06/23 18:28:32, jlabanca wrote: Is there a test suite to which tese should be added? http://gwt-code-reviews.appspot.com/658801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: DefaultSelectionModel#setSelected currently adds an exception even if the default selection stat... (issue658801)
On 2010/06/23 19:08:30, jlabanca wrote: LGTM. http://gwt-code-reviews.appspot.com/658801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: UiBinder. Add tests for horizontal/vertical alignment parsers. (issue633801)
On 2010/06/17 08:52:37, Konstantin.Scheglov wrote: Thanks, LGTM. Submitted at r8281. http://gwt-code-reviews.appspot.com/633801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: DockPanelParser and width/height attributes (issue633802)
On 2010/06/18 15:02:50, Konstantin.Scheglov wrote: Thanks, LGTM. Committed at r8289. http://gwt-code-reviews.appspot.com/633802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adding AbstractListViewAdapter#getViews() to get the views associated with an adapter. Also add... (issue601801)
On 2010/06/10 21:11:08, jlabanca wrote: Woohoo, tests! LGTM. http://gwt-code-reviews.appspot.com/601801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adding TreeItem#insert methods to insert items into a tree. (issue592801)
On 2010/06/07 18:21:24, jlabanca wrote: LGTM http://gwt-code-reviews.appspot.com/592801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: RR : Change ClientBundle ResourcePrototype initialization (issue583801)
On 2010/06/03 14:20:33, bobv wrote: Review requested. LGTM http://gwt-code-reviews.appspot.com/583801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Deleting the stocks sample in favor of the Expenses sample. (issue580801)
On 2010/06/02 16:42:38, jlabanca wrote: LGTM. http://gwt-code-reviews.appspot.com/580801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adding an override to tab based widgets to allow users to choose whether or not an event is fired. (issue559801)
On 2010/05/25 18:37:20, jlabanca wrote: LGTM. http://gwt-code-reviews.appspot.com/559801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds helpers for UIObject.add/removeStyle(Dependent)Name, to address the frequent use case: (issue576801)
On 2010/06/01 16:29:00, Guillaume Ryder wrote: LGTM. http://gwt-code-reviews.appspot.com/576801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: RR : Rework ImageResourceGenerator to address several issues (issue335802)
LGTM. http://gwt-code-reviews.appspot.com/335802/diff/1/4 File user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java (left): http://gwt-code-reviews.appspot.com/335802/diff/1/4#oldcode221 user/src/com/google/gwt/resources/rg/ImageBundleBuilder.java:221: interface HasRect { On 2010/04/16 18:24:03, bobv wrote: Removed this interface because it is silly. Always a good reason :) http://gwt-code-reviews.appspot.com/335802/diff/1/6 File user/test/com/google/gwt/resources/client/ImageResourceTest.java (right): http://gwt-code-reviews.appspot.com/335802/diff/1/6#newcode167 user/test/com/google/gwt/resources/client/ImageResourceTest.java:167: assertEquals(128, r.scaledUp().getHeight()); I assume the point of testing both width and height here (even though only width is set above) is to assert that the image scaled proportionally, right? http://gwt-code-reviews.appspot.com/335802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Firing MenuItem commands using a finally command instead of a deferred command so that the comma... (issue543804)
On 2010/05/25 15:42:00, jlabanca wrote: LGTM. http://gwt-code-reviews.appspot.com/543804/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Have IE's DOMImpl invoke its event dispatching function via a (issue543801)
On 2010/05/18 20:01:26, Lex wrote: This is ready for review. It is absolutely stunning that this is necessary to avoid leaking on IE, but LGTM. Good work! http://gwt-code-reviews.appspot.com/543801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Reapplying r5742 and r5747 to fix RichTextArea issues 3133, 3176, 3503. The original version of... (issue555801)
On 2010/05/24 20:40:56, jlabanca wrote: LGTM. Feel free to commit, but allow me to point out that we don't actually support old mozilla anymore :) http://gwt-code-reviews.appspot.com/555801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds a loading animation to the table. Adds timers to desktop and mobile to refresh tables as d... (issue503802)
On 2010/05/12 20:09:19, jlabanca wrote: LGTM http://gwt-code-reviews.appspot.com/503802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Adds a mobile-friendly drag-scroll implementation. (issue530801)
Reviewers: jlabanca, Description: Adds a mobile-friendly drag-scroll implementation. Changes the standard and mobile expense samples to use said scrolling. (desktop Expense sample still uses regular scrollbars on non-touch devices) Adds onclick='' to cell containers, so that touch devices show tap highlights in the right place. Please review this at http://gwt-code-reviews.appspot.com/530801/show Affected files: A /bikeshed/src/com/google/gwt/mobile/Mobile.gwt.xml A /bikeshed/src/com/google/gwt/mobile/client/MobileScrollPanel.java A /bikeshed/src/com/google/gwt/mobile/client/Momentum.java A /bikeshed/src/com/google/gwt/mobile/client/Point.java A /bikeshed/src/com/google/gwt/mobile/client/Scroller.java A /bikeshed/src/com/google/gwt/mobile/client/Touch.java A /bikeshed/src/com/google/gwt/mobile/client/TouchEvent.java A /bikeshed/src/com/google/gwt/mobile/client/TouchHandler.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/ExpensesCommon.gwt.xml M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpenseDetails.ui.xml M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpenseList.ui.xml M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpensesMobile.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpensesMobileShell.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpensesMobileShell.ui.xml M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpensesShell.ui.xml M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileExpenseList.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileReportList.java M /user/src/com/google/gwt/user/cellview/client/CellList.java M /user/src/com/google/gwt/user/cellview/client/CellTable.java M /user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Style tweaks to the denied popup on the Expense Details page. Also removing the 'no data' messa... (issue515801)
On 2010/05/12 22:05:03, jlabanca wrote: LGTM http://gwt-code-reviews.appspot.com/515801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: The generated data was missing an Expense - Report link. Fixed it. Updated the sample Json data. (issue512801)
On 2010/05/12 14:53:32, amitmanjhi wrote: LGTM. http://gwt-code-reviews.appspot.com/512801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Updating Mobile expenses app now that prices are stored in dollars instead of cents. (issue514801)
On 2010/05/12 16:49:01, jlabanca wrote: LGTM http://gwt-code-reviews.appspot.com/514801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Enables AppStats for bikeshed. (issue477801)
On 2010/05/06 00:26:30, knorton wrote: I have no way of testing this ... but I think it's correct. LGTM. http://gwt-code-reviews.appspot.com/477801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Clickable elements now have pointer cursor style. Fixed gray box around open/close images in Ce... (issue499801)
On 2010/05/10 15:06:17, jlabanca wrote: LGTM. http://gwt-code-reviews.appspot.com/499801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Nicer style for the mobile parts of the sample. (issue502801)
Reviewers: jlabanca, Description: Nicer style for the mobile parts of the sample. More useful mobile page stack. Added edit/save for expense entries. Please review this at http://gwt-code-reviews.appspot.com/502801/show Affected files: D /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/Controller.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpensesMobile.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpensesMobileShell.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpensesMobileShell.ui.xml A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileExpenseDetails.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileExpenseDetails.ui.xml M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileExpenseEntry.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileExpenseEntry.ui.xml M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileExpenseList.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobilePage.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileReportList.java D /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/Page.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ScaffoldMobileShell.ui.xml A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/mobile.css -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Spiffs up the expenses sample a bit. Adds half-baked page/controller system (issue491802)
Reviewers: jlabanca, Description: Spiffs up the expenses sample a bit. Adds half-baked page/controller system for managing pages, which we will need to reconcile with activities/places at some point. Review by: jlaba...@google.com Please review this at http://gwt-code-reviews.appspot.com/491802/show Affected files: A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/Controller.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpensesMobileShell.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpensesMobileShell.ui.xml D /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileExpenseDetails.java D /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileExpenseDetails.ui.xml A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileExpenseEntry.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileExpenseEntry.ui.xml M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileExpenseList.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileReportList.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/Page.java M /bikeshed/war/ExpensesMobile.html -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Moves the Cell interfaces and implementations from Bikeshed into c.g.g.cell. (issue467801)
Reviewers: jlabanca, Description: Moves the Cell interfaces and implementations from Bikeshed into c.g.g.cell. Also makes Cell an interface and changes existing cells to extend AbstractCell. Please review this at http://gwt-code-reviews.appspot.com/467801/show Affected files: D /bikeshed/src/com/google/gwt/bikeshed/cells/Cells.gwt.xml D /bikeshed/src/com/google/gwt/bikeshed/cells/client/ActionCell.java D /bikeshed/src/com/google/gwt/bikeshed/cells/client/ButtonCell.java D /bikeshed/src/com/google/gwt/bikeshed/cells/client/Cell.java D /bikeshed/src/com/google/gwt/bikeshed/cells/client/CheckboxCell.java D /bikeshed/src/com/google/gwt/bikeshed/cells/client/ClickableTextCell.java D /bikeshed/src/com/google/gwt/bikeshed/cells/client/CompositeCell.java D /bikeshed/src/com/google/gwt/bikeshed/cells/client/CurrencyCell.java D /bikeshed/src/com/google/gwt/bikeshed/cells/client/DateCell.java D /bikeshed/src/com/google/gwt/bikeshed/cells/client/DatePickerCell.java D /bikeshed/src/com/google/gwt/bikeshed/cells/client/EditTextCell.java D /bikeshed/src/com/google/gwt/bikeshed/cells/client/EllipsisCell.java D /bikeshed/src/com/google/gwt/bikeshed/cells/client/FieldUpdater.java D /bikeshed/src/com/google/gwt/bikeshed/cells/client/IconCellDecorator.java D /bikeshed/src/com/google/gwt/bikeshed/cells/client/ProfitLossCell.java D /bikeshed/src/com/google/gwt/bikeshed/cells/client/SelectionCell.java D /bikeshed/src/com/google/gwt/bikeshed/cells/client/TextCell.java D /bikeshed/src/com/google/gwt/bikeshed/cells/client/TextInputCell.java D /bikeshed/src/com/google/gwt/bikeshed/cells/client/ValueUpdater.java M /bikeshed/src/com/google/gwt/bikeshed/list/List.gwt.xml M /bikeshed/src/com/google/gwt/bikeshed/list/client/CellList.java M /bikeshed/src/com/google/gwt/bikeshed/list/client/Column.java D /bikeshed/src/com/google/gwt/bikeshed/list/client/HasCell.java M /bikeshed/src/com/google/gwt/bikeshed/list/client/HasViewData.java M /bikeshed/src/com/google/gwt/bikeshed/list/client/Header.java M /bikeshed/src/com/google/gwt/bikeshed/list/client/IdentityColumn.java M /bikeshed/src/com/google/gwt/bikeshed/list/client/TextColumn.java M /bikeshed/src/com/google/gwt/bikeshed/list/client/TextHeader.java M /bikeshed/src/com/google/gwt/bikeshed/tree/Tree.gwt.xml M /bikeshed/src/com/google/gwt/bikeshed/tree/client/CellBrowser.java M /bikeshed/src/com/google/gwt/bikeshed/tree/client/CellTreeNodeView.java M /bikeshed/src/com/google/gwt/bikeshed/tree/client/CellTreeViewModel.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/BasicTableRecipe.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/CellListRecipe.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/Cookbook.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/EditableTableRecipe.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/MailRecipe.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/MyTreeViewModel.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/SelectionColumn.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/ValidatableInputCell.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/ValidationRecipe.java M /bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/ChangeCell.java M /bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/Columns.java M /bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/HighlightingTextCell.java M /bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/PlayerScoresWidget.java A /bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/ProfitLossCell.java M /bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/StockQuoteCell.java M /bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/StocksDesktop.java M /bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/TransactionTreeViewModel.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpenseDetails.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpenseList.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpenseTree.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileExpenseList.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileReportList.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/SortableColumn.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/SortableHeader.java A /user/src/com/google/gwt/cell/Cell.gwt.xml A /user/src/com/google/gwt/cell/client/AbstractCell.java A /user/src/com/google/gwt/cell/client/ActionCell.java A /user/src/com/google/gwt/cell/client/ButtonCell.java A /user/src/com/google/gwt/cell/client/Cell.java A /user/src/com/google/gwt/cell/client/CheckboxCell.java A /user/src/com/google/gwt/cell/client/ClickableTextCell.java
[gwt-contrib] Re: Moves the Cell interfaces and implementations from Bikeshed into c.g.g.cell. (issue467801)
All nits picked, fixed, and committed. http://gwt-code-reviews.appspot.com/467801/diff/1/60 File /user/src/com/google/gwt/cell/client/ActionCell.java (right): http://gwt-code-reviews.appspot.com/467801/diff/1/60#newcode55 /user/src/com/google/gwt/cell/client/ActionCell.java:55: On 2010/05/05 16:19:51, jlabanca wrote: Should implement consumesEvent() to return true. Done. http://gwt-code-reviews.appspot.com/467801/diff/1/60#newcode59 /user/src/com/google/gwt/cell/client/ActionCell.java:59: if (mouseup.equals(event.getType())) { On 2010/05/05 16:19:51, jlabanca wrote: We should probably use click here. Done. http://gwt-code-reviews.appspot.com/467801/diff/1/64 File /user/src/com/google/gwt/cell/client/ClickableTextCell.java (right): http://gwt-code-reviews.appspot.com/467801/diff/1/64#newcode30 /user/src/com/google/gwt/cell/client/ClickableTextCell.java:30: On 2010/05/05 16:19:51, jlabanca wrote: implement consumesEvents, return true Done. http://gwt-code-reviews.appspot.com/467801/diff/1/65 File /user/src/com/google/gwt/cell/client/CompositeCell.java (right): http://gwt-code-reviews.appspot.com/467801/diff/1/65#newcode26 /user/src/com/google/gwt/cell/client/CompositeCell.java:26: * A {...@link Cell} that is composed of other {...@link AbstractCell}s. On 2010/05/05 16:19:51, jlabanca wrote: Cell, not AbstractCell. Done. http://gwt-code-reviews.appspot.com/467801/diff/1/65#newcode30 /user/src/com/google/gwt/cell/client/CompositeCell.java:30: * When this cell is rendered, it will render each component {...@link AbstractCell} inside On 2010/05/05 16:19:51, jlabanca wrote: Cell Done. http://gwt-code-reviews.appspot.com/467801/diff/1/65#newcode31 /user/src/com/google/gwt/cell/client/CompositeCell.java:31: * a span. If the component {...@link AbstractCell} uses block level elements (such as a On 2010/05/05 16:19:51, jlabanca wrote: Cell Done. http://gwt-code-reviews.appspot.com/467801/diff/1/69 File /user/src/com/google/gwt/cell/client/EditTextCell.java (right): http://gwt-code-reviews.appspot.com/467801/diff/1/69#newcode35 /user/src/com/google/gwt/cell/client/EditTextCell.java:35: On 2010/05/05 16:19:51, jlabanca wrote: Implement consumesEvents Done. http://gwt-code-reviews.appspot.com/467801/diff/1/69#newcode68 /user/src/com/google/gwt/cell/client/EditTextCell.java:68: String value; On 2010/05/05 16:19:51, jlabanca wrote: value can be declared on the next line where it is set. Done. http://gwt-code-reviews.appspot.com/467801/diff/1/70 File /user/src/com/google/gwt/cell/client/FieldUpdater.java (right): http://gwt-code-reviews.appspot.com/467801/diff/1/70#newcode32 /user/src/com/google/gwt/cell/client/FieldUpdater.java:32: * Announces a new value for a field within a base object. On 2010/05/05 16:19:51, jlabanca wrote: Add newline between description and @param tags Done. http://gwt-code-reviews.appspot.com/467801/diff/1/71 File /user/src/com/google/gwt/cell/client/HasCell.java (right): http://gwt-code-reviews.appspot.com/467801/diff/1/71#newcode20 /user/src/com/google/gwt/cell/client/HasCell.java:20: * of type T, provide a {...@link AbstractCell} to render that value, and provide a On 2010/05/05 16:19:51, jlabanca wrote: Cell, not AbstractCell Done. http://gwt-code-reviews.appspot.com/467801/diff/1/71#newcode33 /user/src/com/google/gwt/cell/client/HasCell.java:33: * Returns the {...@link AbstractCell} of type C. On 2010/05/05 16:19:51, jlabanca wrote: Cell Done. http://gwt-code-reviews.appspot.com/467801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Creates the c.g.g.view module, moving the abstract interfaces and non-widget implementations fro... (issue474801)
Reviewers: jlabanca, cromwellian, Description: Creates the c.g.g.view module, moving the abstract interfaces and non-widget implementations from the Bikeshed. Creates the (currently empty) c.g.g.user.cellview module, which will eventually hold the Bikeshed widgets. Also cleans up the dependency from bikeshed widgets to the sample Styles object. Please review this at http://gwt-code-reviews.appspot.com/474801/show Affected files: M /bikeshed/src/com/google/gwt/bikeshed/list/client/AbstractPager.java M /bikeshed/src/com/google/gwt/bikeshed/list/client/CellList.java M /bikeshed/src/com/google/gwt/bikeshed/list/client/CellTable.java M /bikeshed/src/com/google/gwt/bikeshed/list/client/Column.java D /bikeshed/src/com/google/gwt/bikeshed/list/client/HasViewData.java D /bikeshed/src/com/google/gwt/bikeshed/list/client/ListView.java M /bikeshed/src/com/google/gwt/bikeshed/list/client/PageSizePager.java D /bikeshed/src/com/google/gwt/bikeshed/list/client/PagingListView.java M /bikeshed/src/com/google/gwt/bikeshed/list/client/SimplePager.java M /bikeshed/src/com/google/gwt/bikeshed/list/client/impl/CellListImpl.java D /bikeshed/src/com/google/gwt/bikeshed/list/shared/AbstractListViewAdapter.java D /bikeshed/src/com/google/gwt/bikeshed/list/shared/AsyncListViewAdapter.java D /bikeshed/src/com/google/gwt/bikeshed/list/shared/DefaultSelectionModel.java D /bikeshed/src/com/google/gwt/bikeshed/list/shared/ListViewAdapter.java D /bikeshed/src/com/google/gwt/bikeshed/list/shared/MultiSelectionModel.java D /bikeshed/src/com/google/gwt/bikeshed/list/shared/ProvidesKey.java D /bikeshed/src/com/google/gwt/bikeshed/list/shared/Range.java D /bikeshed/src/com/google/gwt/bikeshed/list/shared/SelectionModel.java D /bikeshed/src/com/google/gwt/bikeshed/list/shared/SingleSelectionModel.java M /bikeshed/src/com/google/gwt/bikeshed/tree/client/CellBrowser.java M /bikeshed/src/com/google/gwt/bikeshed/tree/client/CellTree.css M /bikeshed/src/com/google/gwt/bikeshed/tree/client/CellTree.java M /bikeshed/src/com/google/gwt/bikeshed/tree/client/CellTreeNodeView.java D /bikeshed/src/com/google/gwt/bikeshed/tree/client/CellTreeViewModel.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/BasicTableRecipe.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/CellBrowserRecipe.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/CellListRecipe.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/CellTreeRecipe.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/Cookbook.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/EditableTableRecipe.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/MailRecipe.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/MyTreeViewModel.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/ScrollbarPager.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/SelectionColumn.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/SimplePager.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/ValidatableInputCell.java M /bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/ValidationRecipe.java M /bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/FavoritesWidget.java M /bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/PlayerScoresWidget.java M /bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/StockQueryWidget.java M /bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/StockService.java M /bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/StockServiceAsync.java M /bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/StocksDesktop.java M /bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/StocksMobile.java M /bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/TransactionTreeViewModel.java M /bikeshed/src/com/google/gwt/sample/bikeshed/stocks/server/StockServiceImpl.java M /bikeshed/src/com/google/gwt/sample/bikeshed/stocks/shared/StockQuote.java M /bikeshed/src/com/google/gwt/sample/bikeshed/stocks/shared/StockRequest.java M /bikeshed/src/com/google/gwt/sample/bikeshed/style/client/Styles.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpenseDetails.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpenseList.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpenseTree.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/Expenses.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileExpenseList.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileReportList.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeListActivity.java M
[gwt-contrib] Rough first pass at something vaguely resembling mobile versions of the (issue392804)
Reviewers: Dan Rice, Description: Rough first pass at something vaguely resembling mobile versions of the scaffold and customized expense apps. Please review this at http://gwt-code-reviews.appspot.com/392804/show Affected files: M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpensesMobile.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpensesMobileShell.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpensesMobileShell.ui.xml A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileExpenseDetails.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileExpenseDetails.ui.xml A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileExpenseList.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/MobileReportList.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ScaffoldActivities.java M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ScaffoldMobile.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ScaffoldMobileShell.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ScaffoldMobileShell.ui.xml M /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/place/ScaffoldPlaceFilter.java M /bikeshed/war/ExpensesMobile.html M /bikeshed/war/ScaffoldMobile.html -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Rough first pass at something vaguely resembling mobile versions of the (issue392804)
http://gwt-code-reviews.appspot.com/392804/diff/1/3 File /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpensesMobileShell.java (right): http://gwt-code-reviews.appspot.com/392804/diff/1/3#newcode34 /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpensesMobileShell.java:34: * TODO On 2010/04/30 21:02:08, Dan Rice wrote: TODO what? Doc. Just trying to make checkstyle happy. http://gwt-code-reviews.appspot.com/392804/diff/1/9 File /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ScaffoldActivities.java (right): http://gwt-code-reviews.appspot.com/392804/diff/1/9#newcode68 /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ScaffoldActivities.java:68: // public Activity filter(EntityListPlace place) { On 2010/04/30 21:02:08, Dan Rice wrote: Remove comment or add TODO Whoops, this shouldn't have been included. http://gwt-code-reviews.appspot.com/392804/diff/1/13 File /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/place/ScaffoldPlaceFilter.java (right): http://gwt-code-reviews.appspot.com/392804/diff/1/13#newcode25 /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/place/ScaffoldPlaceFilter.java:25: // T filter(ScaffoldMobile.EntityListPlace place); On 2010/04/30 21:02:08, Dan Rice wrote: Remove or comment Also whoops. Fixed. http://gwt-code-reviews.appspot.com/392804/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Removing the trivially simple SimpleColumn. (issue420802)
On 2010/04/28 21:32:26, jlabanca wrote: LGTM. http://gwt-code-reviews.appspot.com/420802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Another pass at refactoring the client code. (issue412802)
Reviewers: Ray Ryan, Description: Another pass at refactoring the client code. Please review this at http://gwt-code-reviews.appspot.com/412802/show Affected files: M /bikeshed/src/com/google/gwt/sample/expenses/gwt/Expenses.gwt.xml M /bikeshed/src/com/google/gwt/sample/expenses/gwt/ExpensesCommon.gwt.xml M /bikeshed/src/com/google/gwt/sample/expenses/gwt/ExpensesMobile.gwt.xml D /bikeshed/src/com/google/gwt/sample/expenses/gwt/ExpensesMobileScaffold.gwt.xml D /bikeshed/src/com/google/gwt/sample/expenses/gwt/ExpensesScaffold.gwt.xml A /bikeshed/src/com/google/gwt/sample/expenses/gwt/Scaffold.gwt.xml A /bikeshed/src/com/google/gwt/sample/expenses/gwt/ScaffoldCommon.gwt.xml A /bikeshed/src/com/google/gwt/sample/expenses/gwt/ScaffoldMobile.gwt.xml A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/EditorView.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/EmployeeList.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpenseBrowser.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpenseBrowser.ui.xml A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpenseDetails.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpenseDetails.ui.xml A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpenseList.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpenseList.ui.xml A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/Expenses.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpensesMobile.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpensesShell.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ExpensesShell.ui.xml A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/Scaffold.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ScaffoldActivities.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ScaffoldMobile.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ScaffoldShell.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/ScaffoldShell.ui.xml A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/place/BaseScaffoldPlaceProcessor.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/place/EmployeeScaffoldPlace.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/place/ListScaffoldPlace.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/place/ReportScaffoldPlace.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/place/ScaffoldPlace.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/place/ScaffoldPlaceFilter.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/place/ScaffoldPlaceProcessor.java A /bikeshed/src/com/google/gwt/sample/expenses/gwt/client/place/ScaffoldRecordPlace.java D /bikeshed/src/com/google/gwt/sample/expenses/gwt/customized/CustomizedShell.java D /bikeshed/src/com/google/gwt/sample/expenses/gwt/customized/CustomizedShell.ui.xml D /bikeshed/src/com/google/gwt/sample/expenses/gwt/customized/EmployeeList.java D /bikeshed/src/com/google/gwt/sample/expenses/gwt/customized/ExpenseBrowser.java D /bikeshed/src/com/google/gwt/sample/expenses/gwt/customized/ExpenseBrowser.ui.xml D /bikeshed/src/com/google/gwt/sample/expenses/gwt/customized/ExpenseDetails.java D /bikeshed/src/com/google/gwt/sample/expenses/gwt/customized/ExpenseDetails.ui.xml D /bikeshed/src/com/google/gwt/sample/expenses/gwt/customized/ExpenseList.java D /bikeshed/src/com/google/gwt/sample/expenses/gwt/customized/ExpenseList.ui.xml D /bikeshed/src/com/google/gwt/sample/expenses/gwt/customized/Expenses.java D /bikeshed/src/com/google/gwt/sample/expenses/gwt/customized/ExpensesMobile.java D /bikeshed/src/com/google/gwt/sample/expenses/gwt/scaffold/EditorView.java D /bikeshed/src/com/google/gwt/sample/expenses/gwt/scaffold/ExpensesMobileScaffold.java D /bikeshed/src/com/google/gwt/sample/expenses/gwt/scaffold/ExpensesScaffold.java D /bikeshed/src/com/google/gwt/sample/expenses/gwt/scaffold/Scaffold.java D /bikeshed/src/com/google/gwt/sample/expenses/gwt/scaffold/ScaffoldActivities.java D /bikeshed/src/com/google/gwt/sample/expenses/gwt/scaffold/ScaffoldShell.java D /bikeshed/src/com/google/gwt/sample/expenses/gwt/scaffold/ScaffoldShell.ui.xml D /bikeshed/src/com/google/gwt/sample/expenses/gwt/scaffold/place/BaseScaffoldPlaceProcessor.java D /bikeshed/src/com/google/gwt/sample/expenses/gwt/scaffold/place/EmployeeScaffoldPlace.java D /bikeshed/src/com/google/gwt/sample/expenses/gwt/scaffold/place/ListScaffoldPlace.java D /bikeshed/src/com/google/gwt/sample/expenses/gwt/scaffold/place/ReportScaffoldPlace.java D /bikeshed/src/com/google/gwt/sample/expenses/gwt/scaffold/place/ScaffoldPlace.java D
[gwt-contrib] Re: Templating the Expenses sample. (issue398802)
On 2010/04/27 17:30:52, jlabanca wrote: LGTM, but you're going to have some merge conflicts with the ViewData nuking patch I just committed. http://gwt-code-reviews.appspot.com/398802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixes a bug in SplitLayoutPanel where calling setWidgetMinSize() on the center widget, the last ... (issue378802)
LGTM http://gwt-code-reviews.appspot.com/378802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Reorganizes styles in bikeshed to use CssResource and ClientBundle. (issue388801)
Reviewers: jlabanca, Description: Reorganizes styles in bikeshed to use CssResource and ClientBundle. Moves all styles in stock and cookbook samples to one place. Removes the hovering boxes we stole from Wave for a slightly flatter look. Please review this at http://gwt-code-reviews.appspot.com/388801/show Affected files: M bikeshed/src/com/google/gwt/bikeshed/list/client/PagingTableListView.java M bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/Cookbook.gwt.xml M bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/Cookbook.java M bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/Cookbook.ui.xml M bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/MailRecipe.java M bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/Recipe.java M bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/ValidationRecipe.java D bikeshed/src/com/google/gwt/sample/bikeshed/cookbook/client/common.css M bikeshed/src/com/google/gwt/sample/bikeshed/stocks/StocksCommon.gwt.xml M bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/FavoritesWidget.ui.xml M bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/PlayerScoresWidget.ui.xml M bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/StockQueryWidget.ui.xml M bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/StocksDesktop.ui.xml M bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/StocksMobile.ui.xml D bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/common.css A bikeshed/src/com/google/gwt/sample/bikeshed/style/Style.gwt.xml A bikeshed/src/com/google/gwt/sample/bikeshed/style/client/Styles.java A bikeshed/src/com/google/gwt/sample/bikeshed/style/client/common.css D bikeshed/war/Cookbook.css M bikeshed/war/Cookbook.html D bikeshed/war/Stocks.css M bikeshed/war/Stocks.html M bikeshed/war/StocksMobile.html -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Refactoring Tree code. SideBySideTreeView now uses SimpleCellList, which a protected method tha... (issue390801)
http://gwt-code-reviews.appspot.com/390801/diff/1/3 File bikeshed/src/com/google/gwt/bikeshed/cells/client/CompositeCell.java (right): http://gwt-code-reviews.appspot.com/390801/diff/1/3#newcode32 bikeshed/src/com/google/gwt/bikeshed/cells/client/CompositeCell.java:32: public class CompositeCellC, V extends CellC, V { One thing we should be very clear about is that CompositeCell always concatenates its child cells, and makes no guarantees about how they're likely to lay out. IOW, if a child cell uses a block-level element, they'll stack up. This might be reasonable, but we should make it clear. http://gwt-code-reviews.appspot.com/390801/diff/1/3#newcode49 bikeshed/src/com/google/gwt/bikeshed/cells/client/CompositeCell.java:49: public boolean consumesEvents() { We've never been clear on whether these methods could return different values at different times -- I'm leaning towards *not*, because that would allow a CompositeCell to cache these results (there's no particular reason to expect a container to cache this value on behalf of its Cells, so it might be called very frequently). http://gwt-code-reviews.appspot.com/390801/diff/1/3#newcode59 bikeshed/src/com/google/gwt/bikeshed/cells/client/CompositeCell.java:59: public boolean dependsOnSelection() { Ditto above about caching results. http://gwt-code-reviews.appspot.com/390801/diff/1/5 File bikeshed/src/com/google/gwt/bikeshed/list/client/ListView.java (right): http://gwt-code-reviews.appspot.com/390801/diff/1/5#newcode59 bikeshed/src/com/google/gwt/bikeshed/list/client/ListView.java:59: void setSelectionModel(SelectionModel? super T selectionModel); It's debatable whether all lists should be required to implement selection. We can go ahead and check it in this way, but I'd like to think about it before fully committing to this idea. http://gwt-code-reviews.appspot.com/390801/diff/1/7 File bikeshed/src/com/google/gwt/bikeshed/list/client/SimpleCellList.java (right): http://gwt-code-reviews.appspot.com/390801/diff/1/7#newcode51 bikeshed/src/com/google/gwt/bikeshed/list/client/SimpleCellList.java:51: private static final String STYLENNAME_SELECTED = gwt-sstree-selectedItem; I assume that these classnames are temporary since this widget is being used in the sstree. We can revisit the way we handle styling later, but a TODO might be a good idea. http://gwt-code-reviews.appspot.com/390801/diff/1/7#newcode122 bikeshed/src/com/google/gwt/bikeshed/list/client/SimpleCellList.java:122: int type = event.getTypeInt(); When Dan gets done factoring out the pager concept, I suppose we can eliminate all this built-in button stuff. http://gwt-code-reviews.appspot.com/390801/diff/1/8 File bikeshed/src/com/google/gwt/bikeshed/list/client/impl/SimpleCellListImpl.java (right): http://gwt-code-reviews.appspot.com/390801/diff/1/8#newcode42 bikeshed/src/com/google/gwt/bikeshed/list/client/impl/SimpleCellListImpl.java:42: public abstract class SimpleCellListImplT { If this shouldn't be used, consider making it package protected if possible. http://gwt-code-reviews.appspot.com/390801/diff/1/8#newcode297 bikeshed/src/com/google/gwt/bikeshed/list/client/impl/SimpleCellListImpl.java:297: + 'iloading.../i/div); At some point we'll either have to make this string overridable or turn it into something more i18n-friendly. http://gwt-code-reviews.appspot.com/390801/diff/1/18 File bikeshed/src/com/google/gwt/bikeshed/tree/client/TreeView.java (right): http://gwt-code-reviews.appspot.com/390801/diff/1/18#newcode28 bikeshed/src/com/google/gwt/bikeshed/tree/client/TreeView.java:28: public abstract class TreeView extends Composite { Sure is starting to smell like this class can be nuked... http://gwt-code-reviews.appspot.com/390801/diff/1/19 File bikeshed/src/com/google/gwt/bikeshed/tree/client/TreeViewModel.java (right): http://gwt-code-reviews.appspot.com/390801/diff/1/19#newcode34 bikeshed/src/com/google/gwt/bikeshed/tree/client/TreeViewModel.java:34: class DefaultNodeInfoT implements NodeInfoT { I really like the fact that much of the code is gone from this class. It sure seemed like a symptom that the NodeInfo contract was too complex before. http://gwt-code-reviews.appspot.com/390801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Implement a selection column, sortable columns, and row hovering in MailRecipe (issue356801)
On 2010/04/16 21:30:35, Dan Rice wrote: LGTM http://gwt-code-reviews.appspot.com/356801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixes incorrect direction of isAssignable check when using @UiField(provided = true) (issue359801)
On 2010/04/18 21:38:06, Ray Ryan wrote: Joel, if you can get to this first thing that would be great. Bob, if you're in a charitable mood and get there sooner, that would be great too. LGTM. http://gwt-code-reviews.appspot.com/359801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix for issue 4851 - error in javadoc uibinder example for TabLayoutPanel (issue329802)
On 2010/04/12 20:30:19, bduff wrote: LGTM, thanks! http://gwt-code-reviews.appspot.com/329802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors To unsubscribe, reply using remove me as the subject.
[gwt-contrib] Re: Changes CustomizedShell to use the new Table widget. (issue318801)
http://gwt-code-reviews.appspot.com/318801/diff/3001/4002 File bikeshed/src/com/google/gwt/bikeshed/cells/client/EditTextCell.java (right): http://gwt-code-reviews.appspot.com/318801/diff/3001/4002#newcode8 bikeshed/src/com/google/gwt/bikeshed/cells/client/EditTextCell.java:8: public class EditTextCell extends CellString, String { On 2010/04/08 19:53:38, Ray Ryan wrote: Notice how this cell has to implement totally different behavior depending upon if there is view data or not? So that it basically should be two different classes, like EditTextCell and EditTextViewDataCell? There must be something we can do to simplify this stuff. Hmm. Actually, should it be as simple as that? AbstractTextCellString, V provides render, edit, cancel and commit. EditTextValueDataCell extends AbstractTextCellString, String EditTextCell extends AbstractTextCellString, Void I hope we can find a way to make ViewData into some kind of add-on. Actually, the ViewData is used to describe the cell's state, so it's not really two separate classes. When there's view data, it's being edited. I factored out two separate event handler methods that are dispatched to from the main event handler which should make it a little clearer. I'm not 100% certain I like this whole approach, but it's a starting point. http://gwt-code-reviews.appspot.com/318801/diff/3001/4002#newcode20 bikeshed/src/com/google/gwt/bikeshed/cells/client/EditTextCell.java:20: return cancel(parent, value); On 2010/04/08 19:53:38, Ray Ryan wrote: The returns and the elses are redundant, contributing only this nesting that makes things hard to read. Can you lose the elses where you can? Cleaned up if/else stuff (factored out two separate methods for the 'editing' vs. 'non-editing' cases. Also removed the 'blur' code, which doesn't work yet anyway. http://gwt-code-reviews.appspot.com/318801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Changes CustomizedShell to use the new Table widget. (issue318801)
Reviewers: Ray Ryan, Description: Changes CustomizedShell to use the new Table widget. Please review this at http://gwt-code-reviews.appspot.com/318801/show Affected files: M bikeshed/src/com/google/gwt/bikeshed/cells/client/DateCell.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/customized/Customized.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/customized/CustomizedShell.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/customized/CustomizedShell.ui.xml -- http://groups.google.com/group/Google-Web-Toolkit-Contributors To unsubscribe, reply using remove me as the subject.
[gwt-contrib] Re: Changes CustomizedShell to use the new Table widget. (issue318801)
http://gwt-code-reviews.appspot.com/318801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors To unsubscribe, reply using remove me as the subject.
[gwt-contrib] Changes ListRegistration to carry information about its associated handler (issue314801)
Reviewers: Ray Ryan, Description: Changes ListRegistration to carry information about its associated handler and range of interest. This allows ListListModel (and theoretically other models) to call back directly to views whose range of interest changes without having to re-render all views. Please review this at http://gwt-code-reviews.appspot.com/314801/show Affected files: M bikeshed/src/com/google/gwt/bikeshed/list/client/PagingTableListView.java M bikeshed/src/com/google/gwt/bikeshed/list/client/SimpleCellList.java M bikeshed/src/com/google/gwt/bikeshed/list/shared/AbstractListModel.java M bikeshed/src/com/google/gwt/bikeshed/list/shared/AsyncListModel.java M bikeshed/src/com/google/gwt/bikeshed/list/shared/ListListModel.java M bikeshed/src/com/google/gwt/bikeshed/list/shared/ListModel.java M bikeshed/src/com/google/gwt/bikeshed/list/shared/ListRegistration.java M bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/StocksDesktop.java M bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/StocksMobile.java M bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/TransactionTreeViewModel.java M bikeshed/src/com/google/gwt/sample/bikeshed/tree/TreeSample.gwt.xml M bikeshed/src/com/google/gwt/sample/bikeshed/tree/client/MyTreeViewModel.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/employee/EmployeeListView.java M bikeshed/src/com/google/gwt/sample/expenses/gwt/ui/report/ReportListView.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors To unsubscribe, reply using remove me as the subject.
[gwt-contrib] Re: Adds ProvidesKey and makes the ListModel the source of the key (issue307801)
On 2010/04/02 21:22:23, jlabanca wrote: LGTM http://gwt-code-reviews.appspot.com/307801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add convenience classes and method to simplify demo code. (issue299801)
On 2010/04/01 18:31:20, Dan Rice wrote: LGTM http://gwt-code-reviews.appspot.com/299801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors To unsubscribe, reply using remove me as the subject.
[gwt-contrib] Fixes a list subset error in AbstractListModel that was breaking paging. (issue293801)
Reviewers: ruce_google.com, Description: Fixes a list subset error in AbstractListModel that was breaking paging. Changes ListListModel's deferred command to be more careful about clearing its pending state (so that an exception won't break it permanently). Please review this at http://gwt-code-reviews.appspot.com/293801/show Affected files: M bikeshed/src/com/google/gwt/bikeshed/list/shared/AbstractListModel.java M bikeshed/src/com/google/gwt/bikeshed/list/shared/ListListModel.java Index: bikeshed/src/com/google/gwt/bikeshed/list/shared/AbstractListModel.java === --- bikeshed/src/com/google/gwt/bikeshed/list/shared/AbstractListModel.java (revision 7826) +++ bikeshed/src/com/google/gwt/bikeshed/list/shared/AbstractListModel.java (working copy) @@ -159,7 +159,8 @@ int realStart = curStart start ? start : curStart; int realEnd = curEnd end ? end : curEnd; int realLength = realEnd - realStart; -ListT realValues = values.subList(0, realLength); +ListT realValues = values.subList(realStart - start, +realStart - start + realLength); ListEventT event = new ListEventT(realStart, realLength, realValues); reg.getHandler().onDataChanged(event); } Index: bikeshed/src/com/google/gwt/bikeshed/list/shared/ListListModel.java === --- bikeshed/src/com/google/gwt/bikeshed/list/shared/ListListModel.java (revision 7826) +++ bikeshed/src/com/google/gwt/bikeshed/list/shared/ListListModel.java (working copy) @@ -48,6 +48,8 @@ */ private Command flushCommand = new Command() { public void execute() { +flushPending = false; + int newSize = list.size(); if (curSize != newSize) { curSize = newSize; @@ -61,7 +63,6 @@ } minModified = Integer.MAX_VALUE; maxModified = Integer.MIN_VALUE; -flushPending = false; } }; -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Separates the stock sample into desktop and mobile versions. Makes some (issue276801)
Reviewers: jlabanca, Description: Separates the stock sample into desktop and mobile versions. Makes some other not-quite-perfect changes to sxs-tree layout. Please review this at http://gwt-code-reviews.appspot.com/276801/show Affected files: M bikeshed/src/com/google/gwt/bikeshed/tree/client/SideBySideTreeNodeView.java M bikeshed/src/com/google/gwt/bikeshed/tree/client/SideBySideTreeView.java D bikeshed/src/com/google/gwt/sample/bikeshed/stocks/StockSample.gwt.xml A bikeshed/src/com/google/gwt/sample/bikeshed/stocks/StocksCommon.gwt.xml A bikeshed/src/com/google/gwt/sample/bikeshed/stocks/StocksDesktop.gwt.xml A bikeshed/src/com/google/gwt/sample/bikeshed/stocks/StocksMobile.gwt.xml M bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/Columns.java M bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/PlayerScoresWidget.java D bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/StockSample.java D bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/StockSample.ui.xml A bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/StocksDesktop.java A bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/StocksDesktop.ui.xml A bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/StocksMobile.java A bikeshed/src/com/google/gwt/sample/bikeshed/stocks/client/StocksMobile.ui.xml M bikeshed/src/com/google/gwt/sample/bikeshed/stocks/server/StockServiceImpl.java M bikeshed/src/com/google/gwt/sample/bikeshed/tree/client/TreeSample.java M bikeshed/war/Stocks.html M bikeshed/war/WEB-INF/web.xml -- http://groups.google.com/group/Google-Web-Toolkit-Contributors To unsubscribe from this group, send email to google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with the words REMOVE ME as the subject.
[gwt-contrib] Re: Separates the stock sample into desktop and mobile versions. Makes some (issue276801)
http://gwt-code-reviews.appspot.com/276801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors To unsubscribe from this group, send email to google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with the words REMOVE ME as the subject.
[gwt-contrib] Re: Separates the stock sample into desktop and mobile versions. Makes some (issue276801)
On 2010/03/26 15:37:13, jgw wrote: Yup. Fixed. http://gwt-code-reviews.appspot.com/276801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors To unsubscribe from this group, send email to google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with the words REMOVE ME as the subject.
[gwt-contrib] Re: Add 'view data' to cell, column, and updater classes. (issue248801)
On 2010/03/19 20:26:43, Dan Rice wrote: LGTM. http://gwt-code-reviews.appspot.com/248801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors To unsubscribe from this group, send email to google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with the words REMOVE ME as the subject.
[gwt-contrib] Re: Issue 4720: PopupPanel#removeFromParent doesn't remove glass
On 2010/03/16 15:48:26, jlabanca wrote: LGTM. This thing's getting complicated, but I don't see any way around it. http://gwt-code-reviews.appspot.com/220801 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Fix for issue 4532. Removes the CSS expressions from PopupImplIE6 that were (issue238801)
Reviewers: jlabanca, Description: Fix for issue 4532. Removes the CSS expressions from PopupImplIE6 that were interfering with layout on IE7. Please review this at http://gwt-code-reviews.appspot.com/238801/show Affected files: M user/src/com/google/gwt/user/client/ui/impl/PopupImplIE6.java M user/test/com/google/gwt/user/client/ui/LayoutPanelTest.java Index: user/src/com/google/gwt/user/client/ui/impl/PopupImplIE6.java === --- user/src/com/google/gwt/user/client/ui/impl/PopupImplIE6.java (revision 7743) +++ user/src/com/google/gwt/user/client/ui/impl/PopupImplIE6.java (working copy) @@ -33,6 +33,8 @@ frame.parentElement.removeChild(frame); frame.__popup = null; popup.__frame = null; + popup.onresize = null; + popup.onmove = null; } }-*/; @@ -60,7 +62,7 @@ // Visibility of frame should match visiblity of popup element. style.visibility = popup.currentStyle.visibility; - + // Issue 2443: remove styles that affect the size of the iframe style.border = 0; style.padding = 0; @@ -74,14 +76,18 @@ style.zIndex = popup.currentStyle.zIndex; // updates position and dimensions as popup is moved resized -style.setExpression('left', 'this.__popup.offsetLeft'); -style.setExpression('top', 'this.__popup.offsetTop'); -style.setExpression('width', 'this.__popup.offsetWidth'); -style.setExpression('height', 'this.__popup.offsetHeight'); +popup.onmove = function() { + frame.style.left = popup.offsetLeft; + frame.style.top = popup.offsetTop; +}; +popup.onresize = function() { + frame.style.width = popup.offsetWidth; + frame.style.height = popup.offsetHeight; +}; style.setExpression('zIndex', 'this.__popup.currentStyle.zIndex'); popup.parentElement.insertBefore(frame, popup); }-*/; - + @Override public native void setVisible(Element popup, boolean visible) /*-{ if (popup.__frame) { Index: user/test/com/google/gwt/user/client/ui/LayoutPanelTest.java === --- user/test/com/google/gwt/user/client/ui/LayoutPanelTest.java (revision 7743) +++ user/test/com/google/gwt/user/client/ui/LayoutPanelTest.java (working copy) @@ -15,6 +15,7 @@ */ package com.google.gwt.user.client.ui; +import com.google.gwt.dom.client.Document; import com.google.gwt.dom.client.Style.Unit; import com.google.gwt.layout.client.Layout.AnimationCallback; import com.google.gwt.layout.client.Layout.Layer; @@ -55,4 +56,31 @@ } }); } + + /** + * Ensures that the popup implementation doesn't interfere with layout. This + * cropped up on IE7 as a result of CSS expressions used in PopupImplIE6, as + * described in issue 4532. + */ + public void testWeirdPopupInteraction() { +assertTrue(Document.get().isCSS1Compat()); + +final LayoutPanel lp = new LayoutPanel(); +lp.add(new HTML(foo)); +RootLayoutPanel.get().add(lp); + +PopupPanel popup = new PopupPanel(); +popup.center(); + +delayTestFinish(2000); +DeferredCommand.addCommand(new Command() { + public void execute() { +int offsetWidth = lp.getOffsetWidth(); +int offsetHeight = lp.getOffsetHeight(); +assertTrue(offsetWidth 0); +assertTrue(offsetHeight 0); +finishTest(); + } +}); + } } -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix for issue 4532. Removes the CSS expressions from PopupImplIE6 that were (issue238801)
On 2010/03/18 19:03:29, jlabanca wrote: LGTM Committed at r7745. http://gwt-code-reviews.appspot.com/238801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors To unsubscribe from this group, send email to google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with the words REMOVE ME as the subject.
[gwt-contrib] Re: Don't ever set the base variable to null. It is supposed to be the empty string
On 2010/03/16 15:49:26, Lex wrote: I'm working on a test case for this tricky code, but it's going to take several hours to put together. Shall we put in the one-line fixes? The selection scripts as they are cause null pointer dereferences in some situations. That sounds fine to me for now. Tests would be good, but we can always do them as two separate commits. http://gwt-code-reviews.appspot.com/219801 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Remove caching of style sheets in IE6 (cache may be invalid when there are multiple modules)
On 2010/03/08 19:43:13, Dan Rice wrote: I added a new patch that caches the lengths but not the actual style sheet contents. In the worst case, if the lengths are wrong we will just append in a suboptimal way, but not lose any data. Benchmarking shows that the performance difference is minor -- still just 2-3 ms per injection in my particular benchmark. On 2010/03/05 17:19:51, jlabanca wrote: http://gwt-code-reviews.appspot.com/159804/diff/1/3 File user/src/com/google/gwt/dom/client/StyleInjector.java (right): http://gwt-code-reviews.appspot.com/159804/diff/1/3#newcode101 Line 101: private static native int getDocumentStyleSheetLength(int idx) /*-{ I don't know for sure if reading cssText.length is slow, but I'm worried that we had a reason to cache the length originally. I agree that there is no way to cache the info because we can't control what happens outside of GWT, so we wouldn't even know if external code modified the style sheets. Can you just try injecting 40 large style sheets and compare the performance before and after the change? At the very least, we should warn people if its going to get really slow. LGTM. http://gwt-code-reviews.appspot.com/159804 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors