-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(events): expose uis-open-close callback #1723
Conversation
@user378230 hey, Is this feature in general fits in current roadmap or for now entire codebase is in "maintenance only" mode? |
Could you extract this as a separate directive? (See: Accessing $select) |
@user378230 Yep, this is a very good idea. But not sure which path to choose: Extract this functionality to a separate directive called uiSelect (directive stacking) with two optional attributes on-open and on-close. pros: 👍
cons: 👎
Or create two separate directives: on-open and on-close pros: 👍
cons: 👎
|
One directive (Eg. (uis)OpenCloseCallback or (uis)OpenClose?), one callback and pass true/false to callback for |
It works for me! would love to see this merged 👍 |
@Thirsa Thank you for your interest in this PR. I will get it done by this weekend. |
Ok, here is how it looks like after refactoring. @user378230 when you got some time please take a look and share your thoughts on this. Thanks |
This works smoothly for me as well 👍 |
@user378230 done. https://github.com/angular-ui/ui-select/wiki/uis-open-close Looks pretty simple, but should be a good starting point. |
Might be handy to mention the version in which this was added? Op 1 aug. 2016 15:56 schreef "Denis Bendrikov" notifications@github.com:
|
@ceeram yes, it's easy. Not 100% sure what version will be the next though 😄 |
Is this included in v0.19.1? |
it is in 0.19.0 already : https://github.com/angular-ui/ui-select/commits/v0.19.0 it is incorrectly listed in the changelogs |
@Den-dp When is 0.19.2 supposed to drop? |
Unfortunately this is not quite true for npm. You can try to search for The fun part is this directive is available in @PhilDore11 0.19.2 is already available in npm. |
@Den-dp Is that also true for bower? |
@pdore-netfore sorry, but it is not. It looks like you should wait for the 0.19.2 tag https://github.com/angular-ui/ui-select/tags (bower reads this tags to get a list of available versions) or maybe file an issue. |
In order to push this feature (#1153) I'm suggesting this PR which has a slightly updated and rebased code with a couple of unit tests.
If all this makes sense but some changes are required please let me know.
Closes #432 #1153