Skip to content
This repository was archived by the owner on Aug 23, 2022. It is now read-only.

Resolve Issue 905, defaultChecked bug on Control.checkbox #908

Merged
merged 6 commits into from
Aug 22, 2017

Conversation

maludwig
Copy link
Contributor

This makes defaultChecked work as expected, populating the model properly and everything. Also added some failing tests for the bug, and then made the tests pass.

Minor largely irrelevant changes, I use IDEA as my IDE, and I needed to .gitignore its files.
Also, in debugging, I found that areOwnPropsEqual and areStatePropsEqual had a different signature than assumed, which was lightly confusing, so I fixed the signature. Finally, "modelValue" was set in the return function, which I edited to make it so that I could variable-inspect it, but then decided to leave it in the end, because it followed the format of all the other return values, and I felt dirty setting it back.

Actual meat and potatoes are in "src/actions/model-actions.js" and "src/constants/control-props-map.js", which, are, like, 5 lines of change. The change action was ignoring the second parameter, and control-props-map was giving weird results, and caring about defaultChecked. Not sure why, but removing it solved all things.

@davidkpiano
Copy link
Owner

Pull from master and try again please 🙏

@maludwig
Copy link
Contributor Author

Yay! It built fine this time! Good fix, thanks!

@davidkpiano davidkpiano merged commit ffcc6a8 into davidkpiano:master Aug 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants