base: update to angular 1.5 and boostrap ui 2.1

[ianchi]

Update libraries to more up to date versions, so all APIs are adjusted before code base growths to much. Main changes:

• Angular to 1.5 branch
• Boostrapt to 2.1branch
• Libraries moved to \libs folder
• Adjusted all Boostrap directives with the new 'uib' prefix
• Replace overriding 'angular.element' with (temporary) adding a function '.findAll()'. As now the original object is needed by Angular Animate
• Remove jQuery-like code using findXXX() with more Angular-like code where possible. Still pending to convert the 'tabs' inside cbi to a directive, to eliminate all use of the findXXXs.

[jow-]

Hi @ianchi - I just wanted to leave a note here to tell you that I didn't forget about those PRs - I hope to have them reviewed within the next days.

[ianchi]

Change logic to decentralize wait functions to each element. Add a block on cbiMap while in any wait operation.

As per discussion in PR #22

[ianchi]

@jow- please let me know what you think about this approach.

I still have a pending issue with cbiNetworkList and cbiDeviceList. I need to look more carefully at the code, but for this controls I can't find the binding of element to model data. They don't see any ng-model directive. If you can hint how it is done, it will save me some time.

[jow-]

@ianchi - I like the lock count approach and am currently in the process of testing your changes.

The cbiNetworkList and cbiDeviceList widgets do not bind to the data model through directives but propagate the user choice through ng-click events which call select(). The select function takes care of normalizing the input value and storing it in the data model.

[ianchi]

@jow- sorry I wasn't very specific. What I don't see is how changes in the model propagate to the control (after init). Specifically, after a reset.

[jow-]

@ianchi - ah, now I get you. Well there is no model binding at all atm since back when I started to implement the CBI directives I had some troubles using shared states etc.

But now that I think about it, making the sub-directive observe things like .uciValue of the owner option would simplify a lot of things but I do not know how to properly implement that yet - I guess by using $watch ?

[ianchi]

yes, I think that might be the easiest right now. An alternative way could be to use the same ngModel approach, but you need a form control to bind, so include a hidden in sync. Much the way many "skins" on combo boxes work.

jow- commented 7 years ago

@ianchi - hm, what would you prefer? I could live with a hidden select / hidden field as well

jow- commented 7 years ago

So assuming we would use the $watch() approach we would basically re-render the drop down caption on self.cbiOwnerOption.uciValue changes and turn select() into a simple setter for self.cbiOwnerOption.uciValue. Would that work?

ianchi commented 7 years ago

Yes, I think that should work

jow- commented 7 years ago

@ianchi - I picked your original three commits for bumping Angular and Bootstrap but did the other CBI changes from scratch using input from your proposed changes - I hope this is okay with you.

ianchi commented 7 years ago

It's OK, but I have some comments:

• you left out cbiMap .read() function, but you still call it after save.

• You no longer wait on the parents promise. You can't be sure that the parent has completely initialized when the child "finish" is called. This is not necessary a problem, but it depends on the situation, specially if an external waitfor attribute was set. For me the most intuitive behavior would be that if I set a "waitfor" attribute at cbiMap level, all the nested sections/controls should wait for that. The same if I set it at section level, I would expect all inner options to wait for that, and not that it applies only to enabling the save buttons section. Removing the ".waitPromise" property for child objects to wait for, makes each level completely independent. I think that the first thing is to clearly define what the scope of the "waitfor" attribute is. Depending on the extent you want to give it, waiting on parent "waitPromise" is even not enough. As angular's compile/linking of the child would not wait for that, and if they contained some dynamic content dependent on some data waited for, they will fail to render properly. That was why I nested the cbiMap inside a ng-if, so that even the compile of the children would wait. But I was inconsistent, as I only did this on Map but not Section level.

Anyway, it's not clear for me what is the intended behavior of an external waitfor attribute. I think it must be more clearly defined and handled, or in the current state I almost think that is better to remove it and leave it to the user to handle it in each page.

• Regarding the locking mechanism. I preferred to leave the "unlock" call always in the same function that locked, so it was clearer, and not so easy to forget to unlock, or to have unbalanced calls to unlock. You have a function with an "unlock" at the end, but its not evident and you cannot be positively sure that there has been a "lock" before it was called. You have to follow all possible calling paths to check it.

jow- commented 7 years ago

1) Indeed, I'll fix that soon, probably along with my plan to properly implement apply/rollback 2) The waitfor attribute was something I conceptually carried over from the hand-crafted initial LuCI2 prototype - I now I think that it was a badly thought out idea and that we should rather work on getting rid of it - it simply adds complexity to the whole CBI layer for the sake of accomodating only one or two exotic use cases - so I'm with you here on removing it and letting CBI users to deal with data loading. 3) Ok - I can consider reshaping the code here. I just try to avoid creating anon closures as much as possible, thats why I usually end up distributing logic over multiple functions. This is probably bad habits brought over from old JavaScript days...

• © Githubissues.
• Githubissues is a development platform for aggregating issues.

[jow-]

@ianchi - hm, what would you prefer? I could live with a hidden select / hidden field as well

[jow-]

So assuming we would use the $watch() approach we would basically re-render the drop down caption on self.cbiOwnerOption.uciValue changes and turn select() into a simple setter for self.cbiOwnerOption.uciValue. Would that work?

[ianchi]

Yes, I think that should work

[jow-]

@ianchi - I picked your original three commits for bumping Angular and Bootstrap but did the other CBI changes from scratch using input from your proposed changes - I hope this is okay with you.

[ianchi]

It's OK, but I have some comments:

• you left out cbiMap .read() function, but you still call it after save.

• You no longer wait on the parents promise. You can't be sure that the parent has completely initialized when the child "finish" is called. This is not necessary a problem, but it depends on the situation, specially if an external waitfor attribute was set. For me the most intuitive behavior would be that if I set a "waitfor" attribute at cbiMap level, all the nested sections/controls should wait for that. The same if I set it at section level, I would expect all inner options to wait for that, and not that it applies only to enabling the save buttons section. Removing the ".waitPromise" property for child objects to wait for, makes each level completely independent. I think that the first thing is to clearly define what the scope of the "waitfor" attribute is. Depending on the extent you want to give it, waiting on parent "waitPromise" is even not enough. As angular's compile/linking of the child would not wait for that, and if they contained some dynamic content dependent on some data waited for, they will fail to render properly. That was why I nested the cbiMap inside a ng-if, so that even the compile of the children would wait. But I was inconsistent, as I only did this on Map but not Section level.

Anyway, it's not clear for me what is the intended behavior of an external waitfor attribute. I think it must be more clearly defined and handled, or in the current state I almost think that is better to remove it and leave it to the user to handle it in each page.

• Regarding the locking mechanism. I preferred to leave the "unlock" call always in the same function that locked, so it was clearer, and not so easy to forget to unlock, or to have unbalanced calls to unlock. You have a function with an "unlock" at the end, but its not evident and you cannot be positively sure that there has been a "lock" before it was called. You have to follow all possible calling paths to check it.

[jow-]

1) Indeed, I'll fix that soon, probably along with my plan to properly implement apply/rollback 2) The waitfor attribute was something I conceptually carried over from the hand-crafted initial LuCI2 prototype - I now I think that it was a badly thought out idea and that we should rather work on getting rid of it - it simply adds complexity to the whole CBI layer for the sake of accomodating only one or two exotic use cases - so I'm with you here on removing it and letting CBI users to deal with data loading. 3) Ok - I can consider reshaping the code here. I just try to avoid creating anon closures as much as possible, thats why I usually end up distributing logic over multiple functions. This is probably bad habits brought over from old JavaScript days...