Sorry for the delay - LGTM
On 2011/05/04 21:24:23, jat wrote:
Ping
http://gwt-code-reviews.appspot.com/1425816/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Revision: 10140
Author: rdcas...@google.com
Date: Wed May 4 12:02:05 2011
Log: Fix error in usage of newly-creted helper method in
AttachableHTMLPanel, correct double-calling of wrapElement in exceptional
case, and fix the documentation.
Review at http://gwt-code-reviews.appspot.
committed as r10139
http://gwt-code-reviews.appspot.com/1432801/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Revision: 10139
Author: jlaba...@google.com
Date: Wed May 4 11:30:38 2011
Log: Adds user authentication to MobileWebApp. Tasks are now specific
to each user. Authentication is done using a Filter, which was copied
almost verbatim from the expenses sample. If a user is not logged
LGTM
http://gwt-code-reviews.appspot.com/1427812/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Ping
http://gwt-code-reviews.appspot.com/1425816/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
committed as r10138
http://gwt-code-reviews.appspot.com/1420811/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM
On Wed, May 4, 2011 at 1:10 PM, wrote:
>
>
> http://gwt-code-reviews.appspot.com/1432801/diff/4001/samples/mobilewebapp/war/WEB-INF/web.xml
> File samples/mobilewebapp/war/WEB-INF/web.xml (right):
>
>
> http://gwt-code-reviews.appspot.com/1432801/diff/4001/samples/mobilewebapp/war/WEB-INF/
http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DragEndEvent.java
File user/src/com/google/gwt/event/dom/client/DragEndEvent.java (right):
http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DragEndEvent.ja
LGTM
http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DragEndEvent.java
File user/src/com/google/gwt/event/dom/client/DragEndEvent.java (right):
http://gwt-code-reviews.appspot.com/1420811/diff/2001/user/src/com/google/gwt/event/dom/client/DragEndEv
LGTM if you make OrientationMonitor.provider() reference a singleton
boolean instead of recalculating.
http://gwt-code-reviews.appspot.com/1433801/diff/3001/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/client/ClientFactory.java
File
samples/mobilewebapp/src/main/com/google/gw
http://gwt-code-reviews.appspot.com/1432801/diff/4001/samples/mobilewebapp/war/WEB-INF/web.xml
File samples/mobilewebapp/war/WEB-INF/web.xml (right):
http://gwt-code-reviews.appspot.com/1432801/diff/4001/samples/mobilewebapp/war/WEB-INF/web.xml#newcode11
samples/mobilewebapp/war/WEB-INF/web.xml:
http://gwt-code-reviews.appspot.com/1432801/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1432801/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1432801/diff/4001/samples/mobilewebapp/war/WEB-INF/web.xml
File samples/mobilewebapp/war/WEB-INF/web.xml (right):
http://gwt-code-reviews.appspot.com/1432801/diff/4001/samples/mobilewebapp/war/WEB-INF/web.xml#newcode11
samples/mobilewebapp/war/WEB-INF/web.xml:
We now throw an exception instead of trying to work around the problem.
The code inside of Task should never get called if the user is logged
out because GaeFilter catches it.
We also now redirect correctly when the user hits the default page
(without specifying a file). The app loads, but redir
Submitted as of r10136
http://gwt-code-reviews.appspot.com/1428811/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Revision: 10136
Author: rchan...@google.com
Date: Wed May 4 09:12:17 2011
Log: Adds HTML5 App Cache support to MobileWebApp sample.
Uses a custom linker to figure out which files were generated by GWTC.
Review at http://gwt-code-reviews.appspot.com/1428811
http://code.google.com/p/go
Ready for review.
http://gwt-code-reviews.appspot.com/1433801/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1433801/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
It shouldn't ever happen because GaeFilter blocks users if they aren't
logged in. I'll modify getCurrentUser/getCurrentUserId to throw an
exception if the user isn't logged in.
http://gwt-code-reviews.appspot.com/1432801/diff/1/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/se
Sorry for the noise, not yet ready for review. Soon.
http://gwt-code-reviews.appspot.com/1433801/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: jlabanca,
Description:
Step one in making mobilewebapp more DI friendly. The goal is to
remove all knowledge of ClientFactory from any class but the entry
point.
This gets the basic pattern in place, submitting it as a checkpoint so
that new features can appear in the right place.
In
I guess I'm speaking strictly of the null checks. It's fine for our sample
code not to have a real auth system in its storage, of course.
Seems like your UserServiceWrapper should have a requireCurrentUserId()
method that throws an exception if there is no id, and Task shouldn't be
friendly. Is th
http://gwt-code-reviews.appspot.com/1432801/diff/1/samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/Task.java
File
samples/mobilewebapp/src/main/com/google/gwt/sample/mobilewebapp/server/domain/Task.java
(right):
http://gwt-code-reviews.appspot.com/1432801/diff/1/sa
Yeah, this is several white board discussions at least.
One last point, though: I think it's perfectly reasonable for find() (or any
other read request given domain specific knowledge) to return the latest
known proxy rather than force a trip to the server, especially for mobile
apps. That's basic
Reviewers: rjrjr,
Description:
Adds user authentication to MobileWebApp. Tasks are now specific to
each user. Authentication is done using a Filter, which was copied
almost verbatim from the expenses sample. If a user is not logged in,
they are forwarded to the login page, then redirected back
LGTM
http://gwt-code-reviews.appspot.com/1428811/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Exactly, that's my hope too :-)
Hopefully the numbers will be enough to convince us the work is worth doing.
On Wed, May 4, 2011 at 2:57 PM, Ray Ryan wrote:
> Once we've validated the work, seems like a lot of the Attachable support
> should be baked into UiObject, Widget and Panel in some kind
Once we've validated the work, seems like a lot of the Attachable support
should be baked into UiObject, Widget and Panel in some kind of opt-in
manner.
On Wed, May 4, 2011 at 10:20 AM, Rafael Castro wrote:
> Liked it. With the stuff I added to our subclass of AttachableHTMLPanel,
> this already
http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java
File dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java
(right):
http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAp
http://gwt-code-reviews.appspot.com/1428811/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Done. I didn't know how verbose I should be on the error message. Please
advise :-)
On Wed, May 4, 2011 at 2:04 PM, Ray Ryan wrote:
> I'd be inclined to start with a) and see what happens.
>
>
> On Wed, May 4, 2011 at 10:02 AM, wrote:
>
>>
>>
>> http://gwt-code-reviews.appspot.com/1427812/diff/
Revision: 10135
Author: jbrosenb...@google.com
Date: Wed May 4 07:26:14 2011
Log: Allows enum ordinalization to proceed for enums with static
methods/fields
-- This offers modest code-size gains, and will open way for further
optimizations
-- Also, fixes a bug in checking for ins
http://gwt-code-reviews.appspot.com/1427812/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Liked it. With the stuff I added to our subclass of AttachableHTMLPanel,
this already works pretty well. I have to review some other tricky cases
(like if you add a non-attachable widget to an Attachable panel before
finishing the initialization), but we're pretty close. The other cases that
could
http://gwt-code-reviews.appspot.com/1427812/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
How's that?
Is the bit I wrote about "after adding it to a panel" accurate? Seems
like we're trying to get to a world where the add would be fine, and the
wrap call wouldn't happen until the parent is wrapped — are we there
already?
http://gwt-code-reviews.appspot.com/1427812/diff/6003/user/sr
http://gwt-code-reviews.appspot.com/1428808/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Something as simple as short circuiting all equivalent
RequestFactory.find(EntityProxyId) requests, where equivalent takes into
account the proxy version as well as its id, could be very effective.
On Wed, May 4, 2011 at 10:04 AM, John LaBanca wrote:
> On Wed, May 4, 2011 at 12:58 PM, Ray Ryan
I'd be inclined to start with a) and see what happens.
On Wed, May 4, 2011 at 10:02 AM, wrote:
>
>
> http://gwt-code-reviews.appspot.com/1427812/diff/1/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
> File user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
> (right)
On Wed, May 4, 2011 at 12:58 PM, Ray Ryan wrote:
> On Wed, May 4, 2011 at 9:47 AM, Bob Vawter wrote:
>
>> > Bob, I've lost track -- are there hooks in place yet that we could
>> implement
>> > app specific client side caching in a sample like this?
>>
>> You can call RequestFactory.getSerializer(
http://gwt-code-reviews.appspot.com/1427812/diff/1/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
File user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
(right):
http://gwt-code-reviews.appspot.com/1427812/diff/1/user/src/com/google/gwt/user/client/ui/AttachableHTM
http://gwt-code-reviews.appspot.com/1427812/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On Wed, May 4, 2011 at 9:47 AM, Bob Vawter wrote:
> > Bob, I've lost track -- are there hooks in place yet that we could
> implement
> > app specific client side caching in a sample like this?
>
> You can call RequestFactory.getSerializer() with an implementation of
> a ProxyStore (e.g. DefaultPr
Sorry Rafa, forgot to hit send on this yesterday.
http://gwt-code-reviews.appspot.com/1427812/diff/1/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
File user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
(right):
http://gwt-code-reviews.appspot.com/1427812/diff/1/u
Also, this change is a stark reminder that we need to get real about request
caching in RequestFactory. You fundamentally shouldn't be doing this, you
should be able to make the "redundant" request counting on a lower layer to
send it only if necessary.
Bob, I've lost track -- are there hooks in p
I mean that you should have been able to fix it by replace place1 == place2
with place1.equals(place2).
On Wed, May 4, 2011 at 8:39 AM, wrote:
> @rjrjr -
>
> What do you mean?
>
>
> http://gwt-code-reviews.appspot.com/1428810/
>
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java
File dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java
(right):
http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAp
LGTM, but it'd be good if you could add some comments to record some of
the ideas we discussed.
http://gwt-code-reviews.appspot.com/1428808/diff/1/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java
File dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java
(right):
Please tell me that this is the last step in the following chain. It seems
unlikely if you're only providing the property now.
First you provide the property to allow quirks, and give compiler warnings
that only go away if you set standards mode or set the quirks property. (And
publicize that it w
@rjrjr -
What do you mean?
http://gwt-code-reviews.appspot.com/1428810/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
committed as r10134
http://gwt-code-reviews.appspot.com/1428810/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Your place instances should implement equals correctly.
On May 4, 2011 7:24 AM, wrote:
> LGTM
>
> http://gwt-code-reviews.appspot.com/1428810/
>
> --
> http://groups.google.com/group/Google-Web-Toolkit-Contributors
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Revision: 10134
Author: jlaba...@google.com
Date: Wed May 4 05:01:35 2011
Log: Including the TaskProxy (when available) in TaskEditPlace so we
do not do an extra round trip to the server to lookup the task.
Also fixing a bug where the "Task List" menu item isn't selected when the
You'll need to add this to the mobile web app .gwt.xml:
http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java
File dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java
(right):
http://gwt-code-reviews.appspot.com/14288
I have a few questions about appcache and linkers, but it looks pretty
good.
http://gwt-code-reviews.appspot.com/1428811/diff/1/dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java
File dev/core/src/com/google/gwt/core/linker/SimpleAppCacheLinker.java
(right):
http://gwt-code-revie
This is a breaking change for existing apps that run in quirks mode.
I'm alright with that because I don't see an alternative, but we'll need
to call if out in the release notes and tell people the workaround.
http://gwt-code-reviews.appspot.com/1422816/diff/1/user/src/com/google/gwt/user/UserAg
LGTM
http://gwt-code-reviews.appspot.com/1428810/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
This is probably html unit honoring the locale and changing its behavior
from what the tests expect.
On May 4, 2011 2:02 AM, wrote:
> On 2011/05/03 17:51:18, rjrjr wrote:
>> Running ant clean dist-dev test, this appears to break the i18n suite
> under
>> html unit.
>
> Oops! Only tested with a -Dg
On 2011/05/03 17:51:18, rjrjr wrote:
Running ant clean dist-dev test, this appears to break the i18n suite
under
html unit.
Oops! Only tested with a -Dgwt.junit.testcase.includes (which obviously
didn't include I18N tests)
That seems weird though that simply running the JVM in a different
lo
http://gwt-code-reviews.appspot.com/1377804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
62 matches
Mail list logo
