Fixing a bug in CellWidget where the cell isn't rendered if the initial value is null. We were checking that the value actually changed in setValue(), but this.value defaults to null. We no longer do an equality check when setting the value in the constructor.
Review at http://gwt-code-reviews.appspot.com/1655803
Review by: skybrian@google.com
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@10897 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/user/cellview/client/CellWidget.java b/user/src/com/google/gwt/user/cellview/client/CellWidget.java
index 00996df..0c5b44b 100644
--- a/user/src/com/google/gwt/user/cellview/client/CellWidget.java
+++ b/user/src/com/google/gwt/user/cellview/client/CellWidget.java
@@ -79,6 +79,7 @@
* The {@link ValueUpdater} used to trigger value update events.
*/
private final ValueUpdater<C> valueUpdater = new ValueUpdater<C>() {
+ @Override
public void update(C value) {
// no need to redraw, the Cell took care of it
setValue(value, true, false);
@@ -143,13 +144,22 @@
this.keyProvider = keyProvider;
setElement(elem);
CellBasedWidgetImpl.get().sinkEvents(this, cell.getConsumedEvents());
- setValue(initialValue);
+
+ /*
+ * Skip the equality check. If initialValue is null, it will be equal to
+ * this.value, but that doesn't mean that we don't want to render the
+ * initialValue.
+ */
+ this.value = initialValue;
+ redraw();
}
+ @Override
public HandlerRegistration addValueChangeHandler(ValueChangeHandler<C> handler) {
return addHandler(handler, ValueChangeEvent.getType());
}
+ @Override
public LeafValueEditor<C> asEditor() {
if (editor == null) {
editor = TakesValueEditor.of(this);
@@ -166,10 +176,12 @@
return cell;
}
+ @Override
public ProvidesKey<C> getKeyProvider() {
return keyProvider;
}
+ @Override
public C getValue() {
return value;
}
@@ -213,6 +225,7 @@
* existing value.
* </p>
*/
+ @Override
public void setValue(C value) {
setValue(value, false, true);
}
@@ -224,6 +237,7 @@
* existing value.
* </p>
*/
+ @Override
public void setValue(C value, boolean fireEvents) {
setValue(value, fireEvents, true);
}
diff --git a/user/test/com/google/gwt/user/cellview/client/CellWidgetTest.java b/user/test/com/google/gwt/user/cellview/client/CellWidgetTest.java
index 5b400f8..38b721d 100644
--- a/user/test/com/google/gwt/user/cellview/client/CellWidgetTest.java
+++ b/user/test/com/google/gwt/user/cellview/client/CellWidgetTest.java
@@ -42,6 +42,7 @@
private String lastEventValue;
private Object lastEventKey;
+ private String lastRenderedValue = "never_rendered";
public CustomCell() {
super("change");
@@ -57,6 +58,11 @@
lastEventValue = null;
}
+ public void assertLastRenderedValue(String expected) {
+ assertEquals(expected, lastRenderedValue);
+ lastRenderedValue = null;
+ }
+
@Override
public void onBrowserEvent(Context context, Element parent, String value, NativeEvent event,
ValueUpdater<String> valueUpdater) {
@@ -69,6 +75,7 @@
@Override
public void render(Context context, String value, SafeHtmlBuilder sb) {
+ lastRenderedValue = value;
if (value != null) {
sb.appendEscaped(value);
}
@@ -96,6 +103,7 @@
onValueChangeCalled = false;
}
+ @Override
public void onValueChange(ValueChangeEvent<C> event) {
assertFalse("ValueChangeEvent fired twice", onValueChangeCalled);
onValueChangeCalled = true;
@@ -108,6 +116,16 @@
return "com.google.gwt.user.cellview.CellView";
}
+ /**
+ * Tests that the cell widget will render correctly with an initial value of null.
+ */
+ public void testInitialValueNull() {
+ CustomCell cell = new CustomCell();
+ CellWidget<String> cw = new CellWidget<String>(cell);
+ assertNull(cw.getValue());
+ cell.assertLastRenderedValue(null);
+ }
+
public void testOnBrowserEvent() {
CustomCell cell = new CustomCell();
CellWidget<String> cw = new CellWidget<String>(cell, "test");
@@ -122,6 +140,7 @@
public void testOnBrowserEventWithKeyProvider() {
ProvidesKey<String> keyProvider = new ProvidesKey<String>() {
+ @Override
public Object getKey(String item) {
// Return the first character as the key.
return (item == null) ? null : item.substring(0, 1);
@@ -165,8 +184,10 @@
@Override
public void render(com.google.gwt.cell.client.Cell.Context context, String value,
SafeHtmlBuilder sb) {
- sb.appendHtmlConstant("<div>").appendEscaped(value).appendHtmlConstant("</div>");
- sb.appendHtmlConstant("<div>child2</div>");
+ if (value != null) {
+ sb.appendHtmlConstant("<div>").appendEscaped(value).appendHtmlConstant("</div>");
+ sb.appendHtmlConstant("<div>child2</div>");
+ }
}
};
CellWidget<String> cw = new CellWidget<String>(cell);
@@ -225,11 +246,13 @@
// Check the intial value.
assertEquals("initial", cw.getValue());
assertEquals("initial", cw.getElement().getInnerText());
+ cell.assertLastRenderedValue("initial");
// Set value without firing events.
cw.setValue("test0");
assertEquals("test0", cw.getValue());
assertEquals("test0", cw.getElement().getInnerText());
+ cell.assertLastRenderedValue("test0");
handler.assertOnValueChangeNotCalled();
// Set value to the existing value, shouldn't fire events.
@@ -240,6 +263,7 @@
cw.setValue("test1", true);
assertEquals("test1", cw.getValue());
assertEquals("test1", cw.getElement().getInnerText());
+ cell.assertLastRenderedValue("test1");
handler.assertLastValue("test1");
// Set value, fire events, but do not redraw.
