PTNFLY-1122 : Fix pfSelect options and ng-model watches #187
Conversation
The directive was watching the wrong scope for changes, since it defines
its isolated scope, it will need to watch parent scope.
Also, wrapping selectpicker('refresh') calls in $applyAsync so angular
is quickly aware of changes and shows them.
This fixes 2 problems, not showing changes to options (eg: adding a new
element to options) and not reflecting changes to ng-model (eg: changing
ng-model outside of directive, like with result from REST call).
Fixes 177
|
Cloned, built, ran unit tests, and viewed ng-docs. Looks good to me. However, I noticed that the unit test: angular-patternfly/test/select/select.spec.js has not been updated to test this new functionality. Could you please add one or more unit tests so we can test this new functionality?
|
|
Also, you need to add to this PR all the /dist files which have changed! |
Tests for the bootstrap select generated <div> element to be in sync with ng-options and ng-model (along with the <select> element)
|
@dtaylor113 tests are now part of the PR. Didn't wanted to commit the dist files as it is generally considered a bad practice (that way 2 or more simultaneous PR's will always have merge issues), so I prefer to leave it to the maintainers to generate them as they see appropriate. Let me know if I should still do it. Thanks! |
|
Hi, we have consumers of Patternfly who use the generated /dist files directly; they do not want to have to download and build the repo in order to get these. We require contributors to include the /dist files as part of any PR. |
|
Hi. I know we do, and I am also one of them. Was just saying that it makes it difficult to merge "simultaneous" PRs even if they only touch different files, since all dist files will be altered and will cause collisions. Was just raising awareness for that.. typically when there are dist generated, they should be made apart from the PRs (be it by who merges the PR or at the end of the day, or before release, etc). Something you should think about :) |
|
LGTM |
PTNFLY-1122 : Fix pfSelect options and ng-model watches
The directive was watching the wrong scope for changes, since it defines
its isolated scope, it will need to watch parent scope.
Also, wrapping selectpicker('refresh') calls in $applyAsync so angular
is quickly aware of changes and shows them.
This fixes 2 problems, not showing changes to options (eg: adding a new
element to options) and not reflecting changes to ng-model (eg: changing
ng-model outside of directive, like with result from REST call).
Fixes 177