http://gwt-code-reviews.appspot.com/710802/diff/1/3
File user/src/com/google/gwt/cell/client/Cell.java (right):

http://gwt-code-reviews.appspot.com/710802/diff/1/3#newcode71
user/src/com/google/gwt/cell/client/Cell.java:71: boolean
isEditing(Element element, Object key);
Good idea.

On 2010/07/28 21:50:38, jlabanca wrote:
I think we should also pass the value for consistency.

http://gwt-code-reviews.appspot.com/710802/diff/1/5
File user/src/com/google/gwt/cell/client/EditTextCell.java (right):

http://gwt-code-reviews.appspot.com/710802/diff/1/5#newcode221
user/src/com/google/gwt/cell/client/EditTextCell.java:221: if
("keypress".equals(type)) {
It seems to work fine.

On 2010/07/28 21:50:38, jlabanca wrote:
Please double check that the new value is set in the input box on
keypress.  I'm
pretty sure you have to wait for keyup before the value actually
changes.

http://gwt-code-reviews.appspot.com/710802/diff/1/9
File user/src/com/google/gwt/user/cellview/client/CellTable.java
(right):

http://gwt-code-reviews.appspot.com/710802/diff/1/9#newcode527
user/src/com/google/gwt/user/cellview/client/CellTable.java:527:
focusable.focus();
I deleted this whole block.  Now cell (0, 0) is made focusable on table
initialization.

On 2010/07/28 21:50:38, jlabanca wrote:
Don't focus in the constructor.

http://gwt-code-reviews.appspot.com/710802/diff/1/9#newcode547
user/src/com/google/gwt/user/cellview/client/CellTable.java:547:
Event.KEYEVENTS);
O.K.

On 2010/07/28 21:50:38, jlabanca wrote:
Sinking events is surprisingly costly.  It looks like you only need to
sink
ONKEYPRESS instead of all KEYEVENTS.

http://gwt-code-reviews.appspot.com/710802/diff/1/9#newcode682
user/src/com/google/gwt/user/cellview/client/CellTable.java:682: if
("keypress".equals(eventType) && !cellIsEditing) {
Done.

On 2010/07/28 21:50:38, jlabanca wrote:
Can you move this above the eventTarget stuff?

http://gwt-code-reviews.appspot.com/710802/diff/1/9#newcode757
user/src/com/google/gwt/user/cellview/client/CellTable.java:757:
view.setFocus();
I'll focus if the cell was editing prior to the event and is no longer
editing following the event.

On 2010/07/28 21:50:38, jlabanca wrote:
Do you always want to focus here if the cell isn't editing? What about
an image
load event?

http://gwt-code-reviews.appspot.com/710802/diff/1/12
File
user/src/com/google/gwt/user/cellview/client/PagingListViewPresenter.java
(right):

http://gwt-code-reviews.appspot.com/710802/diff/1/12#newcode486
user/src/com/google/gwt/user/cellview/client/PagingListViewPresenter.java:486:
view.setFocus();
The intent of the method is to give the implementor a chance to grab
focus where appropriate.  It's up to the implementor to decide if they
actually want focus.  Basically the widget may have just been
re-rendered, so the new DOM has just come into existence and this is the
time to call focus() on some appropriate element if focus is desired.

On 2010/07/28 21:50:38, jlabanca wrote:
I don't think we should always steal focus and give it to the view.
We should
probably only update focus if the table already had focus.

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

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

Reply via email to