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

added support for $onChanges, added new binding to disable change detection #622

Merged
merged 2 commits into from
Nov 3, 2017

Conversation

ngehlert
Copy link
Contributor

like discussed in #621 I added support for $onChanges and the new disableChangeDetection binding.
a little side not: due the nature of $onChanges being called before $onInit but the DOM not rendered at this point of time I still kept support for $onInit. So everything stays the same, I just added the $onChanges method to support later changes. A flag if the $onInit was already called is needed for this.

Should I also update the documentation?
Let me know if you have any questions.

@pablojim
Copy link
Owner

pablojim commented Nov 3, 2017

@ngehlert Looks good so far if you could add something to the docs and add a jsfiddle example to the examples directory then I'd be happy to merge.

Thanks!

@ngehlert
Copy link
Contributor Author

ngehlert commented Nov 3, 2017

@pablojim i updated the readme. not quite sure how i should add a jsfiddle since it is not merged yet.
should i use my repository as lib endpoint? should i build it against the current master version, but then you can't test it before the merge? or should i add them in a separate PR after this one is complete?
maybe you have a better idea 🤔 😄

@pablojim pablojim merged commit 6801541 into pablojim:master Nov 3, 2017
@pablojim
Copy link
Owner

pablojim commented Nov 3, 2017

@ngehlert Thanks. I've merged that. If you get a chance to make a jsfiddle and link to it from the readme that would be great.

@ngehlert
Copy link
Contributor Author

ngehlert commented Nov 3, 2017

@pablojim ok awesome. one last question regarding the jsfiddle. do i just create/update an example there and hit save and add the link? i saw that you have a separate directory with jsfiddle files. how does this work, where do they come from?

ngehlert added a commit to ngehlert/highcharts-ng that referenced this pull request Nov 3, 2017
* upstream/master:
  added support for $onChanges, added new binding to disable change detection (pablojim#622)
@pablojim
Copy link
Owner

pablojim commented Nov 3, 2017

So http://jsfiddle.net/gh/get/jquery/3.1.1/pablojim/highcharts-ng/tree/master/jsfiddles/basic/

This gets the html, js and "details" from:
https://github.com/pablojim/highcharts-ng/tree/master/jsfiddles/basic

It means we can manage the source of example jsfiddles in github. This is prefereable to just creating one in jsfiddle itself.

Easiest to just copy one of the existing examples and modify it.

@ngehlert
Copy link
Contributor Author

ngehlert commented Nov 3, 2017

ok, didn't know that. thanks. this is actually a nice feature 😄 going to add the examples then

@ngehlert
Copy link
Contributor Author

ngehlert commented Nov 3, 2017

@pablojim sorry to bother you again. do you have any plans for the release date? is there any other issue / pr you want to have in there as well and need some help with?

@pablojim
Copy link
Owner

pablojim commented Nov 3, 2017

@ngehlert planning to do a release once #623 is dealt with.

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.

None yet

2 participants