[OPEN-ILS-DEV] Web Client fixes and AngularJS Best Practices

[Boyer, Jason A]

As I was working on https://bugs.launchpad.net/evergreen/+bug/1642035 I managed to get things working without too much trouble except for the SMS option. No matter what I did changes in its value weren't reflected until in a fit of flailing about I commented out the ng-if="org_settings['sms.enable']" directive from the span around it; *then* everything worked as expected. Several annoying Google searches later and I find myself here: https://github.com/angular/angular.js/wiki/Understanding-Scopes , which explains that there are multiple directives that introduce a new scope and while references to parent objects work as expected within this child scope, bare primitives (the JS equiv of a perl scalar, basically) on $scope are *copied* to the child scope and so changes to them don't make it back up the chain. So when you do something like this: <div ng-if="showme"><input type="checkbox" ng-model="enabled"/></div> the controller's interpretation of $scope.enabled will have nothing to do with the actual state of that checkbox because the child scope will have a copy of the parent's primitive, not a reference to it.

To avoid being surprised by this there is a semi-official best practice of always having a '.' in your ng-model directives (mentioned in the above link). ([] also works because a JS array is an instance of an Array object) So the above example would work fine if it were <div ng-if="showme"><input type="checkbox" ng-model="uiState.enabled"/></div> instead.

Which brings me to the actual point (only 3 paragraphs in...) If you execute ` grep -rE 'ng-model="[a-zA-Z_]+"' * ` at Open-ILS/src/templates/staff/, back come 72 instances where we use an unadorned primitive in an ng-model directive. *Most* of these are probably ok because they're in the same scope as everything else around them but they're only an enclosing ng-(if, switch, repeat, etc.) away from becoming totally disconnected from the outside world which will make regressions a real possibility and a real pain to track down. (For instance, I looked at https://bugs.launchpad.net/evergreen/+bug/1665465 shortly after 1642035 and it's the same thing, there's an ng-if that prevents the controller from ever seeing that you checked the box.)

SO, rather than fix these on a case by case basis I thought I'd do two things: 1, ask if there are strong opinions for / against wrapping ALL variables used in an ng-model in an enclosing object like uiState or similar, and 2, ask for some help in either transitioning everything over to that OR at least fixing all of this list of 72 that are contained within a directive that creates a child scope (it's not just ng-if!).

What say you, devs?

Jason

--
Jason Boyer
MIS Supervisor
Indiana State Library
http://library.in.gov/

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://libmail.georgialibraries.org/pipermail/open-ils-dev/attachments/20170602/bc855560/attachment.html>