Skip to content
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

makes copy of original configuration and use original callbacks #112

Merged
merged 2 commits into from
Feb 20, 2016

Conversation

shadybones
Copy link
Contributor

so that a global configuration object can be used
related to #111

@outsideris
Copy link
Owner

I understand what you intend in this PR (#111).
I feel it is too complex, and it is not efficient both callbacks, an original callback and a callback from attributes, are invoked in every callback.
So, I fixed such as https://gist.github.com/outsideris/4045c5912d116511d3b2. How do you think about that?
I wanted it use basically callbacks in summernote config object and overwrite it if a user specifies callback in an attribute.

@shadybones
Copy link
Contributor Author

That would work, as long as you still make a copy of the original config object (I can't see it in the gist you made):

var currentElement, summernoteConfig = angular.copy($scope.summernoteConfig) || {};

@outsideris
Copy link
Owner

@shadybones I make the diff in gist on top of your commit so, I ommit the making a copy parts.
If you agreed with the diff, please update your PR. After that I will merge it.
Let me know if you are busy. I will modify it after I merge your current PR.

change listener configuration to have attribute listeners overwrite listeners from the configuration object.
@shadybones
Copy link
Contributor Author

@outsideris I'm wondering, was there a reason onChange was done differently than the rest of the callbacks? I've implemented your changes into the PR.

@outsideris
Copy link
Owner

Thank you.
onChange is defferent because it have to sync with ngModel.

outsideris added a commit that referenced this pull request Feb 20, 2016
makes copy of original configuration and use original callbacks
@outsideris outsideris merged commit ee89dcc into outsideris:master Feb 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants