Added support for radio buttons #62
Conversation
|
@grprakash Thanks for the PR! Unfortunately, the PR broke CI. Could you look into that? |
Hey Erik, Sorry, to say the good old, "works in my machine", but all the Unit Tests including the ones that I have added, are working fine in my local (screenshot below). The Unit Tests that are failing in the CI doesn't seem to relate to the changes that I have made. Is there a way to debug/check what is wrong in the CI?
|
|
Weird. You can see the error log here: https://ci.appveyor.com/project/ErikSchierboom/knockout-pre-rendered/builds/43832940 |
Yes, I checked the logs, and I see four failures. None of them related to the change that I have done. Couldn't figure out why it would fail only in CI. By any chance, the PR works for you in your local? |
|
I'm thoroughly swamped with work. I'll try to find some time. |
|
@grprakash I've pushed a commit to update nodejs to version 16. That fixes the issue. I'll review the PR a bit more in-depth. |
|
The code looks good! Two suggestions:
|
Done. Kindly check |
| @@ -311,7 +311,8 @@ There is a JSBin demo for each of the binding handlers: | |||
| - [`init` with value binding](http://jsbin.com/xuluye/) | |||
| - [`init` with html binding](http://jsbin.com/wilahi/) | |||
| - [`init` with attr binding](http://jsbin.com/zehivi/) | |||
| - [`init` with checked binding](http://jsbin.com/zemode/) | |||
| - [`init` with checked binding for checkbox](http://jsbin.com/zemode/) | |||
| - [`init` with checked binding for radio](http://jsbin.com/pumodetaqu/) | |||
There was a problem hiding this comment.
I have one suggestion: the UI currently displays "Plays" before each radiobutton. Could you add what is being played to the text?
Fixes issue #61