-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix for issue 2521 #2523
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix for issue 2521 #2523
Conversation
|
Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test." |
|
Can one of the admins verify this patch? |
3 similar comments
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
Can one of the admins verify this patch? |
|
ok to test |
|
I need this as well.. |
|
@justinwatkins thanks for your contribution! Would you be able to add a unit test as well, then we can avoid regression of this feature down the line, and add it to master branch as well |
|
Yes, I will add a unit test as well. |
|
which ever lands first can close #1628 as well |
|
Hi @justinwatkins please mention @davidcheung and @loay after you commit the unit test. Thanks. |
|
Also please improve the commit message - describe the change in more detail, follow our guidelines for commit messages. |
|
Closing in favour of #2992 |
Passing the options through to the event handler allows for far more flexibility. Since getCurrentContext is on it's way out and currently acting more strangely than usual, this is a good alternative I think.