Reviewers: devint_google.com,
Description:
Fixes a recently introduced bug in Cell Widgets where clicking on the
first row does not update the SelectionModel when bound to selection.
The DefaultKeyboardSelectionHandler has an overly optimistic
optimization that ignores the click event if it occurs on the current
keyboard selected row. However, the keyboard selected row is
initialized to 0, and updating the keyboard selected row triggers
selection, so the optimization blocks selection of the 0th row.
Removing the optimization check fixes the bug.
Please review this at http://gwt-code-reviews.appspot.com/1509806/
Affected files:
M user/src/com/google/gwt/user/cellview/client/AbstractHasData.java
M user/test/com/google/gwt/user/cellview/client/CellListTest.java
Index: user/src/com/google/gwt/user/cellview/client/AbstractHasData.java
===================================================================
--- user/src/com/google/gwt/user/cellview/client/AbstractHasData.java
(revision 10504)
+++ user/src/com/google/gwt/user/cellview/client/AbstractHasData.java
(working copy)
@@ -138,16 +138,15 @@
* row, just updating it based on where the user clicked.
*/
int relRow = event.getIndex() - display.getPageStart();
- if (display.getKeyboardSelectedRow() != relRow) {
- // If a natively focusable element was just clicked, then do not
steal
- // focus.
- boolean isFocusable = false;
- Element target =
Element.as(event.getNativeEvent().getEventTarget());
- isFocusable = CellBasedWidgetImpl.get().isFocusable(target);
- display.setKeyboardSelectedRow(relRow, !isFocusable);
-
- // Do not cancel the event as the click may have occurred on a
Cell.
- }
+
+ // If a natively focusable element was just clicked, then do not
steal
+ // focus.
+ boolean isFocusable = false;
+ Element target =
Element.as(event.getNativeEvent().getEventTarget());
+ isFocusable = CellBasedWidgetImpl.get().isFocusable(target);
+ display.setKeyboardSelectedRow(relRow, !isFocusable);
+
+ // Do not cancel the event as the click may have occurred on a
Cell.
} else if ("focus".equals(eventType)) {
// Move keyboard focus to match the currently focused element.
int relRow = event.getIndex() - display.getPageStart();
Index: user/test/com/google/gwt/user/cellview/client/CellListTest.java
===================================================================
--- user/test/com/google/gwt/user/cellview/client/CellListTest.java
(revision 10504)
+++ user/test/com/google/gwt/user/cellview/client/CellListTest.java
(working copy)
@@ -22,9 +22,11 @@
import com.google.gwt.safehtml.shared.SafeHtml;
import com.google.gwt.safehtml.shared.SafeHtmlBuilder;
import com.google.gwt.safehtml.shared.SafeHtmlUtils;
+import
com.google.gwt.user.cellview.client.HasKeyboardSelectionPolicy.KeyboardSelectionPolicy;
import com.google.gwt.user.client.ui.Label;
import com.google.gwt.user.client.ui.RootPanel;
import com.google.gwt.view.client.ProvidesKey;
+import com.google.gwt.view.client.SingleSelectionModel;
import java.util.ArrayList;
import java.util.List;
@@ -81,6 +83,7 @@
// Create a model with only one level, and three values at that level.
ProvidesKey<String> keyProvider = new ProvidesKey<String>() {
+ @Override
public Object getKey(String item) {
return Integer.parseInt(item.substring(5));
}
@@ -89,6 +92,37 @@
cellList.setRowData(createData(0, 10));
cellList.getPresenter().flush();
assertEquals(10, rendered.size());
+ }
+
+ /**
+ * Test that clicking on the first item selects the item.
+ */
+ public void testSelectFirstItem() {
+ IndexCell<String> cell = new IndexCell<String>();
+ AbstractHasData<String> display = createAbstractHasData(cell);
+ populateData(display);
+
+ // Bind to selection.
+ SingleSelectionModel<String> selectionModel = new
SingleSelectionModel<String>();
+ display.setSelectionModel(selectionModel);
+
display.setKeyboardSelectionPolicy(KeyboardSelectionPolicy.BOUND_TO_SELECTION);
+ display.getPresenter().flush();
+ assertNull(selectionModel.getSelectedObject());
+ assertEquals(0, display.getKeyboardSelectedRow());
+
+ // Fire a click event on the first item.
+ RootPanel.get().add(display);
+ NativeEvent clickEvent =
+ Document.get().createClickEvent(0, 0, 0, 0, 0, false, false,
false, false);
+ display.getChildElement(0).dispatchEvent(clickEvent);
+ display.getPresenter().flush();
+
+ // Verify that the first item is now selected.
+ assertEquals("test 0", selectionModel.getSelectedObject());
+ assertEquals(0, display.getKeyboardSelectedRow());
+
+ // Cleanup.
+ RootPanel.get().remove(display);
}
@SuppressWarnings("deprecation")
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
