search

LHNCBC / lforms

See the project website at http://lhncbc.github.io/lforms/, or view the demo website at https://lhcforms.nlm.nih.gov/lhcforms.
Other
107 stars 56 forks source link

item._otherValueChecked not true inside updateRadioListForOther() #24

Open dwwinters opened 4 years ago

dwwinters commented 4 years ago

Ran into another test which is failing on my windows/chrome build: https://github.com/lhncbc/lforms/blob/master/test/protractor/spec/lforms_display_controls.spec.js#L81

From what I can gather, clicking on the OTHER radio button should cause value.code and value.text to be null/undefined in $scope. However this doesn't happen on my installation and thus the above test fails. Clicking on any other radio button in the question fires $scope.updateRadioList which sets item._otherValueChecked to false: https://github.com/lhncbc/lforms/blob/master/app/scripts/lforms-controllers.js#L1068

Clicking on the OTHER radio button I can see within $scope.updateRadioListForOther that item._otherValueChecked is still false: https://github.com/lhncbc/lforms/blob/master/app/scripts/lforms-controllers.js#L1077 Clicking the OTHER radio button a second time or entering characters into the text input box causes $scope.updateRadioListForOther to fire again and this time item._otherValueChecked is true.

So for whatever reason item._otherValueChecked isn't getting toggled until after $scope.updateRadioListForOther on my system. This was easy to tell since I can see the text input box showing up after clicking on OTHER. Would it make sense to set _otherValueChecked to true directly within $scope.updateRadioListForOther? This seems to work for me, but wondering if it breaks anything on linux.

plynchnlm commented 4 years ago

Thanks for the report. I've confirmed this happens with AngularJS 1.7.9 (though it worked with 1.5.8).

plynchnlm commented 4 years ago

I think we might have fixed this in [21.0.0] 2020-02-26, because we updated the bower components to Angular 1.7.9 and had to make a change to the event listener. If you are able to re-test with the latest lforms, that would be helpful.