This patch provides access to the underlying InputElement's value property, and
deprecates the old isChecked / setChecked methods that are redundant with those
implemented for HasValue.
Also converts CheckBox to use the new Element methods instead of DOM and
string names all over the place.
Related thread:
http://groups.google.com/group/Google-Web-Toolkit-Contributors/browse_thread/thread/80d51c5ff3b32844/cfef01d8b513ebd0?#cfef01d8b513ebd0
Addresses issues:
http://code.google.com/p/google-web-toolkit/issues/detail?id=458
http://code.google.com/p/google-web-toolkit/issues/detail?id=3249
Reviewed by ecc,jgw
git-svn-id: https://google-web-toolkit.googlecode.com/svn/releases/1.6@4475 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/user/client/ui/CheckBox.java b/user/src/com/google/gwt/user/client/ui/CheckBox.java
index e2f96cf..f8d6013 100644
--- a/user/src/com/google/gwt/user/client/ui/CheckBox.java
+++ b/user/src/com/google/gwt/user/client/ui/CheckBox.java
@@ -15,6 +15,9 @@
*/
package com.google.gwt.user.client.ui;
+import com.google.gwt.dom.client.Document;
+import com.google.gwt.dom.client.InputElement;
+import com.google.gwt.dom.client.LabelElement;
import com.google.gwt.event.dom.client.ClickEvent;
import com.google.gwt.event.dom.client.ClickHandler;
import com.google.gwt.event.logical.shared.ValueChangeEvent;
@@ -22,6 +25,8 @@
import com.google.gwt.event.shared.HandlerRegistration;
import com.google.gwt.user.client.DOM;
import com.google.gwt.user.client.Element;
+import com.google.gwt.user.client.Event;
+import com.google.gwt.user.client.EventListener;
/**
* A standard check box widget.
@@ -47,7 +52,8 @@
* </p>
*/
public class CheckBox extends ButtonBase implements HasName, HasValue<Boolean> {
- private Element inputElem, labelElem;
+ private InputElement inputElem;
+ private LabelElement labelElem;
private boolean valueChangeHandlerInitialized;
/**
@@ -85,15 +91,15 @@
protected CheckBox(Element elem) {
super(DOM.createSpan());
- inputElem = elem;
- labelElem = DOM.createLabel();
+ inputElem = InputElement.as(elem);
+ labelElem = Document.get().createLabelElement();
- DOM.appendChild(getElement(), inputElem);
- DOM.appendChild(getElement(), labelElem);
+ getElement().appendChild(inputElem);
+ getElement().appendChild(labelElem);
String uid = DOM.createUniqueId();
- DOM.setElementProperty(inputElem, "id", uid);
- DOM.setElementProperty(labelElem, "htmlFor", uid);
+ inputElem.setPropertyString("id", uid);
+ labelElem.setHtmlFor(uid);
// Accessibility: setting tab index to be 0 by default, ensuring element
// appears in tab sequence. FocusWidget's setElement method already
@@ -113,59 +119,85 @@
public void onClick(ClickEvent event) {
// No need to compare old value and new value--click handler
// only fires on real click, and value always toggles
- ValueChangeEvent.fire(CheckBox.this, isChecked());
+ ValueChangeEvent.fire(CheckBox.this, getValue());
}
});
}
return addHandler(handler, ValueChangeEvent.getType());
}
+ /**
+ * Returns the value property of the input element that backs this widget.
+ * This is the value that will be associated with the CheckBox name and
+ * submitted to the server if a {@link FormPanel} that holds it is submitted
+ * and the box is checked.
+ * <p>
+ * Don't confuse this with {@link #getValue}, which returns true or false if
+ * the widget is checked.
+ *
+ * @return
+ */
+ public String getFormValue() {
+ return inputElem.getAttribute("value");
+ }
+
@Override
public String getHTML() {
- return DOM.getInnerHTML(labelElem);
+ return labelElem.getInnerHTML();
}
public String getName() {
- return DOM.getElementProperty(inputElem, "name");
+ return inputElem.getName();
}
@Override
public int getTabIndex() {
- return getFocusImpl().getTabIndex(inputElem);
+ return inputElem.getTabIndex();
}
@Override
public String getText() {
- return DOM.getInnerText(labelElem);
+ return labelElem.getInnerText();
}
/**
- * Determines whether this check box is currently checked.
+ * Determines whether this check box is currently checked.
+ * <p>
+ * Note that this <em>is not</em> return the value property of the checkbox
+ * input element wrapped by this widget. For access to that property, see
+ * {@link #getFormValue()}
*
- * @return <code>true</code> if the check box is checked
+ * @return <code>true</code> if the check box is checked, false otherwise.
+ * Will not return null
*/
public Boolean getValue() {
- return isChecked();
+ if (isAttached()) {
+ return inputElem.isChecked();
+ } else {
+ return inputElem.isDefaultChecked();
+ }
}
/**
* Determines whether this check box is currently checked.
*
* @return <code>true</code> if the check box is checked
+ * @deprecated use {@link #getValue} instead
*/
+ @Deprecated
public boolean isChecked() {
- String propName = isAttached() ? "checked" : "defaultChecked";
- return DOM.getElementPropertyBoolean(inputElem, propName);
+ // Funny comparison b/c getValue could in theory return null
+ return getValue() == true;
}
@Override
public boolean isEnabled() {
- return !DOM.getElementPropertyBoolean(inputElem, "disabled");
+ return !inputElem.isDisabled();
}
@Override
public void setAccessKey(char key) {
- DOM.setElementProperty(inputElem, "accessKey", "" + key);
+ inputElem.setAccessKey("" + key);
}
/**
@@ -173,15 +205,16 @@
* (If you want the event to fire, use {@link #setValue(Boolean, boolean)})
*
* @param checked <code>true</code> to check the check box.
+ * @deprecated Use {@link #setValue(Boolean)} instead
*/
+ @Deprecated
public void setChecked(boolean checked) {
- DOM.setElementPropertyBoolean(inputElem, "checked", checked);
- DOM.setElementPropertyBoolean(inputElem, "defaultChecked", checked);
+ setValue(checked);
}
@Override
public void setEnabled(boolean enabled) {
- DOM.setElementPropertyBoolean(inputElem, "disabled", !enabled);
+ inputElem.setDisabled(!enabled);
if (enabled) {
removeStyleDependentName("disabled");
} else {
@@ -192,19 +225,34 @@
@Override
public void setFocus(boolean focused) {
if (focused) {
- getFocusImpl().focus(inputElem);
+ inputElem.focus();
} else {
- getFocusImpl().blur(inputElem);
+ inputElem.blur();
}
}
+ /**
+ * Set the value property on the input element that backs this widget. This is
+ * the value that will be associated with the CheckBox's name and submitted to
+ * the server if a {@link FormPanel} that holds it is submitted and the box is
+ * checked.
+ * <p>
+ * Don't confuse this with {@link #setValue}, which actually checks and
+ * unchecks the box.
+ *
+ * @param value
+ */
+ public void setFormValue(String value) {
+ inputElem.setAttribute("value", value);
+ }
+
@Override
public void setHTML(String html) {
- DOM.setInnerHTML(labelElem, html);
+ labelElem.setInnerHTML(html);
}
public void setName(String name) {
- DOM.setElementProperty(inputElem, "name", name);
+ inputElem.setName(name);
}
@Override
@@ -214,17 +262,21 @@
// CheckBox) setElement method calls setTabIndex before inputElem is
// initialized. See CheckBox's protected constructor for more information.
if (inputElem != null) {
- getFocusImpl().setTabIndex(inputElem, index);
+ inputElem.setTabIndex(index);
}
}
@Override
public void setText(String text) {
- DOM.setInnerText(labelElem, text);
+ labelElem.setInnerText(text);
}
/**
* Checks or unchecks the text box.
+ * <p>
+ * Note that this <em>does not</em> set the value property of the checkbox
+ * input element wrapped by this widget. For access to that property, see
+ * {@link #setFormValue(String)}
*
* @param value true to check, false to uncheck. Must not be null.
* @thows IllegalArgumentException if value is null
@@ -236,7 +288,11 @@
/**
* Checks or unchecks the text box, firing {@link ValueChangeEvent} if
* appropriate.
- *
+ * <p>
+ * Note that this <em>does not</em> set the value property of the checkbox
+ * input element wrapped by this widget. For access to that property, see
+ * {@link #setFormValue(String)}
+ *
* @param value true to check, false to uncheck. Must not be null.
* @param fireEvents If true, and value has changed, fire a
* {@link ValueChangeEvent}
@@ -247,10 +303,11 @@
throw new IllegalArgumentException("value must not be null");
}
- if (isChecked() == value) {
+ if (value.equals(getValue())) {
return;
}
- setChecked(value);
+ inputElem.setChecked(value);
+ inputElem.setDefaultChecked(value);
if (fireEvents) {
ValueChangeEvent.fire(this, value);
}
@@ -261,7 +318,8 @@
@Override
public void sinkEvents(int eventBitsToAdd) {
if (isOrWasAttached()) {
- DOM.sinkEvents(inputElem, eventBitsToAdd | DOM.getEventsSunk(inputElem));
+ Event.sinkEvents(inputElem,
+ eventBitsToAdd | Event.getEventsSunk(inputElem));
} else {
super.sinkEvents(eventBitsToAdd);
}
@@ -280,7 +338,7 @@
super.onEnsureDebugId(baseID);
ensureDebugId(labelElem, baseID, "label");
ensureDebugId(inputElem, baseID, "input");
- DOM.setElementProperty(labelElem, "htmlFor", inputElem.getId());
+ labelElem.setHtmlFor(inputElem.getId());
}
/**
@@ -290,9 +348,7 @@
*/
@Override
protected void onLoad() {
- // Sets the event listener on the inputElem, as in this case that's the
- // element we want so input on.
- DOM.setEventListener(inputElem, this);
+ setEventListener(inputElem, this);
}
/**
@@ -304,49 +360,64 @@
protected void onUnload() {
// Clear out the inputElem's event listener (breaking the circular
// reference between it and the widget).
- DOM.setEventListener(inputElem, null);
- setChecked(isChecked());
+ setEventListener(asOld(inputElem), null);
+ setValue(getValue());
}
/**
- * Replace the current input element with a new one.
+ * Replace the current input element with a new one. Preserves
+ * all state except for the name property, for nasty reasons
+ * related to radio button grouping. (See implementation of
+ * {@link RadioButton#setName}.)
*
* @param elem the new input element
*/
protected void replaceInputElement(Element elem) {
+ InputElement newInputElem = InputElement.as(elem);
// Collect information we need to set
int tabIndex = getTabIndex();
- boolean checked = isChecked();
+ boolean checked = getValue();
boolean enabled = isEnabled();
- String uid = DOM.getElementProperty(inputElem, "id");
- String accessKey = DOM.getElementProperty(inputElem, "accessKey");
- int sunkEvents = DOM.getEventsSunk(inputElem);
+ String formValue = getFormValue();
+ String uid = inputElem.getId();
+ String accessKey = inputElem.getAccessKey();
+ int sunkEvents = Event.getEventsSunk(inputElem);
// Clear out the old input element
- DOM.setEventListener(inputElem, null);
- DOM.setEventListener(inputElem, null);
+ setEventListener(asOld(inputElem), null);
- DOM.removeChild(getElement(), inputElem);
- DOM.insertChild(getElement(), elem, 0);
+ getElement().removeChild(inputElem);
+ getElement().insertBefore(newInputElem, null);
// Sink events on the new element
- DOM.sinkEvents(elem, DOM.getEventsSunk(inputElem));
- DOM.sinkEvents(inputElem, 0);
- inputElem = elem;
+ Event.sinkEvents(elem, Event.getEventsSunk(inputElem));
+ Event.sinkEvents(inputElem, 0);
+ inputElem = newInputElem;
// Setup the new element
- DOM.sinkEvents(inputElem, sunkEvents);
- DOM.setElementProperty(inputElem, "id", uid);
+ Event.sinkEvents(inputElem, sunkEvents);
+ inputElem.setId(uid);
if (!accessKey.equals("")) {
- DOM.setElementProperty(inputElem, "accessKey", accessKey);
+ inputElem.setAccessKey(accessKey);
}
setTabIndex(tabIndex);
- setChecked(checked);
+ setValue(checked);
setEnabled(enabled);
+ setFormValue(formValue);
// Set the event listener
if (isAttached()) {
- DOM.setEventListener(inputElem, this);
+ setEventListener(asOld(inputElem), this);
}
}
+
+ private Element asOld(com.google.gwt.dom.client.Element elem) {
+ Element oldSchool = elem.cast();
+ return oldSchool;
+ }
+
+ private void setEventListener(com.google.gwt.dom.client.Element e,
+ EventListener listener) {
+ DOM.setEventListener(asOld(e), listener);
+ }
}
diff --git a/user/src/com/google/gwt/user/client/ui/RadioButton.java b/user/src/com/google/gwt/user/client/ui/RadioButton.java
index 18a68ca..a51ccb9c 100644
--- a/user/src/com/google/gwt/user/client/ui/RadioButton.java
+++ b/user/src/com/google/gwt/user/client/ui/RadioButton.java
@@ -106,6 +106,9 @@
*/
@Override
public void setName(String name) {
+ // Just changing the radio button name tends to break groupiness,
+ // so we have to replace it. Note that replaceInputElement is careful
+ // not to propagate name when it propagates everything else
super.replaceInputElement(DOM.createInputRadio(name));
}
}
diff --git a/user/test/com/google/gwt/user/client/ui/CheckBoxTest.java b/user/test/com/google/gwt/user/client/ui/CheckBoxTest.java
index c7ba8e2..333109e 100644
--- a/user/test/com/google/gwt/user/client/ui/CheckBoxTest.java
+++ b/user/test/com/google/gwt/user/client/ui/CheckBoxTest.java
@@ -15,6 +15,8 @@
*/
package com.google.gwt.user.client.ui;
+import com.google.gwt.dom.client.Document;
+import com.google.gwt.dom.client.InputElement;
import com.google.gwt.event.dom.client.ClickEvent;
import com.google.gwt.event.logical.shared.ValueChangeEvent;
import com.google.gwt.event.logical.shared.ValueChangeHandler;
@@ -28,49 +30,70 @@
*/
public class CheckBoxTest extends GWTTestCase {
+ @SuppressWarnings("deprecation")
+ static class ListenerTester implements ClickListener {
+ static int fired = 0;
+ static HandlerManager manager;
+
+ public static void fire() {
+ fired = 0;
+ manager.fireEvent(new ClickEvent() {
+ });
+ }
+
+ public void onClick(Widget sender) {
+ ++fired;
+ }
+ }
+
+ private static class Handler implements ValueChangeHandler<Boolean> {
+ Boolean received = null;
+
+ public void onValueChange(ValueChangeEvent<Boolean> event) {
+ received = event.getValue();
+ }
+ }
+
+ private CheckBox cb;
+
@Override
public String getModuleName() {
return "com.google.gwt.user.DebugTest";
}
- @Override
- protected void gwtSetUp() throws Exception {
- super.gwtSetUp();
- RootPanel.get().clear();
- }
-
- @Override
- protected void gwtTearDown() throws Exception {
- RootPanel.get().clear();
- super.gwtTearDown();
- }
-
/**
* Test accessors.
*/
+ @SuppressWarnings("deprecation")
public void testAccessors() {
- final CheckBox cb = new CheckBox();
- RootPanel.get().add(cb);
-
- // Label accessors
cb.setHTML("test HTML");
assertEquals(cb.getHTML(), "test HTML");
cb.setText("test Text");
assertEquals(cb.getText(), "test Text");
- // Input accessors
cb.setChecked(true);
assertTrue(cb.isChecked());
cb.setChecked(false);
assertFalse(cb.isChecked());
+
+ cb.setValue(true);
+ assertTrue(cb.getValue());
+ cb.setValue(false);
+ assertFalse(cb.getValue());
+
cb.setEnabled(false);
assertFalse(cb.isEnabled());
cb.setEnabled(true);
assertTrue(cb.isEnabled());
+
cb.setTabIndex(2);
assertEquals(cb.getTabIndex(), 2);
+
cb.setName("my name");
assertEquals(cb.getName(), "my name");
+
+ cb.setFormValue("valuable");
+ assertEquals("valuable", cb.getFormValue());
}
public void testDebugId() {
@@ -88,8 +111,79 @@
UIObjectTest.assertDebugIdContents("myCheck-label", "myLabel");
}
+ public void testConstructorInputElement() {
+ Element elm = DOM.createInputCheck();
+ CheckBox box = new CheckBox(elm);
+ assertFalse(box.getValue());
+ elm.setAttribute("checked", "true");
+ assertTrue(box.getValue());
+ }
+
+ public void testReplaceInputElement() {
+ cb.setValue(true);
+ cb.setTabIndex(1234);
+ cb.setEnabled(false);
+ cb.setAccessKey('k');
+ cb.setFormValue("valuable");
+
+ InputElement elm = Document.get().createCheckInputElement();
+ assertFalse(elm.isChecked());
+
+ Element asOldElement = elm.cast();
+ cb.replaceInputElement(asOldElement);
+
+ // The values should be preserved
+ assertTrue(cb.getValue());
+ assertEquals(1234, cb.getTabIndex());
+ assertFalse(cb.isEnabled());
+ assertEquals("k", elm.getAccessKey());
+ assertEquals("valuable", cb.getFormValue());
+
+ assertTrue(elm.isChecked());
+ cb.setValue(false);
+ assertFalse(elm.isChecked());
+
+ elm.setChecked(true);
+ assertTrue(cb.getValue());
+
+ // TODO: When event creation is in, test that click on the new element works
+ }
+
+ public void testFormValue() {
+ InputElement elm = Document.get().createCheckInputElement();
+ Element asOldElement = elm.cast();
+ cb.replaceInputElement(asOldElement);
+
+ assertEquals("", elm.getValue());
+ cb.setFormValue("valuable");
+ assertEquals("valuable", elm.getValue());
+
+ elm.setValue("invaluable");
+ assertEquals("invaluable", cb.getFormValue());
+ }
+
+ @SuppressWarnings("deprecation")
+ public void testListenerRemoval() {
+ ClickListener r1 = new ListenerTester();
+ ClickListener r2 = new ListenerTester();
+ ListenerTester.manager = cb.ensureHandlers();
+ cb.addClickListener(r1);
+ cb.addClickListener(r2);
+
+ ListenerTester.fire();
+ assertEquals(ListenerTester.fired, 2);
+
+ cb.removeClickListener(r1);
+ ListenerTester.fire();
+ assertEquals(ListenerTester.fired, 1);
+
+ cb.removeClickListener(r2);
+ ListenerTester.fire();
+ assertEquals(ListenerTester.fired, 0);
+ }
+
+ @SuppressWarnings("deprecation")
public void testValueChangeEvent() {
- CheckBox cb = new CheckBox();
Handler h = new Handler();
cb.addValueChangeHandler(h);
cb.setChecked(false);
@@ -118,49 +212,18 @@
/* pass */
}
}
-
- static class ListenerTester implements ClickListener {
- static int fired = 0;
- static HandlerManager manager;
-
- public static void fire() {
- fired = 0;
- manager.fireEvent(new ClickEvent() {
- });
- }
-
- public void onClick(Widget sender) {
- ++fired;
- }
+
+ @Override
+ protected void gwtSetUp() throws Exception {
+ super.gwtSetUp();
+ RootPanel.get().clear();
+ cb = new CheckBox();
+ RootPanel.get().add(cb);
}
- @SuppressWarnings("deprecation")
- public void testListenerRemoval() {
- CheckBox b = new CheckBox();
-
- ClickListener r1 = new ListenerTester();
- ClickListener r2 = new ListenerTester();
- ListenerTester.manager = b.ensureHandlers();
- b.addClickListener(r1);
- b.addClickListener(r2);
-
- ListenerTester.fire();
- assertEquals(ListenerTester.fired, 2);
-
- b.removeClickListener(r1);
- ListenerTester.fire();
- assertEquals(ListenerTester.fired, 1);
-
- b.removeClickListener(r2);
- ListenerTester.fire();
- assertEquals(ListenerTester.fired, 0);
- }
-
- private static class Handler implements ValueChangeHandler<Boolean> {
- Boolean received = null;
-
- public void onValueChange(ValueChangeEvent<Boolean> event) {
- received = event.getValue();
- }
+ @Override
+ protected void gwtTearDown() throws Exception {
+ RootPanel.get().clear();
+ super.gwtTearDown();
}
}
diff --git a/user/test/com/google/gwt/user/client/ui/RadioButtonTest.java b/user/test/com/google/gwt/user/client/ui/RadioButtonTest.java
index a03ec82..9f75759 100644
--- a/user/test/com/google/gwt/user/client/ui/RadioButtonTest.java
+++ b/user/test/com/google/gwt/user/client/ui/RadioButtonTest.java
@@ -45,9 +45,10 @@
}
/**
- * Test the name and grouping methods.
+ * Test the name and grouping methods via deprecated calls.
*/
- public void testGrouping() {
+ @SuppressWarnings("deprecation")
+ public void testGroupingDeprecated() {
// Create some radio buttons
RadioButton r1 = new RadioButton("group1", "Radio 1");
RadioButton r2 = new RadioButton("group1", "Radio 2");
@@ -76,4 +77,37 @@
assertFalse(r2.isChecked());
assertTrue(r3.isChecked());
}
+
+ /**
+ * Test the name and grouping methods.
+ */
+ public void testGrouping() {
+ // Create some radio buttons
+ RadioButton r1 = new RadioButton("group1", "Radio 1");
+ RadioButton r2 = new RadioButton("group1", "Radio 2");
+ RadioButton r3 = new RadioButton("group2", "Radio 3");
+ RootPanel.get().add(r1);
+ RootPanel.get().add(r2);
+ RootPanel.get().add(r3);
+
+ // Check one button in each group
+ r2.setValue(true);
+ r3.setValue(true);
+
+ // Move a button over
+ r2.setName("group2");
+
+ // Check that the correct buttons are checked
+ assertTrue(r2.getValue());
+ assertFalse(r3.getValue());
+
+ r1.setValue(true);
+ assertTrue(r1.getValue());
+ assertTrue(r2.getValue());
+
+ r3.setValue(true);
+ assertTrue(r1.getValue());
+ assertFalse(r2.getValue());
+ assertTrue(r3.getValue());
+ }
}
