Revision: 10506
Author: [email protected]
Date: Mon Aug 8 08:03:35 2011
Log: 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.
Review at http://gwt-code-reviews.appspot.com/1509806
Review by: [email protected]
http://code.google.com/p/google-web-toolkit/source/detail?r=10506
Modified:
/trunk/user/src/com/google/gwt/user/cellview/client/AbstractHasData.java
/trunk/user/test/com/google/gwt/user/cellview/client/CellListTest.java
=======================================
---
/trunk/user/src/com/google/gwt/user/cellview/client/AbstractHasData.java
Thu Jul 28 04:03:54 2011
+++
/trunk/user/src/com/google/gwt/user/cellview/client/AbstractHasData.java
Mon Aug 8 08:03:35 2011
@@ -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();
=======================================
--- /trunk/user/test/com/google/gwt/user/cellview/client/CellListTest.java
Tue Feb 8 04:44:50 2011
+++ /trunk/user/test/com/google/gwt/user/cellview/client/CellListTest.java
Mon Aug 8 08:03:35 2011
@@ -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));
}
@@ -90,6 +93,37 @@
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")
public void testSetEmptyListWidget() {
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
