Revision: 9278
Author: jlaba...@google.com
Date: Tue Nov 23 02:17:31 2010
Log: Fixing a few bugs in Cell widgets.
Issue 5625: Fixes a bug in CellBrowser where the first item in each list is
not clickable because we are too optimistic about not reselecting a row
that is already selected. Now we will reselect a row if the row index
changed OR if there is no row value selected, which ensures that the 0th
row can be selected.
Issue 5632: Fixes a bug where you have to click an EditTextCell twice to
switch it to edit mode. The bug is actually that we are stealing focus away
from the cell even though it is in edit mode.
Fixes an NPE in EditTextCell when ValueUpdater is null.
Fixes a bug in CellList where the key is not passed to Cell#render().
Review at http://gwt-code-reviews.appspot.com/1141801
Review by: rchan...@google.com
http://code.google.com/p/google-web-toolkit/source/detail?r=9278
Modified:
/trunk/user/src/com/google/gwt/cell/client/EditTextCell.java
/trunk/user/src/com/google/gwt/user/cellview/client/CellBrowser.java
/trunk/user/src/com/google/gwt/user/cellview/client/CellList.java
/trunk/user/src/com/google/gwt/user/cellview/client/CellTable.java
/trunk/user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java
/trunk/user/src/com/google/gwt/user/cellview/client/HasDataPresenter.java
/trunk/user/test/com/google/gwt/user/cellview/client/CellListTest.java
/trunk/user/test/com/google/gwt/user/cellview/client/HasDataPresenterTest.java
=======================================
--- /trunk/user/src/com/google/gwt/cell/client/EditTextCell.java Thu Oct 14
16:36:33 2010
+++ /trunk/user/src/com/google/gwt/cell/client/EditTextCell.java Tue Nov 23
02:17:31 2010
@@ -277,7 +277,9 @@
String value = updateViewData(parent, viewData, false);
clearInput(getInputElement(parent));
setValue(parent, value, viewData);
- valueUpdater.update(value);
+ if (valueUpdater != null) {
+ valueUpdater.update(value);
+ }
}
private void editEvent(Element parent, Object key, ViewData viewData,
=======================================
--- /trunk/user/src/com/google/gwt/user/cellview/client/CellBrowser.java
Mon Nov 22 12:05:46 2010
+++ /trunk/user/src/com/google/gwt/user/cellview/client/CellBrowser.java
Tue Nov 23 02:17:31 2010
@@ -393,7 +393,7 @@
if (level < treeNodes.size() - 1) {
TreeNodeImpl<?> treeNode = treeNodes.get(level + 1);
treeNode.display.getPresenter().setKeyboardSelectedRow(
- treeNode.display.getKeyboardSelectedRow(), true);
+ treeNode.display.getKeyboardSelectedRow(), true, true);
}
}
@@ -524,7 +524,7 @@
checkChildBounds(index);
if (open) {
// Open the child node.
- display.getPresenter().setKeyboardSelectedRow(index, false);
+ display.getPresenter().setKeyboardSelectedRow(index, false, true);
return updateChildState(display, fireEvents);
} else {
// Close the child node if it is currently open.
=======================================
--- /trunk/user/src/com/google/gwt/user/cellview/client/CellList.java Mon
Nov 22 12:05:46 2010
+++ /trunk/user/src/com/google/gwt/user/cellview/client/CellList.java Tue
Nov 23 02:17:31 2010
@@ -400,15 +400,14 @@
}
// Focus on the cell.
- if (isClick
- && getPresenter().getKeyboardSelectedRowInView() != indexOnPage)
{
+ if (isClick) {
/*
* If the selected element is natively focusable, then we do not
want to
* steal focus away from it.
*/
boolean isFocusable =
CellBasedWidgetImpl.get().isFocusable(target);
isFocused = isFocused || isFocusable;
- getPresenter().setKeyboardSelectedRow(indexOnPage, !isFocusable);
+ getPresenter().setKeyboardSelectedRow(indexOnPage, !isFocusable,
false);
}
// Fire the event to the cell if the list has not been refreshed.
@@ -449,7 +448,7 @@
}
SafeHtmlBuilder cellBuilder = new SafeHtmlBuilder();
- cell.render(value, null, cellBuilder);
+ cell.render(value, getValueKey(value), cellBuilder);
if (i == keyboardSelectedRow) {
// This is the focused item.
@@ -496,7 +495,7 @@
setStyleName(elem, style.cellListKeyboardSelectedItem(), selected);
}
setFocusable(elem, selected);
- if (selected && stealFocus) {
+ if (selected && stealFocus && !cellIsEditing) {
elem.focus();
onFocus();
}
=======================================
--- /trunk/user/src/com/google/gwt/user/cellview/client/CellTable.java Mon
Nov 22 12:05:46 2010
+++ /trunk/user/src/com/google/gwt/user/cellview/client/CellTable.java Tue
Nov 23 02:17:31 2010
@@ -971,7 +971,7 @@
boolean isFocusable =
CellBasedWidgetImpl.get().isFocusable(target);
isFocused = isFocused || isFocusable;
keyboardSelectedColumn = col;
- getPresenter().setKeyboardSelectedRow(row, !isFocusable);
+ getPresenter().setKeyboardSelectedRow(row, !isFocusable, true);
}
// Update selection. Selection occurs before firing the event to the
cell
@@ -1164,7 +1164,7 @@
}
// Move focus to the cell.
- if (selected && stealFocus) {
+ if (selected && stealFocus && !cellIsEditing) {
TableCellElement td = tr.getCells().getItem(keyboardSelectedColumn);
final com.google.gwt.user.client.Element cellParent =
getCellParent(td).cast();
CellBasedWidgetImpl.get().resetFocus(new
Scheduler.ScheduledCommand() {
@@ -1404,7 +1404,7 @@
} else {
// Reselect the row to move the selected column.
keyboardSelectedColumn = nextColumn;
- getPresenter().setKeyboardSelectedRow(oldRow, true);
+ getPresenter().setKeyboardSelectedRow(oldRow, true, true);
event.preventDefault();
return true;
}
@@ -1421,7 +1421,7 @@
} else {
// Reselect the row to move the selected column.
keyboardSelectedColumn = prevColumn;
- getPresenter().setKeyboardSelectedRow(oldRow, true);
+ getPresenter().setKeyboardSelectedRow(oldRow, true, true);
event.preventDefault();
return true;
}
=======================================
---
/trunk/user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java
Mon Nov 22 12:05:46 2010
+++
/trunk/user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java
Tue Nov 23 02:17:31 2010
@@ -1241,7 +1241,7 @@
if (accessKey != 0) {
focusImpl.setAccessKey(cellElem, accessKey);
}
- if (stealFocus) {
+ if (stealFocus && !tree.cellIsEditing) {
cellElem.focus();
}
}
=======================================
---
/trunk/user/src/com/google/gwt/user/cellview/client/HasDataPresenter.java
Mon Nov 22 12:05:46 2010
+++
/trunk/user/src/com/google/gwt/user/cellview/client/HasDataPresenter.java
Tue Nov 23 02:17:31 2010
@@ -642,7 +642,7 @@
*/
public void keyboardEnd() {
if (!keyboardPagingPolicy.isLimitedToRange()) {
- setKeyboardSelectedRow(getRowCount() - 1, true);
+ setKeyboardSelectedRow(getRowCount() - 1, true, false);
}
}
@@ -651,7 +651,7 @@
*/
public void keyboardHome() {
if (!keyboardPagingPolicy.isLimitedToRange()) {
- setKeyboardSelectedRow(-getPageStart(), true);
+ setKeyboardSelectedRow(-getPageStart(), true, false);
}
}
@@ -660,7 +660,7 @@
*/
public void keyboardNext() {
if (hasKeyboardNext()) {
- setKeyboardSelectedRow(getKeyboardSelectedRow() + 1, true);
+ setKeyboardSelectedRow(getKeyboardSelectedRow() + 1, true, false);
}
}
@@ -670,9 +670,10 @@
public void keyboardNextPage() {
if (KeyboardPagingPolicy.CHANGE_PAGE == keyboardPagingPolicy) {
// 0th index of next page.
- setKeyboardSelectedRow(getPageSize(), true);
+ setKeyboardSelectedRow(getPageSize(), true, false);
} else if (KeyboardPagingPolicy.INCREASE_RANGE ==
keyboardPagingPolicy) {
- setKeyboardSelectedRow(getKeyboardSelectedRow() + PAGE_INCREMENT,
true);
+ setKeyboardSelectedRow(getKeyboardSelectedRow() + PAGE_INCREMENT,
true,
+ false);
}
}
@@ -681,7 +682,7 @@
*/
public void keyboardPrev() {
if (hasKeyboardPrev()) {
- setKeyboardSelectedRow(getKeyboardSelectedRow() - 1, true);
+ setKeyboardSelectedRow(getKeyboardSelectedRow() - 1, true, false);
}
}
@@ -691,9 +692,10 @@
public void keyboardPrevPage() {
if (KeyboardPagingPolicy.CHANGE_PAGE == keyboardPagingPolicy) {
// 0th index of previous page.
- setKeyboardSelectedRow(-getPageSize(), true);
+ setKeyboardSelectedRow(-getPageSize(), true, false);
} else if (KeyboardPagingPolicy.INCREASE_RANGE ==
keyboardPagingPolicy) {
- setKeyboardSelectedRow(getKeyboardSelectedRow() - PAGE_INCREMENT,
true);
+ setKeyboardSelectedRow(getKeyboardSelectedRow() - PAGE_INCREMENT,
true,
+ false);
}
}
@@ -717,12 +719,23 @@
*
* @param index the row index
* @param stealFocus true to steal focus
+ * @param forceUpdate force the update even if the row didn't change
*/
- public void setKeyboardSelectedRow(int index, boolean stealFocus) {
+ public void setKeyboardSelectedRow(int index, boolean stealFocus,
+ boolean forceUpdate) {
// Early exit if disabled.
if (KeyboardSelectionPolicy.DISABLED == keyboardSelectionPolicy) {
return;
}
+
+ /*
+ * Early exit if the keyboard selected row has not changed and the
keyboard
+ * selected value is already set.
+ */
+ if (!forceUpdate && getKeyboardSelectedRow() == index
+ && getKeyboardSelectedRowValue() != null) {
+ return;
+ }
// Trim to within bounds.
int pageStart = getPageStart();
=======================================
--- /trunk/user/test/com/google/gwt/user/cellview/client/CellListTest.java
Tue Nov 9 07:53:09 2010
+++ /trunk/user/test/com/google/gwt/user/cellview/client/CellListTest.java
Tue Nov 23 02:17:31 2010
@@ -15,7 +15,13 @@
*/
package com.google.gwt.user.cellview.client;
+import com.google.gwt.cell.client.Cell;
import com.google.gwt.cell.client.TextCell;
+import com.google.gwt.safehtml.shared.SafeHtmlBuilder;
+import com.google.gwt.view.client.ProvidesKey;
+
+import java.util.ArrayList;
+import java.util.List;
/**
* Tests for {...@link CellList}.
@@ -29,6 +35,37 @@
// Ensure that calling getRowElement() flushes all pending changes.
assertNotNull(list.getRowElement(9));
}
+
+ /**
+ * Test that the correct values are sent to the Cell to be rendered.
+ */
+ public void testRenderWithKeyProvider() {
+ // Create a cell that verifies the render args.
+ final List<String> rendered = new ArrayList<String>();
+ final Cell<String> cell = new TextCell() {
+ @Override
+ public void render(String data, Object key, SafeHtmlBuilder sb) {
+ int call = rendered.size();
+ rendered.add(data);
+ assertTrue("render() called more than ten times", rendered.size()
< 11);
+
+ assertEquals("test " + call, data);
+ assertTrue(key instanceof Integer);
+ assertEquals(call, key);
+ }
+ };
+
+ // Create a model with only one level, and three values at that level.
+ ProvidesKey<String> keyProvider = new ProvidesKey<String>() {
+ public Object getKey(String item) {
+ return Integer.parseInt(item.substring(5));
+ }
+ };
+ CellList<String> cellList = new CellList<String>(cell, keyProvider);
+ cellList.setRowData(createData(0, 10));
+ cellList.getPresenter().flush();
+ assertEquals(10, rendered.size());
+ }
@Override
protected CellList<String> createAbstractHasData() {
=======================================
---
/trunk/user/test/com/google/gwt/user/cellview/client/HasDataPresenterTest.java
Mon Nov 22 12:05:46 2010
+++
/trunk/user/test/com/google/gwt/user/cellview/client/HasDataPresenterTest.java
Tue Nov 23 02:17:31 2010
@@ -321,7 +321,7 @@
populatePresenter(presenter);
// Select the second element.
- presenter.setKeyboardSelectedRow(2, false);
+ presenter.setKeyboardSelectedRow(2, false, false);
presenter.flush();
assertEquals(2, presenter.getKeyboardSelectedRow());
assertEquals("test 2", presenter.getKeyboardSelectedRowValue());
@@ -381,7 +381,7 @@
presenter.setKeyboardPagingPolicy(KeyboardPagingPolicy.CHANGE_PAGE);
// keyboardPrev in middle.
- presenter.setKeyboardSelectedRow(1, false);
+ presenter.setKeyboardSelectedRow(1, false, false);
presenter.flush();
view.assertKeyboardSelectedRow(0, false);
view.assertKeyboardSelectedRow(1, true);
@@ -403,7 +403,7 @@
assertEquals(new Range(40, 10), presenter.getVisibleRange());
// keyboardNext in middle.
- presenter.setKeyboardSelectedRow(8, false);
+ presenter.setKeyboardSelectedRow(8, false, false);
presenter.flush();
view.assertKeyboardSelectedRow(9, false);
view.assertKeyboardSelectedRow(8, true);
@@ -425,7 +425,7 @@
assertEquals(new Range(50, 10), presenter.getVisibleRange());
// keyboardPrevPage.
- presenter.setKeyboardSelectedRow(5, false);
+ presenter.setKeyboardSelectedRow(5, false, false);
presenter.flush();
view.assertKeyboardSelectedRow(0, false);
view.assertKeyboardSelectedRow(5, true);
@@ -438,7 +438,7 @@
assertEquals(new Range(40, 10), presenter.getVisibleRange());
// keyboardNextPage.
- presenter.setKeyboardSelectedRow(5, false);
+ presenter.setKeyboardSelectedRow(5, false, false);
presenter.flush();
view.assertKeyboardSelectedRow(0, false);
view.assertKeyboardSelectedRow(5, true);
@@ -494,7 +494,7 @@
presenter.setKeyboardPagingPolicy(KeyboardPagingPolicy.CURRENT_PAGE);
// keyboardPrev in middle.
- presenter.setKeyboardSelectedRow(1, false);
+ presenter.setKeyboardSelectedRow(1, false, false);
presenter.flush();
view.assertKeyboardSelectedRow(0, false);
view.assertKeyboardSelectedRow(1, true);
@@ -513,7 +513,7 @@
view.assertKeyboardSelectedRowEmpty();
// keyboardNext in middle.
- presenter.setKeyboardSelectedRow(8, false);
+ presenter.setKeyboardSelectedRow(8, false, false);
presenter.flush();
view.assertKeyboardSelectedRow(0, false);
view.assertKeyboardSelectedRow(8, true);
@@ -567,7 +567,7 @@
presenter.setKeyboardPagingPolicy(KeyboardPagingPolicy.INCREASE_RANGE);
// keyboardPrev in middle.
- presenter.setKeyboardSelectedRow(1, false);
+ presenter.setKeyboardSelectedRow(1, false, false);
presenter.flush();
view.assertKeyboardSelectedRow(0, false);
view.assertKeyboardSelectedRow(1, true);
@@ -591,7 +591,7 @@
assertEquals(new Range(pageStart, pageSize),
presenter.getVisibleRange());
// keyboardNext in middle.
- presenter.setKeyboardSelectedRow(pageSize - 2, false);
+ presenter.setKeyboardSelectedRow(pageSize - 2, false, false);
presenter.flush();
view.assertKeyboardSelectedRow(increment - 1, false);
view.assertKeyboardSelectedRow(pageSize - 2, true);
@@ -614,7 +614,7 @@
assertEquals(new Range(pageStart, pageSize),
presenter.getVisibleRange());
// keyboardPrevPage within range.
- presenter.setKeyboardSelectedRow(increment, false);
+ presenter.setKeyboardSelectedRow(increment, false, false);
presenter.flush();
view.assertKeyboardSelectedRow(pageSize - increment, false);
view.assertKeyboardSelectedRow(increment, true);
@@ -645,7 +645,7 @@
assertEquals(new Range(pageStart, pageSize),
presenter.getVisibleRange());
// keyboardNextPage outside range.
- presenter.setKeyboardSelectedRow(pageSize - 1, false);
+ presenter.setKeyboardSelectedRow(pageSize - 1, false, false);
presenter.flush();
view.assertKeyboardSelectedRow(increment, false);
view.assertKeyboardSelectedRow(pageSize - 1, true);
@@ -795,19 +795,19 @@
assertEquals(0, model.getSelectedSet().size());
// Select an element.
- presenter.setKeyboardSelectedRow(5, false);
+ presenter.setKeyboardSelectedRow(5, false, false);
presenter.flush();
assertEquals(1, model.getSelectedSet().size());
assertTrue(model.isSelected("test 5"));
// Select another element.
- presenter.setKeyboardSelectedRow(9, false);
+ presenter.setKeyboardSelectedRow(9, false, false);
presenter.flush();
assertEquals(1, model.getSelectedSet().size());
assertTrue(model.isSelected("test 9"));
// Select an element on another page.
- presenter.setKeyboardSelectedRow(11, false);
+ presenter.setKeyboardSelectedRow(11, false, false);
presenter.flush();
// Nothing is selected yet because we don't have data.
assertEquals(0, model.getSelectedSet().size());
@@ -836,7 +836,7 @@
view.assertKeyboardSelectedRowEmpty();
// Move to middle.
- presenter.setKeyboardSelectedRow(1, false);
+ presenter.setKeyboardSelectedRow(1, false, false);
assertEquals("test 11", presenter.getKeyboardSelectedRowValue());
presenter.flush();
assertEquals(1, presenter.getKeyboardSelectedRow());
@@ -845,7 +845,7 @@
view.assertKeyboardSelectedRow(1, true);
// Move to same row (should not early out).
- presenter.setKeyboardSelectedRow(1, false);
+ presenter.setKeyboardSelectedRow(1, false, true);
assertEquals("test 11", presenter.getKeyboardSelectedRowValue());
presenter.flush();
assertEquals(1, presenter.getKeyboardSelectedRow());
@@ -854,7 +854,7 @@
view.assertKeyboardSelectedRow(1, true);
// Move to last row.
- presenter.setKeyboardSelectedRow(9, false);
+ presenter.setKeyboardSelectedRow(9, false, false);
assertEquals("test 19", presenter.getKeyboardSelectedRowValue());
presenter.flush();
assertEquals(9, presenter.getKeyboardSelectedRow());
@@ -865,7 +865,7 @@
assertEquals(10, presenter.getVisibleRange().getLength());
// Move to next page.
- presenter.setKeyboardSelectedRow(10, false);
+ presenter.setKeyboardSelectedRow(10, false, false);
populatePresenter(presenter);
assertNull(presenter.getKeyboardSelectedRowValue());
presenter.flush();
@@ -877,7 +877,7 @@
assertEquals(10, presenter.getVisibleRange().getLength());
// Negative index.
- presenter.setKeyboardSelectedRow(-1, false);
+ presenter.setKeyboardSelectedRow(-1, false, false);
populatePresenter(presenter);
assertNull(presenter.getKeyboardSelectedRowValue());
presenter.flush();
@@ -904,35 +904,35 @@
view.assertKeyboardSelectedRowEmpty();
// Negative index (should remain at index 0).
- presenter.setKeyboardSelectedRow(-1, false);
+ presenter.setKeyboardSelectedRow(-1, false, false);
presenter.flush();
assertEquals(0, presenter.getKeyboardSelectedRow());
view.assertKeyboardSelectedRow(0, false);
view.assertKeyboardSelectedRow(0, true);
// Move to middle.
- presenter.setKeyboardSelectedRow(1, false);
+ presenter.setKeyboardSelectedRow(1, false, false);
presenter.flush();
assertEquals(1, presenter.getKeyboardSelectedRow());
view.assertKeyboardSelectedRow(0, false);
view.assertKeyboardSelectedRow(1, true);
// Move to same row (should not early out).
- presenter.setKeyboardSelectedRow(1, false);
+ presenter.setKeyboardSelectedRow(1, false, true);
presenter.flush();
assertEquals(1, presenter.getKeyboardSelectedRow());
view.assertKeyboardSelectedRow(1, false);
view.assertKeyboardSelectedRow(1, true);
// Move to last row.
- presenter.setKeyboardSelectedRow(9, false);
+ presenter.setKeyboardSelectedRow(9, false, false);
presenter.flush();
assertEquals(9, presenter.getKeyboardSelectedRow());
view.assertKeyboardSelectedRow(1, false);
view.assertKeyboardSelectedRow(9, true);
// Move to next page (confined to page).
- presenter.setKeyboardSelectedRow(10, false);
+ presenter.setKeyboardSelectedRow(10, false, false);
presenter.flush();
assertEquals(9, presenter.getKeyboardSelectedRow());
view.assertKeyboardSelectedRow(9, false);
@@ -956,7 +956,7 @@
assertNull(presenter.getKeyboardSelectedRowValue());
view.assertKeyboardSelectedRowEmpty();
- presenter.setKeyboardSelectedRow(1, false);
+ presenter.setKeyboardSelectedRow(1, false, false);
presenter.flush();
assertEquals(-1, presenter.getKeyboardSelectedRow());
assertNull(presenter.getKeyboardSelectedRowValue());
@@ -979,21 +979,21 @@
view.assertKeyboardSelectedRowEmpty();
// Move to middle.
- presenter.setKeyboardSelectedRow(1, false);
+ presenter.setKeyboardSelectedRow(1, false, false);
presenter.flush();
assertEquals(1, presenter.getKeyboardSelectedRow());
view.assertKeyboardSelectedRow(0, false);
view.assertKeyboardSelectedRow(1, true);
// Move to same row (should not early out).
- presenter.setKeyboardSelectedRow(1, false);
+ presenter.setKeyboardSelectedRow(1, false, true);
presenter.flush();
assertEquals(1, presenter.getKeyboardSelectedRow());
view.assertKeyboardSelectedRow(1, false);
view.assertKeyboardSelectedRow(1, true);
// Move to last row.
- presenter.setKeyboardSelectedRow(9, false);
+ presenter.setKeyboardSelectedRow(9, false, false);
presenter.flush();
assertEquals(9, presenter.getKeyboardSelectedRow());
view.assertKeyboardSelectedRow(1, false);
@@ -1002,7 +1002,7 @@
assertEquals(pageSize, presenter.getVisibleRange().getLength());
// Move to next page.
- presenter.setKeyboardSelectedRow(10, false);
+ presenter.setKeyboardSelectedRow(10, false, false);
populatePresenter(presenter);
presenter.flush();
assertEquals(10, presenter.getKeyboardSelectedRow());
@@ -1013,7 +1013,7 @@
assertEquals(pageSize, presenter.getVisibleRange().getLength());
// Negative index near index 0.
- presenter.setKeyboardSelectedRow(-1, false);
+ presenter.setKeyboardSelectedRow(-1, false, false);
populatePresenter(presenter);
presenter.flush();
assertEquals(9, presenter.getKeyboardSelectedRow());
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
