search

marcorinck / jsf-updates-angular

Update angularJS after JSF Ajax requests are complete
5 stars 2 forks source link

minor bug fixes (issues #2 and #3) #4

Open stephanrauh opened 9 years ago

stephanrauh commented 9 years ago

Bug fixes for issue #2 and #3. As for issue #1: trying to use JUA on a PrimeFaces view (or another view without jsf object) now displays an alert box.

marcorinck commented 9 years ago

I'm very very sorry, I'm grateful that you want to help, but I have a problem with the style of your PR.

First, please fix only one issue per PR as it helps to keep history readable and to manage changes (for instance, I can't merge for String.contains fix bc of other problems). Second, your PR is changing the complete library as a whole with new features and with a new code style. And adds some design decisions with which I have some problems.

For instance, the IIFE (immediatly invoked function) at the top of the file is there for a reason and I would like to stay with it. Instead of creating an alert the user has to dismiss, I would prefer that JUA just does nothing when something is missing and is just adding a warning to the javascript console (for the developer to read!). And last, I would really like to stick to one code style (like if (myVar != null)).

Thank you very much for your effort, you found at least two real problems in jua (contains, not found injector) and your hint on PF is much appreciated. If we could streamline your PRs, I will merge them into jua.