Fixes chekedLink inconsistency (controlled inputs)

[cirocosta]

As stated on a thread in ReactJS mailing list, there is an inconsistency when using checkedLink and valueLink. With the second are totally able to create a controlled component, whilst with the first, we aren't.

This PR fixes it by getting the value from the LinkedValueUtils just like how it is done for valueLink, making it consistent.

[zpao]

cc @petehunt

I think this is right but I haven't looked at the valuelink stuff in a long time. Pete might have a better idea.

[sophiebits]

nit: no spaces within braces

[sophiebits]

Looks good except a few style nits – mind fixing those up?

[syranide]

Just bumped into this myself 👍

[syranide]

!!checked ... but that shouldn't be needed either, I'm pretty sure checked is enough.

[cirocosta]

Sorry for taking so long on this, had a crazy week full of tests at the university.

Just made those changes pointed and squashed

[syranide]

@cirocosta The implementation looks great to me.

For completeness, perhaps we should copy/reuse the checkbox test and apply it for radio as well, while we're at it?

[cirocosta]

@syranide , agree with you! I added a test for it. What do you think about the location of the test? I think where i put it (not right after the first test for radios) because in this we are relying on ReactLink, which is tested later, so i think it makes sense adding this test after it.

This test is also a little bigger than the test for checkbox as radio inputs behave differently from checkboxes, as i stated on the commit description. What do you think? Thanks!

[syranide]

While not technically wrong, setting only radioB.checked = true; is enough.

[syranide]

@cirocosta Checking that ReactLink works before checking specific details of it seems sane yes. I'm not all that familiar with tests myself yet, but apart from my two minor nits it looks good to me at least. :)

[syranide]

This shouldn't actually be needed I think. It should be safe to just put null instead of this.handleChange for ReactLink (applies to all of them).

[cirocosta]

Made those changes @syranide . Thinking again it was actually redundant to do those checks. And yeah, assigning var node = stub.getDOMNode() makes much more sense as the operation was being done everywhere. Thanks for the tips!

[syranide]

    var RadioInput = React.createClass({
      render: function () {
        return (
          <input
            type="radio"
            checkedLink={new ReactLink(false, null)} />
        );
      }
    });

Should be enough I think.
Technically this should even be enough:

var stub = ReactTestUtils.renderIntoDocument(
  <input type="radio" checkedLink={new ReactLink(false, null)} /> 
);

But perhaps we don't want to do that? @zpao @spicyj

[cirocosta]

actually the second arg must be a function (emptyFunction could be used) or it will throw as i remember. I'll take a look 👍

---

Thanks for the catch. Much more concise now @syranide :)

[syranide]

There's no doubt to me that the fix is correct 👍

In light of separate discussions about possibly dropping ReactLink support from core, having tests for non-ReactLink controlled inputs too would probably be a good idea. But I'll let @zpao weigh in on that (sorry for being annoying :)).

[jimfb]

This PR is over a year old, and doesn't merge cleanly. Plus, we're getting rid of value-linking in the core. So... I'm going to close this out.