gwtbootstrap3/gwtbootstrap3 — pull request #227 by ArloL (2014-09-24 13:55:34+00:00)
Title: Correct check box and radio button implementations; fixes #216

Here we go again :)

Due to #216 I reviewed the `CheckBox`/`Radio` implementations and found a few problems. The following classes are the center of attention:

* `CheckBox`
`

Check

`
* `InlineCheckBox`
` Check`
* `CheckBoxButton`
` Check`
* `Radio`
`

Radio

`
* `InlineRadio`
` Radio`
* `RadioButton`
` Radio`

# Bad supertypes

`InputButton` implements `HasText`, `HasIcon`, `HasIconPosition` due to extending AbstractIconButton. Neither an `` nor an `` allow for child elements. Therefore it is unnecessary. Similarly for `HasDataToggle`, `HasType`, `HasSize` and `HasDataTarget`. This actually leads to the problems of #216.

`InputButton` should only extend `FocusWidget` similarly to `SimpleCheckBox`. Thinking of `SimpleCheckBox`: It is actually possible to use GWT `SimpleCheckBox` in replacement of `CheckableInputButton`. Change the constructor of `AbstractLabelButton` to

~~~
protected final SimpleCheckBox input;

protected AbstractLabelButton(final SimpleCheckBox input) {
super(ButtonType.DEFAULT);
this.input = input;
}
~~~

and pass new `SimpleRadioButton()` instead of `TypeAttr.RADIO` from e.g. `RadioButton`'s constructor. That would be a good solution: only about 30 lines of code changed and everything works.

You can see that in my [branch checkbox-fix-2](https://github.com/ArloL/gwtbootstrap3/tree/checkbox-fix-2).

Sadly not everything works:

# Handling of `ValueChangeEvent`

`CheckableInputButton` uses the change event to trigger `ValueChangeEvent`s. In modern browsers you can use a change event and everything works as expected. But IE 8 does not follow the same contract calling for solutions like using a `ClickHandler` in `CheckBox`, or manually parsing events like in GWT `RadioButton` or #226 for `SimpleRadioButton`.

Weirdly a `ClickHandler` does not get triggered for `CheckBoxButton`. My guess is that it's because the element is not visible. When you add a `ClickHandler` to the label you will trigger two `ValueChangeEvent`s in `CheckBox` because you clicked on the `` and the `` changed.

You can see that in my [branch checkbox-fix-3](https://github.com/ArloL/gwtbootstrap3/tree/checkbox-fix-3).

I don't see an easy fix for this.

In my PR I use a similar solution to #226.

# Unnecessarily complex structure

This is the current structure:

* `AbstractFormElement`
* `CheckBox` uses `CheckBoxButton`
* `Radio` uses `RadioButton`
* `AbstractLabelButton` uses `CheckableInputButton`
* `CheckBoxButton`
* `InlineCheckBox`
* `RadioButton`
* `InlineRadio`
* `InputButton`
* `CheckableInputButton`

The proposed structure looks like this:

* `CheckBox`
* `CheckBoxButton`
* `InlineCheckBox`
* `Radio`
* `RadioButton`
* `InlineRadio`

# Missing interfaces

In comparison with GWT `CheckBox` the current implementation is missing: `HasWordWrap`, `HasDirectionalSafeHtml` and `HasDirectionEstimator`.

This PR fixes that.

# Testing

I have tested the PR in IE 10, IE 10 in IE 8 compatibility mode and Chrome.

# Further discussion

The current implementation of InputGroupAddon for check box and radio is wrong. Bootstrap uses a simple [input instead of a button](http://getbootstrap.com/components/#input-groups-checkboxes-radios).