[gwt-contrib] Re: GWT support for Chrome Frame (issue1449808)

2011-05-31 Thread jgw

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)

2011-03-02 Thread jgw

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)

2011-01-25 Thread jgw

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)

2011-01-21 Thread jgw

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)

2010-11-24 Thread jgw

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)

2010-11-23 Thread jgw

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)

2010-11-12 Thread jgw

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)

2010-11-08 Thread jgw

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)

2010-11-05 Thread jgw

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)

2010-11-04 Thread jgw


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)

2010-11-03 Thread jgw

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)

2010-11-03 Thread jgw

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)

2010-10-27 Thread jgw

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)

2010-10-26 Thread jgw

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)

2010-10-14 Thread jgw

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)

2010-10-12 Thread jgw

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)

2010-10-11 Thread jgw

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)

2010-09-27 Thread jgw

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)

2010-09-24 Thread jgw

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)

2010-09-14 Thread jgw

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)

2010-09-14 Thread jgw

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)

2010-09-03 Thread jgw

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)

2010-09-03 Thread jgw

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)

2010-09-03 Thread jgw

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)

2010-08-31 Thread jgw

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)

2010-08-20 Thread jgw

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)

2010-08-20 Thread jgw

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)

2010-08-13 Thread jgw

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)

2010-08-03 Thread jgw

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)

2010-08-02 Thread jgw


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)

2010-07-30 Thread jgw

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)

2010-07-02 Thread jgw

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)

2010-07-02 Thread jgw

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)

2010-07-02 Thread jgw

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)

2010-07-02 Thread jgw


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)

2010-07-02 Thread jgw


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)

2010-07-02 Thread jgw

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)

2010-07-02 Thread jgw


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)

2010-07-02 Thread jgw

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)

2010-07-02 Thread jgw


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)

2010-07-02 Thread jgw

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)

2010-07-02 Thread jgw

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)

2010-07-02 Thread jgw

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)

2010-07-01 Thread jgw

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)

2010-06-30 Thread jgw

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)

2010-06-28 Thread jgw

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)

2010-06-23 Thread jgw

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)

2010-06-23 Thread jgw

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)

2010-06-23 Thread jgw

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)

2010-06-21 Thread jgw

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)

2010-06-21 Thread jgw

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)

2010-06-17 Thread jgw

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)

2010-06-07 Thread jgw

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)

2010-06-03 Thread jgw

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)

2010-06-03 Thread jgw

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)

2010-06-03 Thread jgw

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)

2010-06-03 Thread jgw

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)

2010-06-02 Thread jgw

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)

2010-05-25 Thread jgw

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)

2010-05-25 Thread jgw

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)

2010-05-25 Thread jgw

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)

2010-05-14 Thread jgw

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)

2010-05-14 Thread jgw

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)

2010-05-13 Thread jgw

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)

2010-05-12 Thread jgw

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)

2010-05-12 Thread jgw

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)

2010-05-10 Thread jgw

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)

2010-05-10 Thread jgw

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)

2010-05-10 Thread jgw

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)

2010-05-07 Thread jgw

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)

2010-05-05 Thread jgw

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)

2010-05-05 Thread jgw

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)

2010-05-05 Thread jgw

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)

2010-04-30 Thread jgw

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)

2010-04-30 Thread jgw


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)

2010-04-28 Thread jgw

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)

2010-04-28 Thread jgw

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)

2010-04-27 Thread jgw

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)

2010-04-22 Thread jgw

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)

2010-04-22 Thread jgw

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)

2010-04-22 Thread jgw


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)

2010-04-19 Thread jgw

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)

2010-04-19 Thread jgw

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)

2010-04-12 Thread jgw

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)

2010-04-08 Thread jgw


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)

2010-04-07 Thread jgw

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)

2010-04-07 Thread jgw

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)

2010-04-06 Thread jgw

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)

2010-04-05 Thread jgw

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)

2010-04-01 Thread jgw

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)

2010-03-31 Thread jgw

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)

2010-03-26 Thread jgw

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)

2010-03-26 Thread jgw

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)

2010-03-26 Thread jgw

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)

2010-03-22 Thread jgw

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

2010-03-18 Thread jgw

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)

2010-03-18 Thread jgw

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)

2010-03-18 Thread jgw

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

2010-03-16 Thread jgw

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)

2010-03-09 Thread jgw

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


  1   2   3   4   >