Author: [email protected]
Date: Mon May 11 11:43:45 2009
New Revision: 5333
Modified:
trunk/user/src/com/google/gwt/user/client/ui/CheckBox.java
trunk/user/src/com/google/gwt/user/client/ui/RadioButton.java
trunk/user/test/com/google/gwt/user/client/ui/CheckBoxTest.java
Log:
Fixes Issue 3508 Clicking on a CheckBox's label triggers two click events
CheckBoxes were sending two click events when their labels were clicked,
due to
trickery going on to send ValueChangeEvents only when appropriate.
I've moved all the trickery down to RadioButton, the only place it is
actually
needed, and filter out click events on the label.
Testing: CheckBoxTest has been extended for the specific problem, and I've
manually tested keyboard and mouse interaction with both CheckBox and
RadioButton on Safari, FF3, IE and Chrome. Full suite run against IE7.
Modified: trunk/user/src/com/google/gwt/user/client/ui/CheckBox.java
==============================================================================
--- trunk/user/src/com/google/gwt/user/client/ui/CheckBox.java (original)
+++ trunk/user/src/com/google/gwt/user/client/ui/CheckBox.java Mon May 11
11:43:45 2009
@@ -20,10 +20,6 @@
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.dom.client.KeyUpEvent;
-import com.google.gwt.event.dom.client.KeyUpHandler;
-import com.google.gwt.event.dom.client.MouseUpEvent;
-import com.google.gwt.event.dom.client.MouseUpHandler;
import com.google.gwt.event.logical.shared.ValueChangeEvent;
import com.google.gwt.event.logical.shared.ValueChangeHandler;
import com.google.gwt.event.shared.HandlerRegistration;
@@ -56,10 +52,9 @@
* </p>
*/
public class CheckBox extends ButtonBase implements HasName,
HasValue<Boolean> {
- private InputElement inputElem;
- private LabelElement labelElem;
+ InputElement inputElem;
+ LabelElement labelElem;
private boolean valueChangeHandlerInitialized;
- private boolean valueBeforeClick;
/**
* Creates a check box with no label.
@@ -118,31 +113,23 @@
ValueChangeHandler<Boolean> handler) {
// Is this the first value change handler? If so, time to add handlers
if (!valueChangeHandlerInitialized) {
-
- this.addKeyUpHandler(new KeyUpHandler() {
- public void onKeyUp(KeyUpEvent event) {
- valueBeforeClick = getValue();
- }
- });
-
- this.addMouseUpHandler(new MouseUpHandler() {
- public void onMouseUp(MouseUpEvent event) {
- valueBeforeClick = getValue();
- }
- });
-
- this.addClickHandler(new ClickHandler() {
- public void onClick(ClickEvent event) {
- ValueChangeEvent.fireIfNotEqual(CheckBox.this, valueBeforeClick,
- getValue());
- }
- });
-
+ ensureDomEventHandlers();
valueChangeHandlerInitialized = true;
}
return addHandler(handler, ValueChangeEvent.getType());
}
+ protected void ensureDomEventHandlers() {
+ addClickHandler(new ClickHandler() {
+ public void onClick(ClickEvent event) {
+ // Checkboxes always toggle their value, no need to compare
+ // with old value. Radio buttons are not so lucky, see
+ // overrides in RadioButton
+ ValueChangeEvent.fire(CheckBox.this, getValue());
+ }
+ });
+ }
+
/**
* 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
@@ -329,15 +316,13 @@
}
}
- // Unlike other widgets the CheckBox sinks on its constituent elements,
not
- // their wrapper element.
+ // Unlike other widgets the CheckBox sinks on its inputElement, not
+ // its wrapper
@Override
public void sinkEvents(int eventBitsToAdd) {
if (isOrWasAttached()) {
Event.sinkEvents(inputElem,
eventBitsToAdd | Event.getEventsSunk(inputElem));
- Event.sinkEvents(labelElem,
- eventBitsToAdd | Event.getEventsSunk(labelElem));
} else {
super.sinkEvents(eventBitsToAdd);
}
Modified: trunk/user/src/com/google/gwt/user/client/ui/RadioButton.java
==============================================================================
--- trunk/user/src/com/google/gwt/user/client/ui/RadioButton.java
(original)
+++ trunk/user/src/com/google/gwt/user/client/ui/RadioButton.java Mon May
11 11:43:45 2009
@@ -15,19 +15,32 @@
*/
package com.google.gwt.user.client.ui;
+import com.google.gwt.dom.client.Element;
+import com.google.gwt.dom.client.EventTarget;
+import com.google.gwt.event.dom.client.BlurEvent;
+import com.google.gwt.event.dom.client.ClickEvent;
+import com.google.gwt.event.dom.client.KeyDownEvent;
+import com.google.gwt.event.dom.client.MouseUpEvent;
+import com.google.gwt.event.logical.shared.ValueChangeEvent;
import com.google.gwt.user.client.DOM;
+import com.google.gwt.user.client.Event;
/**
- * A mutually-exclusive selection radio button widget.
+ * A mutually-exclusive selection radio button widget. Fires {...@link
ClickEvent}s
+ * when the radio button is clicked, and {...@link ValueChangeEvent}s when the
+ * button becomes checked. Note, however, that browser limitations prevent
+ * ValueChangeEvents from being sent when the radio button is cleared as a
side
+ * effect of another in the group being clicked.
*
* <p>
* <img class='gallery' src='RadioButton.png'/>
* </p>
*
- * <h3>CSS Style Rules</h3>
- * <ul class='css'>
- * <li>.gwt-RadioButton { }</li>
- * </ul>
+ * <h3>CSS Style Rules</h3>
+ * <dl>
+ * <dt>.gwt-RadioButton</dt>
+ * <dd>the outer element</dd>
+ * </dl>
*
* <p>
* <h3>Example</h3> {...@example com.google.gwt.examples.RadioButtonExample}
@@ -35,20 +48,84 @@
*/
public class RadioButton extends CheckBox {
+ private Boolean oldValue;
+
/**
* Creates a new radio associated with a particular group name. All radio
* buttons associated with the same group name belong to a
mutually-exclusive
* set.
*
- * Radio buttons are grouped by their name attribute, so changing their
- * name using the setName() method will also change their associated
- * group.
+ * Radio buttons are grouped by their name attribute, so changing their
name
+ * using the setName() method will also change their associated group.
*
* @param name the group name with which to associate the radio button
*/
public RadioButton(String name) {
super(DOM.createInputRadio(name));
setStyleName("gwt-RadioButton");
+
+ sinkEvents(Event.getTypeInt(MouseUpEvent.getType().getName()));
+ sinkEvents(Event.getTypeInt(BlurEvent.getType().getName()));
+ sinkEvents(Event.getTypeInt(KeyDownEvent.getType().getName()));
+ }
+
+ /**
+ * Overridden to send ValueChangeEvents only when appropriate.
+ */
+ @Override
+ public void onBrowserEvent(Event event) {
+ switch (DOM.eventGetType(event)) {
+ case Event.ONMOUSEUP:
+ case Event.ONBLUR:
+ case Event.ONKEYDOWN:
+ // Note the old value for onValueChange purposes (in ONCLICK case)
+ oldValue = getValue();
+ break;
+
+ case Event.ONCLICK:
+ EventTarget target = event.getEventTarget();
+ if (Element.is(target) &&
labelElem.isOrHasChild(Element.as(target))) {
+
+ // They clicked the label. Note our pre-click value, and
+ // short circuit event routing so that other click handlers
+ // don't hear about it
+ oldValue = getValue();
+ return;
+ }
+
+ // It's not the label. Let our handlers hear about the
+ // click...
+ super.onBrowserEvent(event);
+ // ...and now maybe tell them about the change
+ ValueChangeEvent.fireIfNotEqual(RadioButton.this, oldValue,
getValue());
+ return;
+ }
+
+ super.onBrowserEvent(event);
+ }
+
+ @Override
+ public void sinkEvents(int eventBitsToAdd) {
+ // Like CheckBox, we want to hear about inputElem. We
+ // also want to know what's going on with the label, to
+ // make sure onBrowserEvent is able to record value changes
+ // initiated by label events
+ if (isOrWasAttached()) {
+ Event.sinkEvents(inputElem, eventBitsToAdd
+ | Event.getEventsSunk(inputElem));
+ Event.sinkEvents(labelElem, eventBitsToAdd
+ | Event.getEventsSunk(labelElem));
+ } else {
+ super.sinkEvents(eventBitsToAdd);
+ }
+ }
+
+ /**
+ * No-op. CheckBox's click handler is no good for radio button, so don't
use
+ * it. Our event handling is all done in {...@link #onBrowserEvent}
+ */
+ @Override
+ protected void ensureDomEventHandlers() {
}
/**
@@ -56,9 +133,8 @@
* with the given HTML label. All radio buttons associated with the same
group
* name belong to a mutually-exclusive set.
*
- * Radio buttons are grouped by their name attribute, so changing their
- * name using the setName() method will also change their associated
- * group.
+ * Radio buttons are grouped by their name attribute, so changing their
name
+ * using the setName() method will also change their associated group.
*
* @param name the group name with which to associate the radio button
* @param label this radio button's label
@@ -74,9 +150,8 @@
* buttons associated with the same group name belong to a
mutually-exclusive
* set.
*
- * Radio buttons are grouped by their name attribute, so changing their
- * name using the setName() method will also change their associated
- * group.
+ * Radio buttons are grouped by their name attribute, so changing their
name
+ * using the setName() method will also change their associated group.
*
* @param name name the group with which to associate the radio button
* @param label this radio button's label
@@ -94,19 +169,18 @@
/**
* Change the group name of this radio button.
*
- * Radio buttons are grouped by their name attribute, so changing their
- * name using the setName() method will also change their associated
- * group.
- *
- * If changing this group name results in a new radio group with
- * multiple radio buttons selected, this radio button will remain
- * selected and the other radio buttons will be unselected.
- *
+ * Radio buttons are grouped by their name attribute, so changing their
name
+ * using the setName() method will also change their associated group.
+ *
+ * If changing this group name results in a new radio group with multiple
+ * radio buttons selected, this radio button will remain selected and the
+ * other radio buttons will be unselected.
+ *
* @param name name the group with which to associate the radio button
*/
@Override
public void setName(String name) {
- // Just changing the radio button name tends to break groupiness,
+ // 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));
Modified: trunk/user/test/com/google/gwt/user/client/ui/CheckBoxTest.java
==============================================================================
--- trunk/user/test/com/google/gwt/user/client/ui/CheckBoxTest.java
(original)
+++ trunk/user/test/com/google/gwt/user/client/ui/CheckBoxTest.java Mon May
11 11:43:45 2009
@@ -17,7 +17,9 @@
import com.google.gwt.dom.client.Document;
import com.google.gwt.dom.client.InputElement;
+import com.google.gwt.dom.client.NativeEvent;
import com.google.gwt.event.dom.client.ClickEvent;
+import com.google.gwt.event.dom.client.ClickHandler;
import com.google.gwt.event.logical.shared.ValueChangeEvent;
import com.google.gwt.event.logical.shared.ValueChangeHandler;
import com.google.gwt.event.shared.HandlerManager;
@@ -29,8 +31,7 @@
* Tests the CheckBox Widget.
*/
public class CheckBoxTest extends GWTTestCase {
-
- @SuppressWarnings("deprecation")
+ @SuppressWarnings("deprecation")
static class ListenerTester implements ClickListener {
static int fired = 0;
static HandlerManager manager;
@@ -44,8 +45,9 @@
public void onClick(Widget sender) {
++fired;
}
- }
+ }
+
private static class Handler implements ValueChangeHandler<Boolean> {
Boolean received = null;
@@ -166,6 +168,53 @@
assertEquals(ListenerTester.fired, 0);
}
+ public void testCheckboxClick() {
+ final int[] clickCount = {0};
+
+ CheckBox check = new CheckBox();
+ Element newInput = DOM.createInputCheck();
+ check.replaceInputElement(newInput);
+
+ check.setText("Burger");
+ check.addClickHandler(new ClickHandler() {
+ public void onClick(ClickEvent arg0) {
+ clickCount[0]++;
+ }
+ });
+ RootPanel.get().add(check);
+
+ NativeEvent e = Document.get().createClickEvent(0, 25, 25, 25, 25,
false,
+ false, false, false);
+
+ newInput.dispatchEvent(e);
+ assertEquals(1, clickCount[0]);
+ }
+
+ public void testLabelClick() {
+ final int[] clickCount = {0};
+
+ CheckBox check = new CheckBox();
+ Element newInput = DOM.createInputCheck();
+ check.replaceInputElement(newInput);
+
+ check.setText("Burger");
+ check.addClickHandler(new ClickHandler() {
+ public void onClick(ClickEvent arg0) {
+ clickCount[0]++;
+ }
+ });
+ RootPanel.get().add(check);
+
+ NativeEvent e = Document.get().createClickEvent(0, 25, 25, 25, 25,
false,
+ false, false, false);
+
+ // click the label, which is next to the checkbox
+ // http://code.google.com/p/google-web-toolkit/issues/detail?id=3508
+
+ newInput.getNextSiblingElement().dispatchEvent(e);
+ assertEquals(1, clickCount[0]);
+ }
+
public void testReplaceInputElement() {
cb.setValue(true);
cb.setTabIndex(1234);
@@ -192,8 +241,6 @@
elm.setChecked(true);
assertTrue(cb.getValue());
-
- // TODO: When event creation is in, test that click on the new element
works
}
@SuppressWarnings("deprecation")
@@ -225,6 +272,11 @@
} catch (IllegalArgumentException e) {
/* pass */
}
+
+ // Note that we cannot test this with a simulated click, the way
+ // we do for the click handlers. IE does not change the value of
+ // the native checkbox on simulated click event, and there's
+ // naught to be done about it.
}
@Override
--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---
