http://gwt-code-reviews.appspot.com/767801/diff/1/5
File
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/DynaTableWidget.java
(right):

http://gwt-code-reviews.appspot.com/767801/diff/1/5#newcode125
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/DynaTableWidget.java:125:
// List.indexOf() doesn't work; what are equality semantics?
At the moment it's instance equality.

I've been wondering if we should make it class+id equality, but it needs
thought. E.g., should it be the case that a
foo.equals(request.edit(foo))? Is it okay to ignore field values? I
think so. However, doing that would mean that we can never drop our java
wrappers for the proxies, b/c I believe we cannot give a JSO equals or
has implementations (right?).

We've been able not to think about it too hard because the cell widgets
all support a notion of KeyProvider, a helper that let's you decide
what's equal and isn't w/o relying directly on value objects'
hash/equals impl. Although I have been maintaining my own id -> row
maps.

http://gwt-code-reviews.appspot.com/767801/diff/1/6
File
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/PersonEditor.java
(right):

http://gwt-code-reviews.appspot.com/767801/diff/1/6#newcode58
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/PersonEditor.java:58:
final DynaTableRequestFactory requestFactory) {
I've been making methods like this return a/the HandlerRegistration.
Also, any reason not to make it an instance method?

http://gwt-code-reviews.appspot.com/767801/diff/1/6#newcode72
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/PersonEditor.java:72:
static class PersonEditorDelegate implements EditorDelegate<PersonProxy>
{
This would be the generated code?

http://gwt-code-reviews.appspot.com/767801/diff/1/6#newcode107
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/PersonEditor.java:107:
GWT.log("OK!");
Let's either use java.util.logging everywhere or GWT.log everywhere

http://gwt-code-reviews.appspot.com/767801/diff/1/6#newcode151
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/PersonEditor.java:151:
// GENERATE using ui:field mappings?
The trick will be to do that without making this thing *require*
UiBinder. Or is that more flexibility than we need?

http://gwt-code-reviews.appspot.com/767801/diff/1/8
File
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/SchoolCalendarWidget.java
(right):

http://gwt-code-reviews.appspot.com/767801/diff/1/8#newcode37
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/SchoolCalendarWidget.java:37:
dynaTable = new DynaTableWidget(eventBus, provider, visibleRows);
I made this change on purpose. It's anti-DI for the SchoolCalendarWidget
to be making its DynaTableWidget rather than being handed it. Did you
really need to reverse it?

http://gwt-code-reviews.appspot.com/767801/diff/1/9
File
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/events/EditPersonEvent.java
(right):

http://gwt-code-reviews.appspot.com/767801/diff/1/9#newcode25
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/events/EditPersonEvent.java:25:
* TODO: Make this an Activity.
The event wouldn't be an activity. The code that reacts to this event
would be.

Don't rush on that count. I'm going to blow up that world pretty hard
next week.

http://gwt-code-reviews.appspot.com/767801/diff/1/10
File
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/domain/Person.java
(right):

http://gwt-code-reviews.appspot.com/767801/diff/1/10#newcode96
samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/domain/Person.java:96:
* TODO At the moment requestFactory requires a persist method per type
It
Redundant with the javadoc above

http://gwt-code-reviews.appspot.com/767801/show

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

Reply via email to