commit 0130db84ce189d7a935808db330d4ef95e172584
author jlabanca@google.com <jlabanca@google.com@8db76d5a-ed1c-0410-87a9-c151d255dfc7> Mon Dec 13 19:55:24 2010 +0000
committer jlabanca@google.com <jlabanca@google.com@8db76d5a-ed1c-0410-87a9-c151d255dfc7> Mon Dec 13 19:55:24 2010 +0000
tree e5b8915fb88f842daacf1dd4c75c71f1b24b6a82
parent 683be14d91bd947f3959d49dc071ca455cdcb871

CellWidget should only fire a ValueChangeEvent when the new value is not equal to the previous one. The redraw logic should do the same.  If you really want to redraw the widget unconditionally, call setValue(a, b, false) and then redraw()).

Code Review at http://gwt-code-reviews.appspot.com/1155801/show

Author: tbroyer

Review by: sbrubaker@google.com

git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@9413 8db76d5a-ed1c-0410-87a9-c151d255dfc7

user/src/com/google/gwt/user/cellview/client/CellWidget.java

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 de9ae76..f491a36 100644
--- a/user/src/com/google/gwt/user/cellview/client/CellWidget.java
+++ b/user/src/com/google/gwt/user/cellview/client/CellWidget.java
@@ -183,7 +183,8 @@
   /**
    * {@inheritDoc}
    * <p>
-   * This method will redraw the widget using the new value.
+   * This method will redraw the widget if the new value does not equal the
+   * existing value.
    * </p>
    */
   public void setValue(C value) {
@@ -193,7 +194,8 @@
   /**
    * {@inheritDoc}
    * <p>
-   * This method will redraw the widget using the new value.
+   * This method will redraw the widget if the new value does not equal the
+   * existing value.
    * </p>
    */
   public void setValue(C value, boolean fireEvents) {
@@ -204,18 +206,23 @@
    * Sets this object's value and optionally redraw the widget. Fires
    * {@link com.google.gwt.event.logical.shared.ValueChangeEvent} when
    * fireEvents is true and the new value does not equal the existing value.
+   * Redraws the widget when redraw is true and the new value does not equal the
+   * existing value.
    * 
    * @param value the object's new value
    * @param fireEvents fire events if true and value is new
-   * @param redraw true to redraw the widget, false not to
+   * @param redraw redraw the widget if true and value is new
    */
   public void setValue(C value, boolean fireEvents, boolean redraw) {
-    this.value = value;
-    if (redraw) {
-      redraw();
-    }
-    if (fireEvents) {
-      ValueChangeEvent.fire(this, value);
+    C oldValue = getValue();
+    if (value != oldValue && (oldValue == null || !oldValue.equals(value))) {
+      this.value = value;
+      if (redraw) {
+        redraw();
+      }
+      if (fireEvents) {
+        ValueChangeEvent.fire(this, value);
+      }
     }
   }
 

user/test/com/google/gwt/user/cellview/client/CellWidgetTest.java

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 fff2c2b..c48b1f3 100644
--- a/user/test/com/google/gwt/user/cellview/client/CellWidgetTest.java
+++ b/user/test/com/google/gwt/user/cellview/client/CellWidgetTest.java
@@ -181,6 +181,10 @@
     assertEquals("test0", cw.getElement().getInnerText());
     handler.assertOnValueChangeNotCalled();
 
+    // Set value to the existing value, shouldn't fire events.
+    cw.setValue("test0", true);
+    handler.assertOnValueChangeNotCalled();
+
     // Set value and fire events.
     cw.setValue("test1", true);
     assertEquals("test1", cw.getValue());