Skip to content

Conversation

@Dachaz
Copy link

@Dachaz Dachaz commented Sep 22, 2016

For LinkedInput to be actually useful as a replacement for valueLink/checkedLink on regular input elements, it needs to trigger requestChange of the ReactLink object. This can be achieved by this one-line change.

I wanted to add a corresponding test, but for tests to work for me locally, I had to revert parts of #7168, which I assume is going in a wrong direction.

A couple of additional notes on the tests:

  • Currently, they're not captured by grunt test as both the folder name is wrong and the packages folder is not in testPathDirs of package.json
  • When fixing the previous issue, and getting jest to find this package's tests, they fail on require statements. I had an idea on how to fix this, but it looked to me as if it would break npm packaging of react-linked-input and since I couldn't find instructions on how to build that package, I gave up on fixing the tests.

@ghost
Copy link

ghost commented Sep 22, 2016

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@ghost ghost added the CLA Signed label Sep 22, 2016
@ghost
Copy link

ghost commented Sep 22, 2016

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@aweary
Copy link
Contributor

aweary commented Jan 26, 2017

Thanks for the PR @Dachaz! At this point, I don't think we're going to be making changes to LinkedInput. We don't recommend using the two-way binding helpers and per #8854 it's likely that packages like this will be fully deprecated in favor of community maintained alternatives.

@aweary aweary closed this Jan 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants